1136223 - media web-platform-tests only test ogg

Status: RESOLVED INACTIVE

(Core :: Audio/Video: Playback, defect)

Description

[Bobby Holley (:bholley)]

A bunch of the web platform tests use getVideoURI/getAudioURI in common/media.js. These functions check for mp4 and ogg support, and prefer ogg if it's available.

I think mp4 support is much more useful for us to test at this point, so if it was a choice between the two, I think we should use that. But really, we should just set the tests up to test both cases.

Comment 1

[Bobby Holley (:bholley)]

Syd, is this something you could do?

Comment 2

[Syd Polk :sydpolk]

Setting up the tests to play both types would involve refactoring every single test, as far as I can see. The tests are structured like:

var media = document.getElementById("m");
media.src = getVideoURI("/media/movie_5") + "?" + new Date() + Math.random();
media.play()

The getVideoURI explictly chooses the .ogv form of the movie. There is no way to return both movies.

We might be able to add a variable in getVideoURI that is set at runtime, and then run the tests twice...

It is trivial to have getVideoURI always play .mp4 if we want to.

I await further guidance.

Comment 3

[Philip Jägenstedt [:foolip]]

I don't think testing both sounds worthwhile, most of the tests aren't testing format per as such, they just need something that will play.

Are there tests that don't work well using Ogg, or are you hoping to shake out format-specific bugs for MPEG-4 by using it for more tests? I think it might be best to discuss this in a web-platform-tests pull request.

Comment 4

[Bobby Holley (:bholley)]

