Bug 220103

Summary: [GStreamer] More robust video size handling
Product: WebKit Reporter: Alicia Boya García <aboya>
Component: WebKitGTKAssignee: Alicia Boya García <aboya>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, calvaris, cgarcia, eric.carlson, ews-watchlist, glenn, gustavo, jer.noble, menard, philipj, pnormand, sergio, vjaquez
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Alicia Boya García
Reported 2020-12-22 13:50:08 PST
This patch improves the handling of how video size is detected and reported to WebKit, fixing several issues: a) The value returned by calling MediaPlayerPrivateGStreamer ::naturalSize() should not change during a given main thread tick, since this can potentially be read several times from the rendering code. This caused the elusive racy crash on `ASSERTION FAILED: !intrinsicSizeChanged || !view().frameView().layoutContext().isInRenderTreeLayout()` that has been appearing randomly in many layout tests for a very long time. b) Video rotation used to be handled via bus messages, but this is also racy, since the handling of bus messages in the main thread is run on a different priority than other callbacks. This caused a flaky failure on media/video-orientation.html. c) In MSE, appending a second initialization segment with a different video size triggered a video resize on append, before any frames with the new size has been played. This patch fixes these three issues: Only the first initialization segment will trigger a video resize (this is done so we have a video size on HAVE_METADATA, as the MSE spec expects), but otherwise video size is emitted on caps changes on the sink. In the case of regular playback this is delayed until the first frame arrives so that we have a guarantee that any rotation tag events have traversed the pipeline (the data flow through a GStreamer pipeline is done in this order: CAPS event, optional TAG events and then buffers). Video size changes are done by posting a task to the main thread, which ensures the value doesn't change during a main thread tick. The patch also relinquishes usage of MainThreadNotifier so that successions of quick video size changes (e.g. in a test case) still trigger the expected events instead of being potentially coalesced. Since this patch for the most part fixes race conditions that are not covered in TestExpectations, it doesn't introduce changes in TestExpectations. Note: This patch is not enough to fix imported/w3c/web-platform-tests/media-source/mediasource-config-change-*-framesize tests. That requires further non-trivial fixes regarding how MSE flushes are handled. Note: This patch does not fix framesize WebRTC tests. These seem to be a consequence of notifying the user too early of the frame size and/or ready state, before any frame is readable, which is a problem unrelated to these fixes.
Attachments
Patch (22.03 KB, patch)
2020-12-22 13:57 PST, Alicia Boya García
no flags
Alicia Boya García
Comment 1 2020-12-22 13:57:07 PST
EWS
Comment 2 2021-01-04 02:34:51 PST
Committed r271128: <https://trac.webkit.org/changeset/271128> All reviewed patches have been landed. Closing bug and clearing flags on attachment 416692 [details].
Note You need to log in before you can comment on or make changes to this bug.