Once the new webm demuxer and VPx decoder lands, we'll be able to re-enable those tests.

Comment on attachment 8633241 [details] [diff] [review] P1. Enable new MSE based on MediaFormatReader architecture. wrong bug#

I believe 1091774 will be improved by 1173657. I also don't think it applies with the new MSE as we extract all frames at once.

- don't enable test_SetModeThrows.html since it no longer throws, is this test obsolete with the new MSE? - test_BufferingWait.html currently fails since waiting is triggered at 0.66 instead of 0.7 Buffered ranges are up to 0.801000 848652032[7f6a26986380]: TrackBuffersManager(7f6a269be000:video/webm)::UpdateBufferedRanges: after video ranges=[(0.000000, 0.801000)] And it looks like more frames are decoded: TrackBuffersManager(7f6a269be000:video/webm)::ProcessFrames: Processing video/webm; codecs=vp8 frame(pts:767000 end:801000, dts:767000, duration:34000, kf:0) Decoder=7f6a2682b8b0 playing video frame 733000 (id=12) (queued=21, state-machine=10, decoder-queued=11) This looks like its an issue elsewhere in the MSE stack.

Comment on attachment 8642431 [details] [diff] [review] Bug-1182946-Re-enable-webm-mediasource-tests.patch Review of attachment 8642431 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/profiles/prefs_general.js @@ +165,4 @@ > > // Enable Media Source Extensions for testing > user_pref("media.mediasource.mp4.enabled", true); > +user_pref("media.mediasource.webm.enabled", true); We won't enable webm in mediasource at this stage. It's been decided to be linux only to start with. @@ +165,5 @@ > > // Enable Media Source Extensions for testing > user_pref("media.mediasource.mp4.enabled", true); > +user_pref("media.mediasource.webm.enabled", true); > +user_pref("media.mediasource.format-reader.webm", true); this is out of scope

