RESOLVED FIXED 220046
media-source-webm.html: Handle frame size in HAVE_METADATA
https://bugs.webkit.org/show_bug.cgi?id=220046
Summary media-source-webm.html: Handle frame size in HAVE_METADATA
Alicia Boya García
Reported 2020-12-20 14:00:17 PST
The current version of media-source-webm.html assumes that a `resize` event happens after the first media segment is appended. This is not necessarily true. In fact, an initialization segment should cause a transition to HAVE_METADATA, and that per spec implies the size of the video is known. In practice, some implementations don't report this until a media segment has arrived. Because of the way the current code is written, an implementation emitting `resize` on HAVE_METADATA would timeout the test. This patch fixes that, accomodating both cases.
Attachments
Patch (4.77 KB, patch)
2020-12-20 14:20 PST, Alicia Boya García
no flags
Follow-up patch to fix a failure on Big Sur (3.57 KB, patch)
2020-12-23 11:13 PST, Peng Liu
no flags
Alicia Boya García
Comment 1 2020-12-20 14:20:11 PST
EWS
Comment 2 2020-12-21 04:15:45 PST
Committed r271020: <https://trac.webkit.org/changeset/271020> All reviewed patches have been landed. Closing bug and clearing flags on attachment 416588 [details].
Radar WebKit Bug Importer
Comment 3 2020-12-21 04:16:25 PST
Peng Liu
Comment 4 2020-12-22 20:57:11 PST
With this patch, the test starts to fail on Big Sur. :-(
Alicia Boya García
Comment 5 2020-12-23 04:31:43 PST
(In reply to Peng Liu from comment #4) > With this patch, the test starts to fail on Big Sur. :-( How does it fail? If it's a change in the order of the lines in the output similar to the one included in the attached patch for the GStreamer ports, that is perfectly fine, just needs a rebaseline. Otherwise, it may have exposed some other bug. I remember seeing before accomodations in tests for a slight bug where a resize event with 0x0 dimensions was emitted, before another one with the actual dimensions was emitted.
Peng Liu
Comment 6 2020-12-23 09:22:33 PST
Yes, it's a change in the order of lines. I have to revert the change in media-source-webm-expected.txt to fix the failure. Sounds like both orders are correct? I think it is a good idea to revise the test to support both of them.
Peng Liu
Comment 7 2020-12-23 11:13:04 PST
Created attachment 416719 [details] Follow-up patch to fix a failure on Big Sur
Alicia Boya García
Comment 8 2020-12-23 11:47:27 PST
(In reply to Peng Liu from comment #6) > Sounds like both orders are correct? I think it is a good idea to revise the > test to support both of them. Both orders are in practice acceptable, although the resize happening after the initialization segment without having to wait for the media segment is more correct in accordance with the spec. I personally favor a rebaseline over changing the test to keep the output the same in both cases since if your browser handles resize events at the right moment, that's a behavior that ideally you want to protect against regressions.
Peng Liu
Comment 9 2020-12-23 11:56:12 PST
(In reply to Alicia Boya García from comment #8) > > Both orders are in practice acceptable, although the resize happening after > the initialization segment without having to wait for the media segment is > more correct in accordance with the spec. > > I personally favor a rebaseline over changing the test to keep the output > the same in both cases since if your browser handles resize events at the > right moment, that's a behavior that ideally you want to protect against > regressions. Hmm, you mean using a different media-source-webm-expected.txt for Big Sur?
Alicia Boya García
Comment 10 2020-12-25 14:32:01 PST
(In reply to Peng Liu from comment #9) > Hmm, you mean using a different media-source-webm-expected.txt for Big Sur? Yes.
Peng Liu
Comment 11 2021-01-05 14:40:08 PST
(In reply to Alicia Boya García from comment #10) > (In reply to Peng Liu from comment #9) > > Hmm, you mean using a different media-source-webm-expected.txt for Big Sur? > > Yes. I had a discussion with Jer about that. While it makes sense to fire the "resize" event before the "update" event for the *first* initialization segment, the order is not important for Apple's ports. To avoid the possible test flakiness due to the WebM parser implementation, we would suggest ignoring the event order in this test. Is the order important for GTK and WPE ports? If yes, maybe we need to add one test for it?
Alicia Boya García
Comment 12 2021-01-05 14:46:47 PST
(In reply to Peng Liu from comment #11) > (In reply to Alicia Boya García from comment #10) > > (In reply to Peng Liu from comment #9) > > > Hmm, you mean using a different media-source-webm-expected.txt for Big Sur? > > > > Yes. > > I had a discussion with Jer about that. While it makes sense to fire the > "resize" event before the "update" event for the *first* initialization > segment, the order is not important for Apple's ports. To avoid the possible > test flakiness due to the WebM parser implementation, we would suggest > ignoring the event order in this test. > > Is the order important for GTK and WPE ports? If yes, maybe we need to add > one test for it? If it's necessary to avoid flakiness in a particular implementation, doing it in a "order doesn't matter" way it's okay. Certainly a separate test could be made for checking the availability of dimensions on HAVE_METADATA and mark that one as flaky or skip in the implementations that don't support that.
EWS
Comment 13 2021-01-05 16:33:42 PST
Committed r271181: <https://trac.webkit.org/changeset/271181> All reviewed patches have been landed. Closing bug and clearing flags on attachment 416719 [details].
Note You need to log in before you can comment on or make changes to this bug.