Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSE: Test addSourceBuffer and changeType relaxed type strictness and allowance for implicit changeType #17416

Merged
merged 2 commits into from Jul 16, 2019

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Jun 20, 2019

In preparation for Chrome's relaxation of addSourceBuffer and
changeType codec specificity within the mime type parameter, this
change adds new new tests. It also updates the Blink test expectations
for these new tests to be failures. Later changes will add Chrome's
implementation of the relaxed strictness, and will also remove the
failure expectations.

external/wpt/media-source/mediasource-changetype-play-without-codecs-parameter:
Tests successful addSourceBuffer, changeType, and codec switching
without using any codecs parameter in types passed to addSourceBuffer
and changeType for pairs of test media that are of same media class
(audio or video) and are single track. This test is very similar to
mediasource-changetype-play.html, just with less specific parameters
to addSourceBuffer and changeType. This file is kept separate from
mediasource-changetype-play.html to help identify implementations
(like Chrome at the time of this change) that require more specific
parameters to those methods.

external/wpt/media-source/mediasource-changetype-play-implicit.html:
Tests successful codec switching without using changeType for test
media of the same bytestream format, separately for audio-only and
video-only pairs of media. Also includes a set of the same tests where
the initial addSourceBuffer uses only the (relaxed) mime type/subtype
without any codecs parameters, for any pairs that included codecs
parameters in their full types.
Note: In Chrome, only 1 pair of actually different media files is
supported across the test media: webm vp8 and vp9. We can add more
test media later if greater coverage is desired.

external/wpt/media-source/mediasource-changetype-play-negative.html:
Tests of various explicit, implicit, strict and relaxed scenarios
which should fail.

Main MSE spec issue for this:
w3c/media-source#161
Related codec-switching MSE spec issue:
w3c/media-source#155
These new tests exercise related scenarios to the new non-normative
guidance in the codec-switching incubation spec branch in WICG
(https://github.com/WICG/media-source/tree/codec-switching),
clarifying that implementations can be relaxed in addSourceBuffer
and changeType codec-specificity, but not isTypeSupported; apps
should still provide as much type specifics as they can to achieve
earlier confidence in support or lack thereof, and to avoid issues
with implementations (like Chrome at the time of this change) that
require more specific parameters to those methods.

BUG=535738

Change-Id: I20fd477b2429ef94ee70bf57ed8c18543774da93
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1663349
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672316}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already reviewed downstream.

@chromium-wpt-export-bot chromium-wpt-export-bot changed the title MSE: Relax addSourceBuffer and changeType type strictness and allow implicit changeType MSE: Test addSourceBuffer and changeType relaxed type strictness and allowance for implicit changeType Jun 25, 2019
@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-1663349 branch 5 times, most recently from 52b20d6 to 1e288ac Compare June 26, 2019 14:40
@lukebjerring
Copy link
Contributor

Unstable results

Test Subtest Results Messages
/media-source/mediasource-changetype-play-negative.html ERROR: 7/10, OK: 3/10

wolenetz and others added 2 commits July 16, 2019 15:15
…allowance for implicit changeType

In preparation for Chrome's relaxation of addSourceBuffer and
changeType codec specificity within the mime type parameter, this
change adds new new tests. It also updates the Blink test expectations
for these new tests to be failures. Later changes will add Chrome's
implementation of the relaxed strictness, and will also remove the
failure expectations.

external/wpt/media-source/mediasource-changetype-play-without-codecs-parameter:
  Tests successful addSourceBuffer, changeType, and codec switching
  without using any codecs parameter in types passed to addSourceBuffer
  and changeType for pairs of test media that are of same media class
  (audio or video) and are single track. This test is very similar to
  mediasource-changetype-play.html, just with less specific parameters
  to addSourceBuffer and changeType. This file is kept separate from
  mediasource-changetype-play.html to help identify implementations
  (like Chrome at the time of this change) that require more specific
  parameters to those methods.

external/wpt/media-source/mediasource-changetype-play-implicit.html:
  Tests successful codec switching without using changeType for test
  media of the same bytestream format, separately for audio-only and
  video-only pairs of media. Also includes a set of the same tests where
  the initial addSourceBuffer uses only the (relaxed) mime type/subtype
  without any codecs parameters, for any pairs that included codecs
  parameters in their full types.
  Note: In Chrome, only 1 pair of actually different media files is
  supported across the test media: webm vp8 and vp9. We can add more
  test media later if greater coverage is desired.

external/wpt/media-source/mediasource-changetype-play-negative.html:
  Tests of various explicit, implicit, strict and relaxed scenarios
  which should fail.

Main MSE spec issue for this:
  w3c/media-source#161
Related codec-switching MSE spec issue:
  w3c/media-source#155
These new tests exercise related scenarios to the new non-normative
  guidance in the codec-switching incubation spec branch in WICG
  (https://github.com/WICG/media-source/tree/codec-switching),
  clarifying that implementations can be relaxed in addSourceBuffer
  and changeType codec-specificity, but not isTypeSupported; apps
  should still provide as much type specifics as they can to achieve
  earlier confidence in support or lack thereof, and to avoid issues
  with implementations (like Chrome at the time of this change) that
  require more specific parameters to those methods.

BUG=535738

Change-Id: I20fd477b2429ef94ee70bf57ed8c18543774da93
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1663349
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672316}
mediasource-util.js redefines test.done() to add a final assertion
before actually calling done(). However, the assertion was not wrapped
in test.step(), which would lead to uncaught errors.
@Hexcles Hexcles force-pushed the chromium-export-cl-1663349 branch from 1e288ac to 1a67965 Compare July 16, 2019 19:22
@Hexcles
Copy link
Member

Hexcles commented Jul 16, 2019

The original Chromium commit introduced a new flaky test on Chrome, caused by an context-less assertion introduced by the util script; I pushed a commit to fix that: 1a67965 , which was reviewed by @jgraham (thanks!).

Unfortunately the change in the util script "affects" all tests in media-source and exposes some preexisting flakiness on Firefox (those tests aren't really affected by this PR).

@Hexcles Hexcles merged commit 012de5b into master Jul 16, 2019
@Hexcles Hexcles deleted the chromium-export-cl-1663349 branch July 16, 2019 20:20
@wolenetz
Copy link
Member

Thank you, @Hexcles and @jgraham.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants