[webkit-reviews] review granted: [Bug 230623] MediaStream canvas.captureStream() playback to LocalSampleBufferDisplayLayer is flaky : [Attachment 449663] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Jan 24 02:24:02 PST 2022
Kimmo Kinnunen <kkinnunen at apple.com> has granted review:
Bug 230623: MediaStream canvas.captureStream() playback to
LocalSampleBufferDisplayLayer is flaky
https://bugs.webkit.org/show_bug.cgi?id=230623
Attachment 449663: Patch
https://bugs.webkit.org/attachment.cgi?id=449663&action=review
--- Comment #8 from Kimmo Kinnunen <kkinnunen at apple.com> ---
Comment on attachment 449663
--> https://bugs.webkit.org/attachment.cgi?id=449663
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=449663&action=review
It is a step forward for the users, which is good.
The reenqueue logic is broken in many ways and not very good pattern for
correct software and the class has numerous race conditions. We should spend
some more time writing tests and fixing those for this class, at some point.
>
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStrea
mAVFObjC.mm:284
> + Locker locker { AdoptLock, m_currentVideoSampleLock };
This is like saying "Try to send the sample. if it's a bit inconvenient, maybe
you can just skip sending it."
Either the sample is needed for correct operation and send it or it's not
needed and it should not be sent.
This especially does not make sense in this "reenqueue" phase.
So you can consider using unconditional locking.
More information about the webkit-reviews
mailing list