Bug 123353 - [MSE] Add MediaSource compatible loading functions to MediaPlayer
Summary: [MSE] Add MediaSource compatible loading functions to MediaPlayer
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords:
Depends on:
Blocks: 123374
  Show dependency treegraph
 
Reported: 2013-10-25 11:50 PDT by Jer Noble
Modified: 2013-10-30 19:37 PDT (History)
5 users (show)

See Also:


Attachments
Patch (6.61 KB, patch)
2013-10-25 11:52 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (13.36 KB, patch)
2013-10-29 17:00 PDT, Jer Noble
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2013-10-25 11:50:22 PDT
[MSE] Add MediaSource compatable loading functions to MediaPlayer
Comment 1 Jer Noble 2013-10-25 11:52:30 PDT
Created attachment 215197 [details]
Patch
Comment 2 Eric Carlson 2013-10-29 08:17:43 PDT
Comment on attachment 215197 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=215197&action=review

> Source/WebCore/Modules/mediasource/MediaSource.cpp:207
> -    return MIMETypeRegistry::isSupportedMediaSourceMIMEType(contentType.type(), codecs);
> +    return MediaPlayer::supportsType(contentType, emptyString(), URL(), 0, true) != MediaPlayer::IsNotSupported;

Can you remove MIMETypeRegistry::isSupportedMediaSourceMIMEType now?

> Source/WebCore/platform/graphics/MediaPlayer.cpp:744
> +MediaPlayer::SupportsType MediaPlayer::supportsType(const ContentType& contentType, const String& keySystem, const URL& url, const MediaPlayerSupportsTypeClient* client, bool isMediaSource)

We will have to modify this again for MediaStream so this one bool won't last long. Why not just pass the MediaEngineSupportParameters struct?
Comment 3 Jer Noble 2013-10-29 16:24:17 PDT
(In reply to comment #2)
> (From update of attachment 215197 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=215197&action=review
> 
> > Source/WebCore/Modules/mediasource/MediaSource.cpp:207
> > -    return MIMETypeRegistry::isSupportedMediaSourceMIMEType(contentType.type(), codecs);
> > +    return MediaPlayer::supportsType(contentType, emptyString(), URL(), 0, true) != MediaPlayer::IsNotSupported;
> 
> Can you remove MIMETypeRegistry::isSupportedMediaSourceMIMEType now?

Sure can.

> > Source/WebCore/platform/graphics/MediaPlayer.cpp:744
> > +MediaPlayer::SupportsType MediaPlayer::supportsType(const ContentType& contentType, const String& keySystem, const URL& url, const MediaPlayerSupportsTypeClient* client, bool isMediaSource)
> 
> We will have to modify this again for MediaStream so this one bool won't last long. Why not just pass the MediaEngineSupportParameters struct?

Good plan.  I'll do that!
Comment 4 Jer Noble 2013-10-29 17:00:48 PDT
Created attachment 215447 [details]
Patch for landing
Comment 5 Jer Noble 2013-10-30 11:16:47 PDT
Committed r158291: <http://trac.webkit.org/changeset/158291>
Comment 6 Ryosuke Niwa 2013-10-30 11:51:01 PDT
Fixed builds in http://trac.webkit.org/changeset/158295.
Comment 7 Csaba Osztrogonác 2013-10-30 12:57:13 PDT
Comment on attachment 215447 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=215447&action=review

> Source/WebCore/dom/DOMImplementation.cpp:359
> +    MediaEngineSupportParameters parameters = { };

It broke the EFL build:
/mnt/buildbot/efl-linux-slave-2/efl-linux-64-release-wk2/build/Source/WebCore/dom/DOMImplementation.cpp: In static member function ‘static WTF::PassRefPtr<WebCore::Document> WebCore::DOMImplementation::createDocument(const WTF::String&, WebCore::Frame*, const WebCore::URL&, bool)’:
/mnt/buildbot/efl-linux-slave-2/efl-linux-64-release-wk2/build/Source/WebCore/dom/DOMImplementation.cpp:359:49: error: missing initializer for member ‘WebCore::MediaEngineSupportParameters::type’ [-Werror=missing-field-initializers]
/mnt/buildbot/efl-linux-slave-2/efl-linux-64-release-wk2/build/Source/WebCore/dom/DOMImplementation.cpp:359:49: error: missing initializer for member ‘WebCore::MediaEngineSupportParameters::codecs’ [-Werror=missing-field-initializers]
/mnt/buildbot/efl-linux-slave-2/efl-linux-64-release-wk2/build/Source/WebCore/dom/DOMImplementation.cpp:359:49: error: missing initializer for member ‘WebCore::MediaEngineSupportParameters::url’ [-Werror=missing-field-initializers]
cc1plus: all warnings being treated as errors


Could you fix it please?
Comment 8 Jer Noble 2013-10-30 13:16:19 PDT
(In reply to comment #7)
> (From update of attachment 215447 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=215447&action=review
> 
> > Source/WebCore/dom/DOMImplementation.cpp:359
> > +    MediaEngineSupportParameters parameters = { };
> 
> It broke the EFL build:
> /mnt/buildbot/efl-linux-slave-2/efl-linux-64-release-wk2/build/Source/WebCore/dom/DOMImplementation.cpp: In static member function ‘static WTF::PassRefPtr<WebCore::Document> WebCore::DOMImplementation::createDocument(const WTF::String&, WebCore::Frame*, const WebCore::URL&, bool)’:
> /mnt/buildbot/efl-linux-slave-2/efl-linux-64-release-wk2/build/Source/WebCore/dom/DOMImplementation.cpp:359:49: error: missing initializer for member ‘WebCore::MediaEngineSupportParameters::type’ [-Werror=missing-field-initializers]
> /mnt/buildbot/efl-linux-slave-2/efl-linux-64-release-wk2/build/Source/WebCore/dom/DOMImplementation.cpp:359:49: error: missing initializer for member ‘WebCore::MediaEngineSupportParameters::codecs’ [-Werror=missing-field-initializers]
> /mnt/buildbot/efl-linux-slave-2/efl-linux-64-release-wk2/build/Source/WebCore/dom/DOMImplementation.cpp:359:49: error: missing initializer for member ‘WebCore::MediaEngineSupportParameters::url’ [-Werror=missing-field-initializers]
> cc1plus: all warnings being treated as errors
> 
> 
> Could you fix it please?

Does the EFL builder not support empty initializer lists for structs?
Comment 9 Csaba Osztrogonác 2013-10-30 13:23:30 PDT
I don't know ... cc-ing the EFL bot maintainers, they can answer it.
(Otherwise EWS bots are to signal this kind of build failures.)
Comment 10 Csaba Osztrogonác 2013-10-30 13:30:36 PDT
I checked, it isn't a gcc bug:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55805
Comment 11 Jer Noble 2013-10-30 13:33:26 PDT
Okay, I'll convert this from a POD struct to one with a default constructor.
Comment 12 Jer Noble 2013-10-30 13:50:47 PDT
Committed r158311: <http://trac.webkit.org/changeset/158311>
Comment 13 Ryosuke Niwa 2013-10-30 19:37:53 PDT
This patch caused media/media-fragments/TC0054.html and media/media-fragments/TC0061.html to start crashing:
http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=media%2Fmedia-fragments%2FTC0054.html%2Cmedia%2Fmedia-fragments%2FTC0061.html