WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Follow-up patch to fix a failure on Big Sur
(3.57 KB, patch)
2020-12-23 11:13 PST
,
Peng Liu
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Alicia Boya García
Comment 1
2020-12-20 14:20:11 PST
Created
attachment 416588
[details]
Patch
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
<
rdar://problem/72547169
>
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.
Top of Page
Format For Printing
XML
Clone This Bug