82200 2012-03-26 07:01:58 -0700 Reset paramters when BiquadFilterNode.type get new value. 2012-03-27 13:19:05 -0700 1 1 1 Unclassified WebKit Web Audio 528+ (Nightly build) Unspecified Unspecified RESOLVED INVALID P2 Normal --- 0 chun.gao webkit-unassigned crogers jer.noble oldest_to_newest 587686 0 chun.gao 2012-03-26 07:01:58 -0700 When BiquadFilterNode.type changed, the Q and gain paramters should be set with new names and ranges. 587837 1 133835 chun.gao 2012-03-26 09:45:06 -0700 Created attachment 133835 Patch 588560 2 133835 crogers 2012-03-26 22:09:12 -0700 Comment on attachment 133835 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133835&action=review Hi Chun, thanks for the patch. I don't think we should change the AudioParams in this way, but a good change would be to remove the deprecated filter nodes. > Source/WebCore/Modules/webaudio/BiquadProcessor.cpp:-48 > - m_parameter3 = AudioParam::create("gain", 0.0, -40, 40); The attributes for BiquadFilterNode are defined in the spec as "frequency", "Q", "gain", so I think we shouldn't remove this code here, even though for some types "Q" is unused. > Source/WebCore/Modules/webaudio/BiquadProcessor.cpp:-76 > - } I think it may be a good time to think about removing these old/deprecated AudioNodes (LowPass2FilterNode and HighPass2FilterNode). There have been warning messages for many months. But that should probably be in a different patch called "Remove deprecated LowPass2FilterNode and HighPass2FilterNode" > Source/WebCore/Modules/webaudio/BiquadProcessor.cpp:157 > +} I don't think we should re-create these AudioParams -- instead just create in the normal constructor and leave named the same always 588578 3 chun.gao 2012-03-26 22:53:23 -0700 (In reply to comment #2) > (From update of attachment 133835 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=133835&action=review > > Hi Chun, thanks for the patch. I don't think we should change the AudioParams in this way, but a good change would be to remove the deprecated filter nodes. > > > Source/WebCore/Modules/webaudio/BiquadProcessor.cpp:-48 > > - m_parameter3 = AudioParam::create("gain", 0.0, -40, 40); > > The attributes for BiquadFilterNode are defined in the spec as "frequency", "Q", "gain", so I think we shouldn't remove this code here, even though for some types "Q" is unused. > > > Source/WebCore/Modules/webaudio/BiquadProcessor.cpp:-76 > > - } > > I think it may be a good time to think about removing these old/deprecated AudioNodes (LowPass2FilterNode and HighPass2FilterNode). There have been warning messages for many months. But that should probably be in a different patch called "Remove deprecated LowPass2FilterNode and HighPass2FilterNode" > > > Source/WebCore/Modules/webaudio/BiquadProcessor.cpp:157 > > +} > > I don't think we should re-create these AudioParams -- instead just create in the normal constructor and leave named the same always (In reply to comment #2) > (From update of attachment 133835 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=133835&action=review > > Hi Chun, thanks for the patch. I don't think we should change the AudioParams in this way, but a good change would be to remove the deprecated filter nodes. > > > Source/WebCore/Modules/webaudio/BiquadProcessor.cpp:-48 > > - m_parameter3 = AudioParam::create("gain", 0.0, -40, 40); > > The attributes for BiquadFilterNode are defined in the spec as "frequency", "Q", "gain", so I think we shouldn't remove this code here, even though for some types "Q" is unused. > > > Source/WebCore/Modules/webaudio/BiquadProcessor.cpp:-76 > > - } > > I think it may be a good time to think about removing these old/deprecated AudioNodes (LowPass2FilterNode and HighPass2FilterNode). There have been warning messages for many months. But that should probably be in a different patch called "Remove deprecated LowPass2FilterNode and HighPass2FilterNode" > > > Source/WebCore/Modules/webaudio/BiquadProcessor.cpp:157 > > +} > > I don't think we should re-create these AudioParams -- instead just create in the normal constructor and leave named the same always Hi Chris, thanks for your suggestion, I'll mark this bug as invalid and fire a new one to remove LowPass2FilterNode and HighPass2FilterNode. 589207 4 133835 eric 2012-03-27 13:19:05 -0700 Comment on attachment 133835 Patch Cleared review? from attachment 133835 so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again). 133835 2012-03-26 09:45:06 -0700 2012-03-27 13:19:05 -0700 Patch bug-82200-20120327004820.patch text/plain 4650 chun.gao U3VidmVyc2lvbiBSZXZpc2lvbjogMTExNDg0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNWVkNGIxMzY5NzI5Yzc0 MjE3MzE1YmM4NWI0NWQ2ZDU3NWUwZTA0NC4uNjJmZmNlMThiMWQxZTUwZmJlYWQyYjNmM2MyMGI3 MTVlM2RjZTFlNCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDEyLTAzLTI2ICBHYW8g Q2h1biAgPGNodW4uZ2FvQGludGVsLmNvbT4KKworICAgICAgICBSZXNldCBwYXJhbXRlcnMgd2hl biBCaXF1YWRGaWx0ZXJOb2RlLnR5cGUgZ2V0IG5ldyB2YWx1ZS4KKyAgICAgICAgaHR0cHM6Ly9i dWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTgyMjAwCisKKyAgICAgICAgUmV2aWV3ZWQg YnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQ292ZXJlZCBieSBleGlzdCBsYXlvdXQgdGVz dHMuCisKKyAgICAgICAgKiBNb2R1bGVzL3dlYmF1ZGlvL0JpcXVhZFByb2Nlc3Nvci5jcHA6Cisg ICAgICAgIChXZWJDb3JlOjpCaXF1YWRQcm9jZXNzb3I6OkJpcXVhZFByb2Nlc3Nvcik6CisgICAg ICAgIChXZWJDb3JlOjpCaXF1YWRQcm9jZXNzb3I6OnNldFR5cGUpOgorICAgICAgICAoV2ViQ29y ZTo6QmlxdWFkUHJvY2Vzc29yOjpyZXNldFBhcmFtdGVycyk6CisgICAgICAgIChXZWJDb3JlKToK KyAgICAgICAgKiBNb2R1bGVzL3dlYmF1ZGlvL0JpcXVhZFByb2Nlc3Nvci5oOgorICAgICAgICAo QmlxdWFkUHJvY2Vzc29yKToKKwogMjAxMi0wMy0yMCAgVy4gSmFtZXMgTWFjTGVhbiAgPHdqbWFj bGVhbkBjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgW2Nocm9taXVtXSBDb252ZXJ0IFRvdWNoRmxp bmdQbGF0Zm9ybUdlc3R1cmVDdXJ2ZSB0byBhIDItRCBCZXppZXIgZm9yIGJldHRlciBjb250cm9s IG9mIGN1cnZlIHNoYXBlLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvTW9kdWxlcy93ZWJh dWRpby9CaXF1YWRQcm9jZXNzb3IuY3BwIGIvU291cmNlL1dlYkNvcmUvTW9kdWxlcy93ZWJhdWRp by9CaXF1YWRQcm9jZXNzb3IuY3BwCmluZGV4IGUxYWVjNzU4ODE1NjIwYzlhNmJiMDhlMGI2NzVm NTE2MTFlYmY1N2IuLmFhOWI4ZmM4MDM0YzgzOTFlNjEzOTY2MWQxMGIxN2VjMTBmNjI5NDggMTAw NjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL01vZHVsZXMvd2ViYXVkaW8vQmlxdWFkUHJvY2Vzc29y LmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9Nb2R1bGVzL3dlYmF1ZGlvL0JpcXVhZFByb2Nlc3Nv ci5jcHAKQEAgLTQ0LDggKzQ0LDcgQEAgQmlxdWFkUHJvY2Vzc29yOjpCaXF1YWRQcm9jZXNzb3Io ZmxvYXQgc2FtcGxlUmF0ZSwgc2l6ZV90IG51bWJlck9mQ2hhbm5lbHMsIGJvb2wKIAogICAgIC8v IENyZWF0ZSBwYXJhbWV0ZXJzIGZvciBCaXF1YWRGaWx0ZXJOb2RlLgogICAgIG1fcGFyYW1ldGVy MSA9IEF1ZGlvUGFyYW06OmNyZWF0ZSgiZnJlcXVlbmN5IiwgMzUwLjAsIDEwLjAsIG55cXVpc3Qp OwotICAgIG1fcGFyYW1ldGVyMiA9IEF1ZGlvUGFyYW06OmNyZWF0ZSgiUSIsIDEsIDAuMDAwMSwg MTAwMC4wKTsKLSAgICBtX3BhcmFtZXRlcjMgPSBBdWRpb1BhcmFtOjpjcmVhdGUoImdhaW4iLCAw LjAsIC00MCwgNDApOworICAgIHJlc2V0UGFyYW10ZXJzKG1fdHlwZSk7CiAKICAgICBpZiAoYXV0 b0luaXRpYWxpemUpCiAgICAgICAgIGluaXRpYWxpemUoKTsKQEAgLTYyLDE4ICs2MSw4IEBAIEJp cXVhZFByb2Nlc3Nvcjo6QmlxdWFkUHJvY2Vzc29yKEZpbHRlclR5cGUgdHlwZSwgZmxvYXQgc2Ft cGxlUmF0ZSwgc2l6ZV90IG51bWJlCiAgICAgZG91YmxlIG55cXVpc3QgPSAwLjUgKiB0aGlzLT5z YW1wbGVSYXRlKCk7CiAgICAgCiAgICAgLy8gSGFuZGxlIHRoZSBkZXByZWNhdGVkIExvd1Bhc3My RmlsdGVyTm9kZSBhbmQgSGlnaFBhc3MyRmlsdGVyTm9kZS4KLSAgICBzd2l0Y2ggKHR5cGUpIHsK LSAgICAvLyBIaWdocGFzcyBhbmQgbG93cGFzcyBzaGFyZSB0aGUgc2FtZSBwYXJhbWV0ZXJzIGFu ZCBvbmx5IGRpZmZlciBpbiBmaWx0ZXIgdHlwZS4KLSAgICBjYXNlIExvd1Bhc3M6Ci0gICAgY2Fz ZSBIaWdoUGFzczoKLSAgICAgICAgbV9wYXJhbWV0ZXIxID0gQXVkaW9QYXJhbTo6Y3JlYXRlKCJm cmVxdWVuY3kiLCAzNTAuMCwgMjAuMCwgbnlxdWlzdCk7Ci0gICAgICAgIG1fcGFyYW1ldGVyMiA9 IEF1ZGlvUGFyYW06OmNyZWF0ZSgicmVzb25hbmNlIiwgMC4wLCAtMjAuMCwgMjAuMCk7Ci0gICAg ICAgIG1fcGFyYW1ldGVyMyA9IEF1ZGlvUGFyYW06OmNyZWF0ZSgidW51c2VkIiwgMC4wLCAwLjAs IDEuMCk7Ci0gICAgICAgIGJyZWFrOwotCi0gICAgZGVmYXVsdDoKLSAgICAgICAgQVNTRVJUX05P VF9SRUFDSEVEKCk7Ci0gICAgfQorICAgIG1fcGFyYW1ldGVyMSA9IEF1ZGlvUGFyYW06OmNyZWF0 ZSgiZnJlcXVlbmN5IiwgMzUwLjAsIDIwLjAsIG55cXVpc3QpOworICAgIHJlc2V0UGFyYW10ZXJz KG1fdHlwZSk7CiAKICAgICBpZiAoYXV0b0luaXRpYWxpemUpCiAgICAgICAgIGluaXRpYWxpemUo KTsKQEAgLTEzMSwxMSArMTIwLDQyIEBAIHZvaWQgQmlxdWFkUHJvY2Vzc29yOjpwcm9jZXNzKGNv bnN0IEF1ZGlvQnVzKiBzb3VyY2UsIEF1ZGlvQnVzKiBkZXN0aW5hdGlvbiwgc2l6CiB2b2lkIEJp cXVhZFByb2Nlc3Nvcjo6c2V0VHlwZShGaWx0ZXJUeXBlIHR5cGUpCiB7CiAgICAgaWYgKHR5cGUg IT0gbV90eXBlKSB7CisgICAgICAgIHJlc2V0UGFyYW10ZXJzKHR5cGUpOwogICAgICAgICBtX3R5 cGUgPSB0eXBlOwogICAgICAgICByZXNldCgpOyAvLyBUaGUgZmlsdGVyIHN0YXRlIG11c3QgYmUg cmVzZXQgb25seSBpZiB0aGUgdHlwZSBoYXMgY2hhbmdlZC4KICAgICB9CiB9CiAKK3ZvaWQgQmlx dWFkUHJvY2Vzc29yOjpyZXNldFBhcmFtdGVycyhGaWx0ZXJUeXBlIHR5cGUpCit7CisgICAgc3dp dGNoICh0eXBlKSB7CisgICAgY2FzZSBMb3dQYXNzOgorICAgIGNhc2UgSGlnaFBhc3M6CisgICAg ICAgIG1fcGFyYW1ldGVyMiA9IEF1ZGlvUGFyYW06OmNyZWF0ZSgicmVzb25hbmNlIiwgMC4wLCAt MjAuMCwgMjAuMCk7CisgICAgICAgIG1fcGFyYW1ldGVyMyA9IEF1ZGlvUGFyYW06OmNyZWF0ZSgi dW51c2VkIiwgMC4wLCAwLjAsIDEuMCk7CisgICAgICAgIGJyZWFrOworICAgIGNhc2UgQmFuZFBh c3M6CisgICAgY2FzZSBOb3RjaDoKKyAgICAgICAgbV9wYXJhbWV0ZXIyID0gQXVkaW9QYXJhbTo6 Y3JlYXRlKCJRIiwgMSwgMC4wMDAxLCAyMC4wKTsKKyAgICAgICAgbV9wYXJhbWV0ZXIzID0gQXVk aW9QYXJhbTo6Y3JlYXRlKCJ1bnVzZWQiLCAwLjAsIDAuMCwgMS4wKTsKKyAgICAgICAgYnJlYWs7 CisgICAgY2FzZSBBbGxwYXNzOgorICAgICAgICBtX3BhcmFtZXRlcjIgPSBBdWRpb1BhcmFtOjpj cmVhdGUoIlEiLCAxLCAwLjAwMDEsIDEwMDAwLjApOworICAgICAgICBtX3BhcmFtZXRlcjMgPSBB dWRpb1BhcmFtOjpjcmVhdGUoInVudXNlZCIsIDAuMCwgMC4wLCAxLjApOworICAgIGNhc2UgTG93 U2hlbGY6CisgICAgY2FzZSBIaWdoU2hlbGY6CisgICAgICAgIG1fcGFyYW1ldGVyMiA9IEF1ZGlv UGFyYW06OmNyZWF0ZSgidW51c2VkIiwgMC4wLCAwLjAsIDEpOworICAgICAgICBtX3BhcmFtZXRl cjMgPSBBdWRpb1BhcmFtOjpjcmVhdGUoImdhaW4iLCAwLjAsIC00MCwgNDApOworICAgICAgICBi cmVhazsKKyAgICBjYXNlIFBlYWtpbmc6CisgICAgICAgIG1fcGFyYW1ldGVyMiA9IEF1ZGlvUGFy YW06OmNyZWF0ZSgiUSIsIDEsIDAuMDAwMSwgMjAuMCk7CisgICAgICAgIG1fcGFyYW1ldGVyMyA9 IEF1ZGlvUGFyYW06OmNyZWF0ZSgiZ2FpbiIsIDAuMCwgLTQwLCA0MCk7CisgICAgICAgIGJyZWFr OworICAgIGRlZmF1bHQ6CisgICAgICAgIEFTU0VSVF9OT1RfUkVBQ0hFRCgpOworICAgIH0KK30K Kwogdm9pZCBCaXF1YWRQcm9jZXNzb3I6OmdldEZyZXF1ZW5jeVJlc3BvbnNlKGludCBuRnJlcXVl bmNpZXMsCiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgY29uc3Qg ZmxvYXQqIGZyZXF1ZW5jeUh6LAogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgIGZsb2F0KiBtYWdSZXNwb25zZSwKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL01v ZHVsZXMvd2ViYXVkaW8vQmlxdWFkUHJvY2Vzc29yLmggYi9Tb3VyY2UvV2ViQ29yZS9Nb2R1bGVz L3dlYmF1ZGlvL0JpcXVhZFByb2Nlc3Nvci5oCmluZGV4IDkxYTFjY2NkMTcwNDlkMTQ4ODgxZDAw NDYzOTY3OTA3M2JjZGQ3ZDAuLmU4NjkwMTA2ZTIxZGY5N2FiZDViN2I3ZjRlN2I2MzdmMTVkNGUx ODAgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL01vZHVsZXMvd2ViYXVkaW8vQmlxdWFkUHJv Y2Vzc29yLmgKKysrIGIvU291cmNlL1dlYkNvcmUvTW9kdWxlcy93ZWJhdWRpby9CaXF1YWRQcm9j ZXNzb3IuaApAQCAtODcsNiArODcsOCBAQCBwcml2YXRlOgogCiAgICAgLy8gc28gRFNQIGtlcm5l bHMga25vdyB3aGVuIHRvIHJlLWNvbXB1dGUgY29lZmZpY2llbnRzCiAgICAgYm9vbCBtX2ZpbHRl ckNvZWZmaWNpZW50c0RpcnR5OworCisgICAgdm9pZCByZXNldFBhcmFtdGVycyhGaWx0ZXJUeXBl KTsKIH07CiAKIH0gLy8gbmFtZXNwYWNlIFdlYkNvcmUK