Bug 36654 – It should be possible to use GStreamer in addition to another media engine

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

[Philippe Normand]

It doesn't seem possible currently to have multiple media player private implementations because they all define the "same" MediaPlayerPrivate class.

[Philippe Normand]

I am experimenting with the GStreamer backend on the mac port and I stumbled upon this ;)

In file included from /Users/phil/gst/jhbuild/build/WebKit/WebCore/platform/graphics/MediaPlayer.cpp:46:
/Users/phil/gst/jhbuild/build/WebKit/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:52: error: redefinition of 'class WebCore::MediaPlayerPrivate'
/Users/phil/gst/jhbuild/build/WebKit/WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.h:58: error: previous definition of 'class WebCore::MediaPlayerPrivate'


So for now I had to deactivate build of the MediaPlayerPrivateQTKit. It'd be great to figure out a solution for this so we could eventually have 2 backends and let the MediaPlayer correctly decide which one to use depending on the media to play.

[Philippe Normand]

I guess I can rename MediaPlayerPrivate in the gstreamer backend to MediaPlayerPrivateGStreamer :)

Renaming would be needed in other implementations as well.

[Philippe Normand]

(In reply to comment #2)
> I guess I can rename MediaPlayerPrivate in the gstreamer backend to
> MediaPlayerPrivateGStreamer :)
> 
> Renaming would be needed in other implementations as well.

MediaPlayerPrivate:: is used in MediaPlayer::installedMediaEngines :(

Eric, what do you think we could do?

[Eric Carlson]

(In reply to comment #3)
> (In reply to comment #2)
> > I guess I can rename MediaPlayerPrivate in the gstreamer backend to
> > MediaPlayerPrivateGStreamer :)
> > 
> > Renaming would be needed in other implementations as well.
> 
> MediaPlayerPrivate:: is used in MediaPlayer::installedMediaEngines :(
> 
> Eric, what do you think we could do?

I agree that the QTKit implementation class name should change, I will do that. 

Can't you do something like this in any case?

#if PLATFORM(MAC)
        MediaPlayerPrivate::registerMediaEngine(addMediaEngine);
        MediaPlayerPrivateGStreamer::registerMediaEngine(addMediaEngine);
#else
        MediaPlayerPrivate::registerMediaEngine(addMediaEngine);
#endif

[Philippe Normand]

(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > I guess I can rename MediaPlayerPrivate in the gstreamer backend to
> > > MediaPlayerPrivateGStreamer :)
> > > 
> > > Renaming would be needed in other implementations as well.
> > 
> > MediaPlayerPrivate:: is used in MediaPlayer::installedMediaEngines :(
> > 
> > Eric, what do you think we could do?
> 
> I agree that the QTKit implementation class name should change, I will do that. 
> 

Thanks!

> Can't you do something like this in any case?
> 
> #if PLATFORM(MAC)
>         MediaPlayerPrivate::registerMediaEngine(addMediaEngine);
>         MediaPlayerPrivateGStreamer::registerMediaEngine(addMediaEngine);
> #else
>         MediaPlayerPrivate::registerMediaEngine(addMediaEngine);
> #endif

Hmm but then the gst player won't be compiled for the gtk port. Could we rather add a new feature macro and use it like

#if ENABLE(GSTREAMER)
....
#endif

?

[Philippe Normand]

(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > (In reply to comment #2)
> > > > I guess I can rename MediaPlayerPrivate in the gstreamer backend to
> > > > MediaPlayerPrivateGStreamer :)
> > > > 
> > > > Renaming would be needed in other implementations as well.
> > > 
> > > MediaPlayerPrivate:: is used in MediaPlayer::installedMediaEngines :(
> > > 
> > > Eric, what do you think we could do?
> > 
> > I agree that the QTKit implementation class name should change, I will do that. 
> > 
> 
> Thanks!
> 
> > Can't you do something like this in any case?
> > 
> > #if PLATFORM(MAC)
> >         MediaPlayerPrivate::registerMediaEngine(addMediaEngine);
> >         MediaPlayerPrivateGStreamer::registerMediaEngine(addMediaEngine);
> > #else
> >         MediaPlayerPrivate::registerMediaEngine(addMediaEngine);
> > #endif
> 
> Hmm but then the gst player won't be compiled for the gtk port. Could we rather
> add a new feature macro and use it like
> 
> #if ENABLE(GSTREAMER)
> ....
> #endif
> 

and register the QTKit player for PLATFORM(MAC) || PLATFORM(WIN) ? Maybe it'd make sense to have a macro for this feature as well.

[Eric Carlson]

(In reply to comment #5)
> (In reply to comment #4)
> > Can't you do something like this in any case?
> > 
> > #if PLATFORM(MAC)
> >         MediaPlayerPrivate::registerMediaEngine(addMediaEngine);
> >         MediaPlayerPrivateGStreamer::registerMediaEngine(addMediaEngine);
> > #else
> >         MediaPlayerPrivate::registerMediaEngine(addMediaEngine);
> > #endif
> 
> Hmm but then the gst player won't be compiled for the gtk port. Could we rather
> add a new feature macro and use it like
> 
> #if ENABLE(GSTREAMER)
> ....
> #endif
> 
This seems reasonable.


(In reply to comment #6)
> and register the QTKit player for PLATFORM(MAC) || PLATFORM(WIN) ? Maybe it'd
> make sense to have a macro for this feature as well.
> 
QTKit is Mac only.

[Eric Carlson]

Rename bug to more accurately describe its goal

[Eric Carlson]

Wrote https://bugs.webkit.org/show_bug.cgi?id=36663 for the QTKit engine class name change.

[Philippe Normand]

Created attachment 51762 [details]
renamed gstreamer player and added a WTF_USE_GSTREAMER define

[Eric Carlson]

Comment on attachment 51762 [details]
renamed gstreamer player and added a WTF_USE_GSTREAMER define

r=me

[Philippe Normand]

Landed in r56707. Thanks!