Bug 36654

Summary: It should be possible to use GStreamer in addition to another media engine
Product: WebKit Reporter: Philippe Normand <pnormand>
Component: MediaAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: eric.carlson, jer.noble
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
renamed gstreamer player and added a WTF_USE_GSTREAMER define eric.carlson: review+

Description Philippe Normand 2010-03-26 04:23:57 PDT
It doesn't seem possible currently to have multiple media player private implementations because they all define the "same" MediaPlayerPrivate class.
Comment 1 Philippe Normand 2010-03-26 04:26:28 PDT
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.
Comment 2 Philippe Normand 2010-03-26 05:59:39 PDT
I guess I can rename MediaPlayerPrivate in the gstreamer backend to MediaPlayerPrivateGStreamer :)

Renaming would be needed in other implementations as well.
Comment 3 Philippe Normand 2010-03-26 06:09:55 PDT
(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?
Comment 4 Eric Carlson 2010-03-26 08:08:50 PDT
(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
Comment 5 Philippe Normand 2010-03-26 08:14:55 PDT
(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

?
Comment 6 Philippe Normand 2010-03-26 08:19:28 PDT
(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.
Comment 7 Eric Carlson 2010-03-26 08:23:28 PDT
(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.
Comment 8 Eric Carlson 2010-03-26 08:31:04 PDT
Rename bug to more accurately describe its goal
Comment 9 Eric Carlson 2010-03-26 08:36:21 PDT
Wrote https://bugs.webkit.org/show_bug.cgi?id=36663 for the QTKit engine class name change.
Comment 10 Philippe Normand 2010-03-26 10:59:00 PDT
Created attachment 51762 [details]
renamed gstreamer player and added a WTF_USE_GSTREAMER define
Comment 11 Eric Carlson 2010-03-26 11:22:21 PDT
Comment on attachment 51762 [details]
renamed gstreamer player and added a WTF_USE_GSTREAMER define

r=me
Comment 12 Philippe Normand 2010-03-29 00:31:08 PDT
Landed in r56707. Thanks!