Skip to content

Commit

Permalink
Stop using findMediaFile(...) in LayoutTests
Browse files Browse the repository at this point in the history
Historically, findMediaFile() helped choose the right extension based
on what the browser could play. Chromium cannot play some proprietary
formats (h264, aac, etc), so this check would choose the .ogv format
in that case.

But things have gotten crufty.
1) A long time ago we started running layout tests on builds that *do*
   include proprietary codecs
2) But we lie about that to the tests via
     media::RemoveProprietaryMediaTypesAndCodecsForTests()
   This makes debugging tests a pain because manual execution doesn't
   call this method and its easy to forget that the test wrapper does.
3) Lots of tests never bothered to call findMediaFile anway and just
   hardcoded the use of an mp4 with proprietary codecs, which worked
   because the method in #2 above only changes canPlayType responses,
   it doesn't remove the actual support for proprietary codecs.
4) findMediaFile is about to be busted anyway because it only queries the
   file mime type without supplying codec info. eg
      canPlayType("video/mp4") == "maybe" -> lets use the mp4!
   and even chromium will now "maybe" for this now that we no longer
   consider mp4 proprietary (though codecs like h264 still are!)

So this patch does the following:

1) Deletes findMediaFile() and instead hardcode use of the ogg file. This
   maintains the existing behavior and avoids a massive rebaseline.
2) Delete media::RemoveProprietaryMediaTypesAndCodecsForTests(). Tests
   and manual runs now behave the same.
3) Delete ancient media-can-play-* LayoutTests that just call canPlayType
   with various codecs and are duplicated by content and chrome browser
   tests (which is a better place for these checks)
4) Updates mediasource-config-change-mp4-* expectations to expect that
   they should run and pass on *all* platforms (previously just android)

There are still many tests that hard code the use of mp4 files. If we
later desire to see LayoutTests run without proprietary codecs, someone
can transition those tests. No one seems to mind at the moment.

It is also not a goal to make LayoutTests try all the supported codecs.
That is covered by unit/integration tests.

Due to #2 above, a handful of tests in external/wpt/media-source now fail
because they use mp4. Fixing these failures is tracked in Issue 794338.

Change-Id: Ie357ae075c880b78d5ee9e95c1b7cc69d9d8a328
BUG: 327115,746579,787575,568704,794338
Reviewed-on: https://chromium-review.googlesource.com/807604
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Chrome Cunningham <chcunningham@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523821}
  • Loading branch information
chcunningham authored and chromium-wpt-export-bot committed Dec 13, 2017
1 parent 9e4cb25 commit 76a59f3
Showing 1 changed file with 4 additions and 2 deletions.
6 changes: 4 additions & 2 deletions media-source/mediasource-endofstream.html
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@
// Note that segmentInfo.duration is expected to also be the
// highest track buffer range end time. Therefore, endOfStream() should
// not change duration with this media.
assert_equals(threeDecimalPlaces(segmentInfo.duration), threeDecimalPlaces(mediaSource.duration));
assert_equals(threeDecimalPlaces(segmentInfo.duration), threeDecimalPlaces(mediaSource.duration),
'SegmentInfo duration should initially roughly match mediaSource duration');
assert_less_than_equal(highestEndTime, mediaSource.duration,
'Media duration may be slightly longer than intersected track buffered ranges');

Expand All @@ -64,7 +65,8 @@
assert_equals(sourceBuffer.buffered.length, 1,
'Media data properly buffered after endOfStream');

assert_equals(threeDecimalPlaces(segmentInfo.duration), threeDecimalPlaces(mediaSource.duration));
assert_equals(threeDecimalPlaces(segmentInfo.duration), threeDecimalPlaces(mediaSource.duration),
'SegmentInfo duration should still roughly match mediaSource duration');
assert_less_than_equal(highestEndTime, mediaSource.duration,
'Media duration may be slightly longer than intersected track buffered ranges');
assert_equals(sourceBuffer.buffered.end(0), mediaSource.duration,
Expand Down

0 comments on commit 76a59f3

Please sign in to comment.