Bug 150836 - [GStreamer] Use RunLoop::Timer for ready state timer in MediaPlayerPrivateGStreamer
Summary: [GStreamer] Use RunLoop::Timer for ready state timer in MediaPlayerPrivateGSt...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2015-11-03 04:19 PST by Carlos Garcia Campos
Modified: 2015-11-04 02:14 PST (History)
2 users (show)

See Also:


Attachments
Patch (5.99 KB, patch)
2015-11-03 04:20 PST, Carlos Garcia Campos
pnormand: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2015-11-03 04:19:05 PST
We don't really need a GThreadSafeMainLoopSource for this simple timer.
Comment 1 Carlos Garcia Campos 2015-11-03 04:20:35 PST
Created attachment 264690 [details]
Patch
Comment 2 Philippe Normand 2015-11-03 04:35:10 PST
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) ?
Comment 3 Carlos Garcia Campos 2015-11-03 05:12:55 PST
(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
Comment 4 Philippe Normand 2015-11-03 06:04:33 PST
(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.
Comment 5 Carlos Garcia Campos 2015-11-03 22:28:10 PST
(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.
Comment 6 Philippe Normand 2015-11-04 00:12:42 PST
(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?
Comment 7 Carlos Garcia Campos 2015-11-04 00:15:26 PST
(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?
Comment 8 Philippe Normand 2015-11-04 00:24:38 PST
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.
Comment 9 Carlos Garcia Campos 2015-11-04 02:14:28 PST
Committed r192019: <http://trac.webkit.org/changeset/192019>