Bug 150836 – [GStreamer] Use RunLoop::Timer for ready state timer in MediaPlayerPrivateGStreamer

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Carlos Garcia Campos]

We don't really need a GThreadSafeMainLoopSource for this simple timer.

[Carlos Garcia Campos]

Created attachment 264690 [details]
Patch

[Philippe Normand]

Comment on attachment 264690 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:208
> +#if PLATFORM(GTK)

Why not USE(GLIB) ?

[Carlos Garcia Campos]

(In reply to comment #2)
> Comment on attachment 264690 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=264690&action=review
> 
> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:208
> > +#if PLATFORM(GTK)
> 
> Why not USE(GLIB) ?

Because EFL doesn't support it

[Philippe Normand]

(In reply to comment #3)
> (In reply to comment #2)
> > Comment on attachment 264690 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=264690&action=review
> > 
> > > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:208
> > > +#if PLATFORM(GTK)
> > 
> > Why not USE(GLIB) ?
> 
> Because EFL doesn't support it

But aren't they using Glib? If not it's quite silly because quite a few deps like GStreamer and libsoup have an implicit dependency on it already.

[Carlos Garcia Campos]

(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > Comment on attachment 264690 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=264690&action=review
> > > 
> > > > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:208
> > > > +#if PLATFORM(GTK)
> > > 
> > > Why not USE(GLIB) ?
> > 
> > Because EFL doesn't support it
> 
> But aren't they using Glib? If not it's quite silly because quite a few deps
> like GStreamer and libsoup have an implicit dependency on it already.

They don't use glib sources in the RunLoop implementation and win cairo either. See how setPriority is defined in RunLoop.h

#if USE(GLIB) && !PLATFORM(EFL)
        void setPriority(int);
#endif

EFL has its own implementation of RunLoop and RunLoop::Timer using ecore.

[Philippe Normand]

(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > (In reply to comment #2)
> > > > Comment on attachment 264690 [details]
> > > > Patch
> > > > 
> > > > View in context:
> > > > https://bugs.webkit.org/attachment.cgi?id=264690&action=review
> > > > 
> > > > > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:208
> > > > > +#if PLATFORM(GTK)
> > > > 
> > > > Why not USE(GLIB) ?
> > > 
> > > Because EFL doesn't support it
> > 
> > But aren't they using Glib? If not it's quite silly because quite a few deps
> > like GStreamer and libsoup have an implicit dependency on it already.
> 
> They don't use glib sources in the RunLoop implementation and win cairo
> either. See how setPriority is defined in RunLoop.h
> 
> #if USE(GLIB) && !PLATFORM(EFL)
>         void setPriority(int);
> #endif
> 
> EFL has its own implementation of RunLoop and RunLoop::Timer using ecore.

Alright :) So let's use the same ifdef as above for consistency then?

[Carlos Garcia Campos]

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > (In reply to comment #3)
> > > > (In reply to comment #2)
> > > > > Comment on attachment 264690 [details]
> > > > > Patch
> > > > > 
> > > > > View in context:
> > > > > https://bugs.webkit.org/attachment.cgi?id=264690&action=review
> > > > > 
> > > > > > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:208
> > > > > > +#if PLATFORM(GTK)
> > > > > 
> > > > > Why not USE(GLIB) ?
> > > > 
> > > > Because EFL doesn't support it
> > > 
> > > But aren't they using Glib? If not it's quite silly because quite a few deps
> > > like GStreamer and libsoup have an implicit dependency on it already.
> > 
> > They don't use glib sources in the RunLoop implementation and win cairo
> > either. See how setPriority is defined in RunLoop.h
> > 
> > #if USE(GLIB) && !PLATFORM(EFL)
> >         void setPriority(int);
> > #endif
> > 
> > EFL has its own implementation of RunLoop and RunLoop::Timer using ecore.
> 
> Alright :) So let's use the same ifdef as above for consistency then?

And do you want me to submit an updated patch just for this?

[Philippe Normand]

Comment on attachment 264690 [details]
Patch

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

>>>>>>> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:208
>>>>>>> +#if PLATFORM(GTK)
>>>>>> 
>>>>>> Why not USE(GLIB) ?
>>>>> 
>>>>> Because EFL doesn't support it
>>>> 
>>>> But aren't they using Glib? If not it's quite silly because quite a few deps like GStreamer and libsoup have an implicit dependency on it already.
>>> 
>>> They don't use glib sources in the RunLoop implementation and win cairo either. See how setPriority is defined in RunLoop.h
>>> 
>>> #if USE(GLIB) && !PLATFORM(EFL)
>>>         void setPriority(int);
>>> #endif
>>> 
>>> EFL has its own implementation of RunLoop and RunLoop::Timer using ecore.
>> 
>> Alright :) So let's use the same ifdef as above for consistency then?
> 
> And do you want me to submit an updated patch just for this?

No.

[Carlos Garcia Campos]

Committed r192019: <http://trac.webkit.org/changeset/192019>