Bug 26354 - [Gtk] GStreamer-CRITICAL's (and other warnings) on <video>
Summary: [Gtk] GStreamer-CRITICAL's (and other warnings) on <video>
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-06-12 13:42 PDT by Adrian Bunk
Modified: 2009-11-24 10:55 PST (History)
4 users (show)

See Also:


Attachments
warnings displayed (1.45 KB, text/plain)
2009-06-12 13:43 PDT, Adrian Bunk
no flags Details
backtrace of the first GStreamer-CRITICAL (2.45 KB, text/plain)
2009-06-12 13:44 PDT, Adrian Bunk
no flags Details
Implemented MediaPlayerPrivate::isAvailable by checking the presence of the playbin2 GStreamer element. (2.71 KB, patch)
2009-11-24 02:10 PST, Philippe Normand
gustavo: review+
Details | Formatted Diff | Diff
Implemented MediaPlayerPrivate::isAvailable by checking the presence of the playbin2 GStreamer element. (2.71 KB, patch)
2009-11-24 05:55 PST, Philippe Normand
no flags Details | Formatted Diff | Diff
Implemented MediaPlayerPrivate::isAvailable by checking the presence of the playbin2 GStreamer element. (2.66 KB, patch)
2009-11-24 05:58 PST, Philippe Normand
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adrian Bunk 2009-06-12 13:42:14 PDT
- Liferea 1.6
- WekKitGtk+ 1.1.9
- Debian unstable, only installed GStreamer packages are libgstreamer-plugins-base0.10-0 and libgstreamer0.10-0
- viewing http://macslow.net/?p=278 (contains an ogg/theora <video>)

I'll attach the complete log of the warnings and the trace of the first warning.
Comment 1 Adrian Bunk 2009-06-12 13:43:18 PDT
Created attachment 31210 [details]
warnings displayed
Comment 2 Adrian Bunk 2009-06-12 13:44:05 PDT
Created attachment 31211 [details]
backtrace of the first GStreamer-CRITICAL
Comment 3 Adrian Bunk 2009-06-12 13:54:46 PDT
Note:

I do not expect to see the video in this case, but some proper error handling.
Comment 4 Philippe Normand 2009-09-23 09:46:36 PDT
With git WebKitGtk+ and git GStreamer I don't see any warning, either when the page loads or when I play the video.
Comment 5 Adrian Bunk 2009-09-23 10:47:48 PDT
(In reply to comment #4)
> With git WebKitGtk+ and git GStreamer I don't see any warning, either when the
> page loads or when I play the video.

When you are able to play the video you are obviously outside the scope of this bug.

Please remove all GStreamer plugins except for the base ones and retry.
Comment 6 Philippe Normand 2009-09-24 01:43:47 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > With git WebKitGtk+ and git GStreamer I don't see any warning, either when the
> > page loads or when I play the video.
> 
> When you are able to play the video you are obviously outside the scope of this
> bug.
> 
> Please remove all GStreamer plugins except for the base ones and retry.

If I remove the gstreamer ogg plugin the page in MacSlow's blog still loads fine without gst warning. Can you test again please? I think this bug can be closed.
Comment 7 Adrian Bunk 2009-11-17 15:47:00 PST
(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > With git WebKitGtk+ and git GStreamer I don't see any warning, either when the
> > > page loads or when I play the video.
> > 
> > When you are able to play the video you are obviously outside the scope of this
> > bug.
> > 
> > Please remove all GStreamer plugins except for the base ones and retry.
> 
> If I remove the gstreamer ogg plugin the page in MacSlow's blog still loads
> fine without gst warning. Can you test again please? I think this bug can be
> closed.

No, it is still present with 1.1.16.

You were trying to remove the wrong plugin.

/usr/lib/gstreamer-0.10/libgstplaybin.so (just look at the trace in comment #2) is the file you have to remove for reproducing this bug.
Comment 8 Philippe Normand 2009-11-24 02:10:05 PST
Created attachment 43757 [details]
Implemented MediaPlayerPrivate::isAvailable by checking the presence of the playbin2 GStreamer element.
Comment 9 Gustavo Noronha (kov) 2009-11-24 03:52:18 PST
Comment on attachment 43757 [details]
Implemented MediaPlayerPrivate::isAvailable by checking the presence of the playbin2 GStreamer element.

Good. Do you need someone to commit this, or you have commit privileges by now?

Some nits:

 163     bool available = false;
 164     GstElementFactory* factory;
 165 
 166     do_gst_init();
 167     factory = gst_element_factory_find("playbin2");

factory should only be declared here, where it is first used. Perhaps you can also lose the available variable, and just return true inside the if (factory) check, returning false otherwise.
Comment 10 Philippe Normand 2009-11-24 04:01:07 PST
(In reply to comment #9)
> (From update of attachment 43757 [details])
> Good. Do you need someone to commit this, or you have commit privileges by now?
> 

Received the invitation today, might take some time for the paper work to be done ;)

> Some nits:
> 
>  163     bool available = false;
>  164     GstElementFactory* factory;
>  165 
>  166     do_gst_init();
>  167     factory = gst_element_factory_find("playbin2");
> 
> factory should only be declared here, where it is first used. Perhaps you can
> also lose the available variable, and just return true inside the if (factory)
> check, returning false otherwise.

I used a factory variable because the return value of gst_element_factory_find needs to be unreffed after use.

Do you want me to send a new patch?
Comment 11 Gustavo Noronha (kov) 2009-11-24 04:05:27 PST
(In reply to comment #10)

> > Some nits:
> > 
> >  163     bool available = false;
> >  164     GstElementFactory* factory;
> >  165 
> >  166     do_gst_init();
> >  167     factory = gst_element_factory_find("playbin2");
> > 
> > factory should only be declared here, where it is first used. Perhaps you can
> > also lose the available variable, and just return true inside the if (factory)
> > check, returning false otherwise.
> 
> I used a factory variable because the return value of gst_element_factory_find
> needs to be unreffed after use.

Yeah. You do need factory. What you could do to it is use GOwnPtr.

The variable I say you should lose is 'bool available'. You can just return true inside the if that checks for factory.

If you can't commit, then yeah, please upload a new patch =)
Comment 12 Philippe Normand 2009-11-24 05:55:28 PST
Created attachment 43763 [details]
Implemented MediaPlayerPrivate::isAvailable by checking the presence of the playbin2 GStreamer element.
Comment 13 Philippe Normand 2009-11-24 05:58:33 PST
Created attachment 43764 [details]
Implemented MediaPlayerPrivate::isAvailable by checking the presence of the playbin2 GStreamer element.
Comment 14 Gustavo Noronha (kov) 2009-11-24 10:41:41 PST
Comment on attachment 43764 [details]
Implemented MediaPlayerPrivate::isAvailable by checking the presence of the playbin2 GStreamer element.

Thanks.
Comment 15 WebKit Commit Bot 2009-11-24 10:55:21 PST
Comment on attachment 43764 [details]
Implemented MediaPlayerPrivate::isAvailable by checking the presence of the playbin2 GStreamer element.

Clearing flags on attachment: 43764

Committed r51343: <http://trac.webkit.org/changeset/51343>
Comment 16 WebKit Commit Bot 2009-11-24 10:55:26 PST
All reviewed patches have been landed.  Closing bug.