Bug 264488 – Use SIMD in HasConstantValue() in BiquadDSPKernel

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Audio

[Ho Cheung]

Use SIMD to optimize this. This would speed up processing by a factor of 4 because we can process 4 floats at a time.

[Chris Dumez]

Pull request: https://github.com/WebKit/WebKit/pull/20233

[Ho Cheung]

(In reply to Chris Dumez from comment #1)
> Pull request: https://github.com/WebKit/WebKit/pull/20233

Thank you for taking my patch from Blink. I originally wanted to implement it in Webkit myself, but you have done it for me, so I won't submit a new PR on this issue for the time being lol

[Chris Dumez]

(In reply to Ho Cheung from comment #2)
> (In reply to Chris Dumez from comment #1)
> > Pull request: https://github.com/WebKit/WebKit/pull/20233
> 
> Thank you for taking my patch from Blink. I originally wanted to implement
> it in Webkit myself, but you have done it for me, so I won't submit a new PR
> on this issue for the time being lol

Sorry, I realized too late you were the patch author on Blink side. It wasn't clear when you filed the bug that you intended on actually making the code change yourself.

If you end up making more contributions to WebKit (and I hope you do), I encourage you to make it clearer in the bug description that a patch is incoming.

[Ho Cheung]

(In reply to Chris Dumez from comment #3)
> (In reply to Ho Cheung from comment #2)
> > (In reply to Chris Dumez from comment #1)
> > > Pull request: https://github.com/WebKit/WebKit/pull/20233
> > 
> > Thank you for taking my patch from Blink. I originally wanted to implement
> > it in Webkit myself, but you have done it for me, so I won't submit a new PR
> > on this issue for the time being lol
> 
> Sorry, I realized too late you were the patch author on Blink side. It
> wasn't clear when you filed the bug that you intended on actually making the
> code change yourself.
> 
> If you end up making more contributions to WebKit (and I hope you do), I
> encourage you to make it clearer in the bug description that a patch is
> incoming.

This is my first time submitting a bug report to WebKit. Thank you for your suggestions and help.

Another question I would like to ask is that on the Blink side, I wrote a unit test for this modification. Does this also apply to WebKit?

https://chromium-review.googlesource.com/c/chromium/src/+/4622707

[Chris Dumez]

(In reply to Ho Cheung from comment #4)
> (In reply to Chris Dumez from comment #3)
> > (In reply to Ho Cheung from comment #2)
> > > (In reply to Chris Dumez from comment #1)
> > > > Pull request: https://github.com/WebKit/WebKit/pull/20233
> > > 
> > > Thank you for taking my patch from Blink. I originally wanted to implement
> > > it in Webkit myself, but you have done it for me, so I won't submit a new PR
> > > on this issue for the time being lol
> > 
> > Sorry, I realized too late you were the patch author on Blink side. It
> > wasn't clear when you filed the bug that you intended on actually making the
> > code change yourself.
> > 
> > If you end up making more contributions to WebKit (and I hope you do), I
> > encourage you to make it clearer in the bug description that a patch is
> > incoming.
> 
> This is my first time submitting a bug report to WebKit. Thank you for your
> suggestions and help.
> 
> Another question I would like to ask is that on the Blink side, I wrote a
> unit test for this modification. Does this also apply to WebKit?
> 
> https://chromium-review.googlesource.com/c/chromium/src/+/4622707

Oh, I missed that. In general, yes but API tests are located in a different place in WebKit:
- Tools/TestWebKitAPI/Tests/

or Tools/TestWebKitAPI/Tests/WebCore in this case.

I'll look into updating my PR to include your test soon.

[EWS]

Committed 270669@main (0a313d9769ba): <https://commits.webkit.org/270669@main>

Reviewed commits have been landed. Closing PR #20233 and removing active labels.

[Radar WebKit Bug Importer]

<rdar://problem/118354762>