Bug 44951 – [GStreamer] can't play m4v videos

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Philippe Normand]

The typefind factory for m4v is not advertised as video/x-m4v but as video/mpeg. In this case I see no other option but check the file extensions registered by the factory and if we find the m4v extension, add "video/x-m4v" to our hash table of supported media types.

[Philippe Normand]

Created attachment 66056 [details]
proposed patch

[Eric Carlson]

Comment on attachment 66056 [details]
proposed patch


> +        * media/media-can-play-mpeg-video-expected.txt: Added.
> +        * media/media-can-play-mpeg-video.html: Added.
The test name should probably change because it is testing for MPEG-4 support and "mpeg" can mean MPEG-1.


> +        <p>Test HTMLMediaElement <em>canPlayType()</em> method with
> +        multiple video mpeg MIME types.</p>
> +
This is incorrect because you you only test for "video/x-m4v", but you might as well add a test for the standard type as well.

r=me with these minor changes.

[Philippe Normand]

(In reply to comment #2)
> (From update of attachment 66056 [details])
> 
> > +        * media/media-can-play-mpeg-video-expected.txt: Added.
> > +        * media/media-can-play-mpeg-video.html: Added.
> The test name should probably change because it is testing for MPEG-4 support and "mpeg" can mean MPEG-1.
> 
> 
> > +        <p>Test HTMLMediaElement <em>canPlayType()</em> method with
> > +        multiple video mpeg MIME types.</p>
> > +
> This is incorrect because you you only test for "video/x-m4v", but you might as well add a test for the standard type as well.
> 
> r=me with these minor changes.

What if I add tests for video/mp4 and video/mpeg? Wouldn't hurt either ;)

[Philippe Normand]

Sorry I thought you were CC'ed already. Can you reply to comment 3 please? Thanks :)

[Eric Carlson]

> What if I add tests for video/mp4 and video/mpeg? Wouldn't hurt either ;)

I think adding a test for "video/mp4" is a good idea, but a test for "video/mpeg" isn't a good idea because not all ports that support MPEG-4 video support MPEG-1 video (eg. Chrome).

[Philippe Normand]

Landed in http://trac.webkit.org/changeset/66646

I renamed the test to mention mpeg4 and added a test for video/mp4
Thanks!

[WebKit Review Bot]

http://trac.webkit.org/changeset/66646 might have broken Qt Linux Release

[Philippe Normand]

(In reply to comment #7)
> http://trac.webkit.org/changeset/66646 might have broken Qt Linux Release

See bug 45093