After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 723554 - request for "stats" property of "payX" pipeline element may raise GLib warning
request for "stats" property of "payX" pipeline element may raise GLib warning
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-rtsp-server
git master
Other Linux
: Normal normal
: 1.2.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-02-03 19:56 UTC by Andrey Utkin
Modified: 2014-02-25 22:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch fixing pointers handling (1.32 KB, patch)
2014-02-03 20:43 UTC, Andrey Utkin
none Details | Review
another patch (821 bytes, patch)
2014-02-04 21:57 UTC, Andrey Utkin
none Details | Review

Description Andrey Utkin 2014-02-03 19:56:17 UTC
In gst_rtsp_stream_get_rtpinfo () at rtsp-stream.c:1890 there is a request g_object_get (..., "stats", ...).
The object on which this request is done is a GstElement in a pipeline given to gst_rtsp_media_factory_set_launch (), named payX and intended for pulling data from it to send to RTSP client. But there's no requirement declared that payX must be derived from GstRtpBasePayload class, which implements "stats" property. For example, in my application a pipeline provided to gst_rtsp_media_factory_set_launch () looks like this

( appsrc name=%s ! capsfilter name=capsfilter_%s ! identity name=pay%d )

When it is constructed, i save references to appsrc and capsfilter objects to be able to push data there and set actual caps. And this request for "stats" property makes app stop when G_DEBUG=all is set, which i use for debug purposes, with such message:

GLib-GObject-WARNING **: g_object_get_valist: object class 'GstIdentity' has no property named 'stats'

So could this be somehow enhanced so that property is not requested when it is not present? Or to make it so that ...get_rtpinfo () is not executed at all?
Comment 1 Andrey Utkin 2014-02-03 20:05:39 UTC
And when G_DEBUG is unset, this may result in crash:

GLib-GObject-WARNING **: g_object_get_valist: object class 'GstIdentity' has no property named 'stats'
*** glibc detected *** appname: free(): invalid pointer: 0x0000000000ad4a50 ***

The reason is probably there:

GstStructure *stats;
...
g_object_get (priv->payloader, "stats", &stats, NULL);
if (stats == NULL)
  goto no_stats;

stats is not initialized to NULL, but checked against it. I guess g_object_set () does not modify/init it in any way, thus we check uninitialized value against NULL, which doesn't work.
Comment 2 Andrey Utkin 2014-02-03 20:39:21 UTC
When stats = NULL init added, there are still problems: rtpinfo string is subject to g_string_free() being NULL, which causes critical GLib warning.


  • #7 g_logv
    at /var/tmp/portage/dev-libs/glib-2.38.2/work/glib-2.38.2/glib/gmessages.c line 989
  • #8 g_log
    at /var/tmp/portage/dev-libs/glib-2.38.2/work/glib-2.38.2/glib/gmessages.c line 1025
  • #9 g_return_if_fail_warning
    at /var/tmp/portage/dev-libs/glib-2.38.2/work/glib-2.38.2/glib/gmessages.c line 1034
  • #10 g_string_free
    at /var/tmp/portage/dev-libs/glib-2.38.2/work/glib-2.38.2/glib/gstring.c line 219
  • #11 gst_rtsp_session_media_get_rtpinfo
    at rtsp-session-media.c line 340
  • #12 handle_play_request
    at rtsp-client.c line 1125
  • #13 handle_request
    at rtsp-client.c line 2006
  • #14 gst_rtsp_client_handle_message
    at rtsp-client.c line 2566
  • #15 message_received
    at rtsp-client.c line 2619
  • #16 gst_rtsp_source_dispatch_read
    at gstrtspconnection.c line 2972

Comment 3 Andrey Utkin 2014-02-03 20:43:43 UTC
Created attachment 267999 [details] [review]
patch fixing pointers handling

fixes invalid pointer access, but not a warning about non-existing property.
Comment 4 Wim Taymans 2014-02-04 09:15:50 UTC
commit 71c45fce5a50146ca799215874095679708872d3
Author: Wim Taymans <wtaymans@redhat.com>
Date:   Tue Feb 4 10:14:45 2014 +0100

    stream: add fallback for missing stats property
    
    Use a fallback when the payloader does not have a stats property
    
    Fixes https://bugzilla.gnome.org/show_bug.cgi?id=723554
Comment 5 Andrey Utkin 2014-02-04 21:57:46 UTC
Created attachment 268104 [details] [review]
another patch
Comment 6 Andrey Utkin 2014-02-04 21:59:25 UTC
Please apply the above patch, because a part of problem still wasn't fixed.

  • #6 <signal handler called>
  • #7 g_logv
    at /var/tmp/portage/dev-libs/glib-2.38.2/work/glib-2.38.2/glib/gmessages.c line 989
  • #8 g_log
    at /var/tmp/portage/dev-libs/glib-2.38.2/work/glib-2.38.2/glib/gmessages.c line 1025
  • #9 g_return_if_fail_warning
    at /var/tmp/portage/dev-libs/glib-2.38.2/work/glib-2.38.2/glib/gmessages.c line 1034
  • #10 g_string_free
    at /var/tmp/portage/dev-libs/glib-2.38.2/work/glib-2.38.2/glib/gstring.c line 219
  • #11 gst_rtsp_session_media_get_rtpinfo
    at rtsp-session-media.c line 340
  • #12 handle_play_request
    at rtsp-client.c line 1125
  • #13 handle_request
    at rtsp-client.c line 2006
  • #14 gst_rtsp_client_handle_message
    at rtsp-client.c line 2566
  • #15 message_received
    at rtsp-client.c line 2619
  • #16 gst_rtsp_source_dispatch_read
    at gstrtspconnection.c line 2972

Comment 7 Wim Taymans 2014-02-07 15:25:23 UTC
commit 271f5330985fbf4b4e41028a43cc00788a0fcfff
Author: Andrey Utkin <andrey.krieger.utkin@gmail.com>
Date:   Mon Feb 3 22:41:48 2014 +0200

    Don't free rtpinfo GString when it is NULL
    
    Fixes https://bugzilla.gnome.org/show_bug.cgi?id=723554