Bug 123353 – [MSE] Add MediaSource compatible loading functions to MediaPlayer

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

[MSE] Add MediaSource compatable loading functions to MediaPlayer

[Jer Noble]

Created attachment 215197 [details]
Patch

[Eric Carlson]

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?

[Jer Noble]

(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!

[Jer Noble]

Created attachment 215447 [details]
Patch for landing

[Jer Noble]

Committed r158291: <http://trac.webkit.org/changeset/158291>

[Ryosuke Niwa]

Fixed builds in http://trac.webkit.org/changeset/158295.

[Csaba Osztrogonác]

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?

[Jer Noble]

(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?

[Csaba Osztrogonác]

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.)

[Csaba Osztrogonác]

I checked, it isn't a gcc bug:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55805

[Jer Noble]

Okay, I'll convert this from a POD struct to one with a default constructor.

[Jer Noble]

Committed r158311: <http://trac.webkit.org/changeset/158311>

[Ryosuke Niwa]

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