Bug 44951 - [GStreamer] can't play m4v videos
Summary: [GStreamer] can't play m4v videos
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL: http://www.robwinters.co.uk/lab/html5...
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-31 07:50 PDT by Philippe Normand
Modified: 2010-09-02 01:23 PDT (History)
4 users (show)

See Also:


Attachments
proposed patch (4.79 KB, patch)
2010-08-31 08:16 PDT, Philippe Normand
eric.carlson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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