(In reply to philip from comment #3)
> I don't think testing both sounds worthwhile, most of the tests aren't
> testing format per as such, they just need something that will play.

Gecko's mp4 and ogg codepaths are quite different (the latter is newer and the older does a lot of legacy stuff), so we would gain a lot (for now) by running both. The exact cost-benefit analysis will depend on the increase in run time from running both, which someone would need to measure.

> Are there tests that don't work well using Ogg, or are you hoping to shake
> out format-specific bugs for MPEG-4 by using it for more tests?

See above.

I think ideally, the tests would allow us to specify whether we want ogg, mp4, or both in the metadata. Can you think of a simple way to do that?

> I think it
> might be best to discuss this in a web-platform-tests pull request.

That depends on whether upstream is interested in the changes we want to make. It would be nice to avoid forking things on Gecko's side. Thoughts?

Comment 5

[James Graham [:jgraham]]

(In reply to Bobby Holley (:bholley) from comment #4)

> That depends on whether upstream is interested in the changes we want to
> make. It would be nice to avoid forking things on Gecko's side. Thoughts?

Please don't fork the tests. If it comes down to it I can use this as the excuse to finally do the work needed to support query strings in test urls so that everything is run in both formats. That isn't an entirely trivial undertaking though, so it would be good to avoid it if possible.

Comment 6

[Bobby Holley (:bholley)]

(In reply to James Graham [:jgraham] from comment #5)
> (In reply to Bobby Holley (:bholley) from comment #4)
> 
> > That depends on whether upstream is interested in the changes we want to
> > make. It would be nice to avoid forking things on Gecko's side. Thoughts?
> 
> Please don't fork the tests. If it comes down to it I can use this as the
> excuse to finally do the work needed to support query strings in test urls
> so that everything is run in both formats. That isn't an entirely trivial
> undertaking though, so it would be good to avoid it if possible.

Do you have an alternative suggestion as to how to run the tests in both formats without query string support?

Comment 7

[James Graham [:jgraham]]

> Do you have an alternative suggestion as to how to run the tests in both formats without query string support?

Edit the tests? Obviously there are quite a few of them, but maybe it wouldn't actually be as bad as all that? It's hard to know without trying. I don't have a cleverer suggestion though.

If we need a cheap fix, just preferring h264 where supported seems like it would work for our short-term requirements (better testing of new code) and work for Philip / Blink if the tests are effectively format agnostic there.

Comment 8

[Bobby Holley (:bholley)]

(In reply to James Graham [:jgraham] from comment #7)
> > Do you have an alternative suggestion as to how to run the tests in both formats without query string support?
> 
> Edit the tests? Obviously there are quite a few of them, but maybe it
> wouldn't actually be as bad as all that? It's hard to know without trying. I
> don't have a cleverer suggestion though.

Do you mean editing them to run both? That would only work if upstream was interested in running both by default, which it sounds like they aren't.

The query strings stuff already involves editing the tests, IIUC. It's just a question of whether the edit forces the behavior we want (running both) or simply allows it via implementation-specific things like query strings.

> If we need a cheap fix, just preferring h264 where supported seems like it
> would work for our short-term requirements (better testing of new code) and
> work for Philip / Blink if the tests are effectively format agnostic there.

Yes, but then we lose testing of the ogg code, which is suboptimal, since our test coverage there is already poor.

Syd, can you investigate the total runtime of all of the tests that use these media files? That would presumably give us a time impact estimate of running them all twice.

Comment 9

[James Graham [:jgraham]]

(In reply to Bobby Holley (:bholley) from comment #8)
> (In reply to James Graham [:jgraham] from comment #7)
> > > Do you have an alternative suggestion as to how to run the tests in both formats without query string support?
> > 
> > Edit the tests? Obviously there are quite a few of them, but maybe it
> > wouldn't actually be as bad as all that? It's hard to know without trying. I
> > don't have a cleverer suggestion though.
> 
> Do you mean editing them to run both? That would only work if upstream was
> interested in running both by default, which it sounds like they aren't.

I mean to run both, yes. I didn't get the impression that Philip would block such a change, but I will let him speak to that.

> The query strings stuff already involves editing the tests, IIUC. It's just
> a question of whether the edit forces the behavior we want (running both) or
> simply allows it via implementation-specific things like query strings.

Well the query strings approach I was imagining wouldn't be implementation-specific. Each query would give one top-level test in the manifest and all formats would be run by default. Of course anyone who didn't want to run everything could then disable the tests for a subset of formats, so allowing more flexibility than the approach where we just run the tests in a loop with all possible formats.
 
> > If we need a cheap fix, just preferring h264 where supported seems like it
> > would work for our short-term requirements (better testing of new code) and
> > work for Philip / Blink if the tests are effectively format agnostic there.
> 
> Yes, but then we lose testing of the ogg code, which is suboptimal, since
> our test coverage there is already poor.

OK.

Comment 10

[Philip Jägenstedt [:foolip]]

I think a lot of the tests would be uglier if they were edited to run twice with two formats, but I wouldn't say I would block that, let's just keep talking until we agree :)

Probably the cleanest solution would be to append #type=mp4 or ?type=mp4 to test URLs to force a particular format. Do you generate a list of test URLs so that this would be practical, or would that only help in manual testing?

Comment 11

[James Graham [:jgraham]]

(In reply to philip from comment #10)

> Probably the cleanest solution would be to append #type=mp4 or ?type=mp4 to
> test URLs to force a particular format. Do you generate a list of test URLs
> so that this would be practical, or would that only help in manual testing?

Upstream web-platform-tests autogenerates a list of tests based on file names and file contents. There has long been a plan to add support for a single file producing multiple tests with different query strings / fragments using an override manifest, or in-file metadata, or something, but we never got there before. This sounds like it is a good excuse to do that work.

Comment 12

[Bobby Holley (:bholley)]

(In reply to James Graham [:jgraham] from comment #11)
> (In reply to philip from comment #10)
> 
> > Probably the cleanest solution would be to append #type=mp4 or ?type=mp4 to
> > test URLs to force a particular format. Do you generate a list of test URLs
> > so that this would be practical, or would that only help in manual testing?
> 
> Upstream web-platform-tests autogenerates a list of tests based on file
> names and file contents. There has long been a plan to add support for a
> single file producing multiple tests with different query strings /
> fragments using an override manifest, or in-file metadata, or something, but
> we never got there before. This sounds like it is a good excuse to do that
> work.

Agreed. Do you have a bug on file?

Comment 13

[James Graham [:jgraham]]

(In reply to Bobby Holley (:bholley) from comment #12)
> Agreed. Do you have a bug on file?

No bmo bug, but https://github.com/w3c/web-platform-tests/pull/1651 and https://github.com/w3c/wptrunner/pull/78

Comment 14

[Syd Polk :sydpolk]

The bug as written is not true for all of the web-platform tests. The tests that call getVideoURI mostly reside in html/semantics/embedded-content/media-elements. The tests we are interested in, namely in the media-source directory, do their own logic about which versions of media to load; they only load .webm, .mp4, and .mp3 files.

I am not sure if there is any need to worry about the html/semantics/embedded-content/media-elements directory or not.

Comment 15

[Bobby Holley (:bholley)]

(In reply to Syd Polk :sydpolk from comment #14)
> The bug as written is not true for all of the web-platform tests. The tests
> that call getVideoURI mostly reside in
> html/semantics/embedded-content/media-elements. The tests we are interested
> in

I am specifically interested in the correct semantics of media elements (MSE or not) as I refactor the core media code.

> I am not sure if there is any need to worry about the
> html/semantics/embedded-content/media-elements directory or not.

Per above, I think there is value in running them.

Reapplying ni from comment 8 - having a sense of the total run time of the tests we're discussing here would give us a sense of the cost of running them for multiple formats.

Comment 16

[Syd Polk :sydpolk]

The only directory that would have to be modified to run the tests twice would be html/semantics/embedded-content/media-elements. The total time to run that directory on my Mac was just under 1 minute. Do I need to add up the individual tests that call getVideoURI(), or is this enough information?

Comment 17

[Bobby Holley (:bholley)]

(In reply to Syd Polk :sydpolk from comment #16)
> The only directory that would have to be modified to run the tests twice
> would be html/semantics/embedded-content/media-elements. The total time to
> run that directory on my Mac was just under 1 minute. Do I need to add up
> the individual tests that call getVideoURI(), or is this enough information?

Nope, that's fine. That seems like a low enough upper bound that I think we should run in both configurations.

Comment 18

[Bobby Holley (:bholley)]

(Thanks for doing that, btw)

Comment 19

[Nils Ohlmeier [:drno]]

Mass closing do to inactivity.
Feel free to re-open if still needed.