Bug 44951

Summary: [GStreamer] can't play m4v videos
Product: WebKit Reporter: Philippe Normand <pnormand>
Component: MediaAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric.carlson, eric, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
URL: http://www.robwinters.co.uk/lab/html5/video/basic.htm
Attachments:
Description Flags
proposed patch eric.carlson: review+

Description Philippe Normand 2010-08-31 07:50:29 PDT
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.
Comment 1 Philippe Normand 2010-08-31 08:16:53 PDT
Created attachment 66056 [details]
proposed patch
Comment 2 Eric Carlson 2010-08-31 15:22:21 PDT
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.
Comment 3 Philippe Normand 2010-09-01 00:08:03 PDT
(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 ;)
Comment 4 Philippe Normand 2010-09-01 14:06:41 PDT
Sorry I thought you were CC'ed already. Can you reply to comment 3 please? Thanks :)
Comment 5 Eric Carlson 2010-09-01 14:31:23 PDT
> 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).
Comment 6 Philippe Normand 2010-09-02 00:08:51 PDT
Landed in http://trac.webkit.org/changeset/66646

I renamed the test to mention mpeg4 and added a test for video/mp4
Thanks!
Comment 7 WebKit Review Bot 2010-09-02 01:18:29 PDT
http://trac.webkit.org/changeset/66646 might have broken Qt Linux Release
Comment 8 Philippe Normand 2010-09-02 01:23:17 PDT
(In reply to comment #7)
> http://trac.webkit.org/changeset/66646 might have broken Qt Linux Release

See bug 45093