(In reply to Jean-Yves Avenard [:jya] from comment #8) > Comment on attachment 8642431 [details] [diff] [review] > Bug-1182946-Re-enable-webm-mediasource-tests.patch > > Review of attachment 8642431 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: testing/profiles/prefs_general.js > @@ +165,4 @@ > > > > // Enable Media Source Extensions for testing > > user_pref("media.mediasource.mp4.enabled", true); > > +user_pref("media.mediasource.webm.enabled", true); > > We won't enable webm in mediasource at this stage. > > It's been decided to be linux only to start with. Ok only enabling mediasource for tests on linux for now. > @@ +165,5 @@ > > > > // Enable Media Source Extensions for testing > > user_pref("media.mediasource.mp4.enabled", true); > > +user_pref("media.mediasource.webm.enabled", true); > > +user_pref("media.mediasource.format-reader.webm", true); > > this is out of scope You want to run the webm mediasource tests with WebMReader? On the other hand might be best to get rid of media.mediasource.format-reader.webm and always use MediaFormatReader for MSE (filed Bug 1190748)

(In reply to Jan Gerber from comment #9) > You want to run the webm mediasource tests with WebMReader? On the other > hand might be best to get rid of media.mediasource.format-reader.webm and > always use MediaFormatReader for MSE (filed Bug 1190748) no. but this bug is about enabling webm with the new MSE . Not enabling webm with the old MSE which we'll have no one to properly test

Comment on attachment 8642930 [details] [diff] [review] Bug-1182946-Re-enable-webm-mediasource-tests-on-Linux.patch Review of attachment 8642930 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/profiles/prefs_general.js @@ +166,5 @@ > // Enable Media Source Extensions for testing > user_pref("media.mediasource.mp4.enabled", true); > +#if defined(XP_LINUX) > +user_pref("media.mediasource.webm.enabled", true); > +#else oh sorry my bad ; I misread, I thought you were enabling MSE for non-testing. so yes, always enabling webm here is fine.

(In reply to Jean-Yves Avenard [:jya] from comment #10) > (In reply to Jan Gerber from comment #9) > > You want to run the webm mediasource tests with WebMReader? On the other > > hand might be best to get rid of media.mediasource.format-reader.webm and > > always use MediaFormatReader for MSE (filed Bug 1190748) > > no. > > but this bug is about enabling webm with the new MSE . > Not enabling webm with the old MSE which we'll have no one to properly test Sorry my bad, was under the impression media.mediasource.format-reader.webm is needed to test the new MSE. Looking at it again, this was only used to use MediaFormatReader with the old MSE stack.

- media.mediasource.webm.enabled is enabled for all tests and tests only run on linux. - test_BufferingWait.html is disabled for now, created Bug 1190776 to enable it.

- run tests on all platforms. - test_BufferingWait.html is disabled for now, created Bug 1190776 to enable it.

Comment on attachment 8642964 [details] [diff] [review] Bug-1182946-Re-enable-webm-mediasource-tests.patch Review of attachment 8642964 [details] [diff] [review]: ----------------------------------------------------------------- let see how that go.. If you checked the original code, some were excluded on some platforms. let see how it goes via a try run ::: dom/media/mediasource/test/mochitest.ini @@ +79,4 @@ > [test_SeekTwice_mp4.html] > skip-if = ((os == "win" && os_version == "5.1") || (os != "win" && os != "mac")) # Only supported on osx and vista+ > [test_SetModeThrows.html] > +skip-if = true if the test isn't valid ; then it should be removed completely alltogether

Looking at the results from https://treeherder.mozilla.org/#/jobs?repo=try&revision=85f1ef18fcc1 All media-source web-platform-tests need to be updated too. Here comes a new patch including web-platform-tests.

Comment on attachment 8644420 [details] [diff] [review] Bug-1182946-Re-enable-webm-mediasource-tests.patch Review of attachment 8644420 [details] [diff] [review]: ----------------------------------------------------------------- The mochitest needs to be split from the webref changes. Have karlt review the webref changes. This looks very good for the webref though. Unfortunate however, that webm working now hides h264 failures. ::: testing/web-platform/meta/media-source/mediasource-is-type-supported.html.ini @@ +1,4 @@ > [mediasource-is-type-supported.html] > type: testharness > prefs: [media.mediasource.enabled:true] > + [Test invalid MIME format "video/webm"] # Bug 1191833 did you test that those changes work? you can't have comments there unfortunately. Only with "disabled:" ::: testing/web-platform/meta/media-source/mediasource-seek-during-pending-seek.html.ini @@ +4,5 @@ > disabled: > if os == "mac": https://bugzilla.mozilla.org/show_bug.cgi?id=1183523 > if (os == "win") and (version != "5.1.2600"): https://bugzilla.mozilla.org/show_bug.cgi?id=1183523 > + if (os == "linux"): https://bugzilla.mozilla.org/show_bug.cgi?id=1183523 > + if (os == "win") and (version == "5.1.2600"): https://bugzilla.mozilla.org/show_bug.cgi?id=1183523 so that means it's now to be disabled on all platforms ; don't need to have all platforms listed ; though I guess it doesn't really matter

Comment on attachment 8644420 [details] [diff] [review] Bug-1182946-Re-enable-webm-mediasource-tests.patch > [test_SetModeThrows.html] >-skip-if = true # bug 1182946 >+skip-if = true Link to a bug tracking this issue. >+++ b/testing/web-platform/meta/media-source/mediasource-seek-during-pending-seek.html.ini >@@ -4,13 +4,5 @@ > disabled: > if os == "mac": https://bugzilla.mozilla.org/show_bug.cgi?id=1183523 > if (os == "win") and (version != "5.1.2600"): https://bugzilla.mozilla.org/show_bug.cgi?id=1183523 >+ if (os == "linux"): https://bugzilla.mozilla.org/show_bug.cgi?id=1183523 >+ if (os == "win") and (version == "5.1.2600"): https://bugzilla.mozilla.org/show_bug.cgi?id=1183523 These look like they can be unified. I suspect there is no platform where it is not disabled. (In reply to Jean-Yves Avenard [:jya] from comment #17) > The mochitest needs to be split from the webref changes. I don't think that is possible because prefs_general.js affects both web platform and mochitest.

(In reply to Karl Tomlinson (ni?:karlt) from comment #18) > Comment on attachment 8644420 [details] [diff] [review] > Bug-1182946-Re-enable-webm-mediasource-tests.patch > > > [test_SetModeThrows.html] > >-skip-if = true # bug 1182946 > >+skip-if = true > > Link to a bug tracking this issue. This test was actually outdated, testing now that it does not throw. > >+++ b/testing/web-platform/meta/media-source/mediasource-seek-during-pending-seek.html.ini > >@@ -4,13 +4,5 @@ > > disabled: > > if os == "mac": https://bugzilla.mozilla.org/show_bug.cgi?id=1183523 > > if (os == "win") and (version != "5.1.2600"): https://bugzilla.mozilla.org/show_bug.cgi?id=1183523 > > >+ if (os == "linux"): https://bugzilla.mozilla.org/show_bug.cgi?id=1183523 > >+ if (os == "win") and (version == "5.1.2600"): https://bugzilla.mozilla.org/show_bug.cgi?id=1183523 > > These look like they can be unified. > I suspect there is no platform where it is not disabled. true, disabling on all platforms

upload new patch...

Comment on attachment 8644858 [details] [diff] [review] Bug-1182946-Re-enable-webm-mediasource-tests.patch Review of attachment 8644858 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/mediasource/test/test_SetModeThrows.html @@ +21,5 @@ > ok("true", "Setting to segments does not throw"); > try { > sb.mode = "sequence"; > + ok("true", "Setting to segments does not throw"); > + } catch (e) { ok(/supported/.test(e), "Should not throw setting mode to sequence: " + e); } that line doesn't sound right... how would that fail if it did throw?

(In reply to Jean-Yves Avenard [:jya] from comment #21) > ::: dom/media/mediasource/test/test_SetModeThrows.html > @@ +21,5 @@ > > ok("true", "Setting to segments does not throw"); > > try { > > sb.mode = "sequence"; > > + ok("true", "Setting to segments does not throw"); > > + } catch (e) { ok(/supported/.test(e), "Should not throw setting mode to sequence: " + e); } > > that line doesn't sound right... > > how would that fail if it did throw? lets try this again, now with ok(false, "Should not throw setting mode to sequence: " + e);

sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=12625937&repo=mozilla-inbound

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/eb58591dc190

This hit OSX as well: https://treeherder.mozilla.org/logviewer.html#?job_id=12628019&repo=mozilla-inbound

looks like those tests are a bit racy. in any case the reason seams to be that the first chunk attached (up to the end of the first media segment) was [0, 25223] and not [0, 25523]. The first media segment ends at 25523 though. Updating all tests that used 25223 instead of 25523.

the patch updates all tests to use 25523 instead of 25223 of cause.

(In reply to Jan Gerber from comment #28) > Created attachment 8645035 [details] [diff] [review] > Bug-1182946-end-of-first-media-segment-is-25523.patch > > looks like those tests are a bit racy. in any case the reason seams to be > that the first chunk attached (up to the end of the first media segment) was > [0, 25223] and not [0, 25523]. The first media segment ends at 25523 though. > Updating all tests that used 25223 instead of 25523. Why would that be racy? Partial media segment should be perfectly acceptable ; and I think should probably safe to miss just 300 bytes of it. Couldn't this indicate an issue with the webm demuxer not properly handling partial media segment? The old MSE never had issues with those tests. try looks good. But I think we should investigate

https://hg.mozilla.org/integration/mozilla-inbound/rev/997748793428 https://hg.mozilla.org/integration/mozilla-inbound/rev/fcf7077de8fe

https://hg.mozilla.org/mozilla-central/rev/997748793428 https://hg.mozilla.org/mozilla-central/rev/fcf7077de8fe

Note that bug comments in ini files are removed on updates: https://hg.mozilla.org/integration/mozilla-inbound/diff/dcdf6e96e9a0/testing/web-platform/meta/media-source/mediasource-is-type-supported.html.ini You can use [test name] bug: ... which will stay around.

(In reply to Jean-Yves Avenard [:jya] from comment #31) > (In reply to Jan Gerber from comment #28) > > Created attachment 8645035 [details] [diff] [review] > > Bug-1182946-end-of-first-media-segment-is-25523.patch > > > > looks like those tests are a bit racy. in any case the reason seams to be > > that the first chunk attached (up to the end of the first media segment) was > > [0, 25223] and not [0, 25523]. The first media segment ends at 25523 though. > > Updating all tests that used 25223 instead of 25523. > > Why would that be racy? > > Partial media segment should be perfectly acceptable ; and I think should > probably safe to miss just 300 bytes of it. > > Couldn't this indicate an issue with the webm demuxer not properly handling > partial media segment? > > The old MSE never had issues with those tests. > > try looks good. > > But I think we should investigate Looks indeed like there is a problem with partial segments. Patch and some more details in Bug 1192517

Is there a reason why test_EndOfStream.html didn't get re-enabled? https://hg.mozilla.org/mozilla-central/rev/7bf6ab248f72#l1.20

(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #36) > Is there a reason why test_EndOfStream.html didn't get re-enabled? > https://hg.mozilla.org/mozilla-central/rev/7bf6ab248f72#l1.20 it is only disabled on android. though I see another that should have been re-enabled will open a new bug for it