GNOME Bugzilla – Bug 677560
rtpjpegdepay: crash in copy_into_unchecked
Last modified: 2013-04-26 17:47:03 UTC
Segmentation fault on streaming application. GstPipelines initialized using: gst_parse_launch( "rtspsrc name=rtspsrc ! queue ! rtpjpegdepay ! ffdec_mjpeg ! ffmpegcolorspace ! xvimagesink name=videosink",NULL); gdb backtrace: (gdb) backtrace
+ Trace 230315
Gstreamer 0.10.28 packages installed under Ubuntu Lucid Lynx
Perhaps you could try with a more recent version of Ubuntu and GStreamer? (that is: core/base 0.10.36, -good 0.10.31, etc.)
I'll try newer versions as soon as possible. I have installed: - gnonlin-0.10.17 - gst-ffmpeg-0.10.11 - gst-plugins-base-0.10.36 - gst-plugins-good-0.10.31 - gstreamer-0.10.36 I have had problems configuring gst-ffmpeg-0.10.12 and 13.... Thanks in advance, Alvaro
Thanks! Since the crash seems to be in the depayloader and not in the decoder, the older version of gst-ffmpeg might be okay as well. Or you could try jpegdec instead of ffdec_mjpeg. Or just put a fakesink after the depayloader.
Than you very much Tim, I'll try your options as soon as possible.
I still get a segmentation fault with copy_into_unchecked using GStreamer 0.10.36...(packages version in a previous message 2012-06-08...). I get the following gdb backtrace:
+ Trace 230437
Components used (now without a queue): rtspsrc ! rtpjpegdepay ! ffdec_mjpeg ! ffmpegcolorspace ! xvimagesink
It also takes place with jpegdec as in: rtspsrc name=rtspsrc ! queue ! rtpjpegdepay ! jpegdec ! xvimagesink name=videosink Is it possible to do something? I attach again the backtrace:
+ Trace 230511
This changes made the trick at gstreamer-0.10.36/libs/gst/base/gstadapter.c: /* copy data into @dest, skipping @skip bytes from the head buffers */ static void copy_into_unchecked (GstAdapter * adapter, guint8 * dest, guint skip, guint size) { GSList *g; GstBuffer *buf; guint bsize, csize; /* first step, do skipping */ /* we might well be copying where we were scanning */ if (adapter->priv->scan_entry && (adapter->priv->scan_offset <= skip)) { g = adapter->priv->scan_entry; skip -= adapter->priv->scan_offset; } else { g = adapter->buflist; } buf = g->data; bsize = GST_BUFFER_SIZE (buf); while (G_UNLIKELY (skip >= bsize)) { skip -= bsize; g = g_slist_next (g); /********************/ if(g != NULL) { buf = g->data; bsize = GST_BUFFER_SIZE (buf); } else break; /********************/ } /* copy partial buffer */ csize = MIN (bsize - skip, size); memcpy (dest, GST_BUFFER_DATA (buf) + skip, csize); size -= csize; dest += csize; /* second step, copy remainder */ while (size > 0) { g = g_slist_next (g); /********************/ if(g != NULL) { buf = g->data; bsize = GST_BUFFER_SIZE (buf); if (G_LIKELY (bsize > 0)) { csize = MIN (bsize, size); memcpy (dest, GST_BUFFER_DATA (buf), csize); size -= csize; dest += csize; } } else break; /********************/ } }
Thanks for tracking this down, but could you attach a proper patch please? Preferably in git format-patch format, but git diff or diff -u will do as well. And could you describe the rationale for the change? Thanks!
And could you give us your full name please, so we can attribute the patch properly? (ideally change your bugzilla preferences)
Created attachment 223399 [details] [review] copy_into_unchecked revision to avoid segmentation faults I have added a check to the GSList *g pointer after the call to g_slist_next(g) in order to avoid access to its data property (g->data) when pointing to NULL.
Please, check the patch. I have also changed my preferences at bugzilla. Thanks again.
Thanks for the update. It's not entirely clear to me yet what's going on here exactly. At first glance it looks like it shouldn't happen that the list nodes are NULL there. Any chance you could you capture some of the jpeg RTP stream and make it available somewhere, so we can try and reproduce it? gst-launch-0.10 -e rtspsrc location=rtsp://.... ! gdppay ! filesink location=jpeg-rtp.gdp Then just press control-C after a while. (I don't know how long it takes for the crash to happen - does it crash immediately or only after hours or .. ?)
Any chance you could capture a stream like described in the previous comment, Álvaro?
Álvaro, ping
Hello, Sorry for the delay. For the moment I cannot make a capture like the one you asked for :( What I can say is that it was not a deterministic crash. It sometimes happened from time to time and sometimes I had few crashes in a while. After patching that library I have not noticed any strange effect any more so I´m afraid sometimes the pointer was NULL. As you didn´t check it, it would crash. Maybe my patch is incomplete or has an error and you should include any other code... for me, GStreamer is now working OK :)
I forgot to mention the IP cameras used in my tests (in case you have the opportunity to try them): - Axis: M3113R-M12 and M3114-R - Vivotek: MD7560 and MD8562 The crash happened with H.264 and MJPEG traffic.
Created attachment 242118 [details] [review] Check for avail >= 2 in gstrtpjpegdepay I have run into this same bug when streaming data gstreamer -> gstreamer with MJPEG encoding over RTP & UDP. To me, it seems that this bug occurs when 'avail' in gst_rtp_jpeg_depay_process is less than 2. Then skip = -1, which causes the crash in copy_into_unchecked. This can be seen in the backtrace as skip = 4294967294. I think the attached patch should be more reasonable than the one proposed earlier. These 0- or 1-byte images are obviously corrupted, but gstreamer should still be able to continue and not crash.
Instead of inserting EOI there the frame should just be dropped: commit fdb667ae00401e01fa9e014995bce541d8cec288 Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Mon Apr 22 10:19:29 2013 +0200 rtpjpegdepay: Drop frame if it's less than 2 bytes large https://bugzilla.gnome.org/show_bug.cgi?id=677560
It is marked as RESOLVED but both patches have been rejected. So...what is the solution for "gstreamer-0.10.36/libs/gst/base/gstadapter.c" ?
Do you still get this issue using the latest GStreamer 1.0 or master branches with the above patch applied? I stated earlier in comment #14 that I didn't understand the adapter patch and asked you to explain it in more detail - could you? (And the code probably looks a bit different in 1.x anyway)
Sorry I am developing under Ubuntu Lucid Lynx 10.04, so I´m afraid I cannot build GStreamer 1.x. However, as I have told before, my application is currently working with the "patch" written above. I have no longer experienced those segmentation faults, which brought me to start this thread.
Well, I am not going to apply a patch that I don't understand and which I don't think should be needed. Looking at the stack trace again, it seems quite likely to me that the above commit fixes the actual issue and is the correct way of fixing it.
I also hit the same segmentation fault and back trace. I initially thought to do the same fix as Álvaro, but as Tim mentions I don't think this check is needed (if everything went fine before). I think there's some issue with the JPEG depayloader when packets are lost or there's latency and jitter and that the depayloader is not very resilient to these facts. I end up switching to TCP for JPEG streams (I send 1 image every 4 seconds). This is the investigation that I end up with: ---------------- So here is the problematic code (see at the end). Specially: gst_adapter_copy (rtpjpegdepay->adapter, end, avail - 2, 2); which will internally call: copy_into_unchecked (adapter, dest=end, skip=4294967294, size=2); Note that skip is "2^32 - 2", which is the same as saying "0 - 2" for a guint type. My feeling is that for some reason we have an RTP packet with the marker bit (indicating this is might be the end of the image): if (gst_rtp_buffer_get_marker (buf)) { Then we get the available bytes in the adapter: avail = gst_adapter_available (rtpjpegdepay->adapter); And the problem might be here, avail could be 0 which would make the following code crash: gst_adapter_copy (rtpjpegdepay->adapter, end, avail - 2, 2); So, how is it possible that the adapter doesn't contain anything if we just did? gst_adapter_push (rtpjpegdepay->adapter, outbuf); That was the only thing I could not figure out. ============== /* take JPEG data, push in the adapter */ GST_DEBUG_OBJECT (rtpjpegdepay, "pushing data at offset %d", header_len); outbuf = gst_rtp_buffer_get_payload_subbuffer (buf, header_len, -1); gst_adapter_push (rtpjpegdepay->adapter, outbuf); outbuf = NULL; if (gst_rtp_buffer_get_marker (buf)) { guint avail; guint8 end[2]; guint8 *data; /* last buffer take all data out of the adapter */ avail = gst_adapter_available (rtpjpegdepay->adapter); GST_DEBUG_OBJECT (rtpjpegdepay, "marker set, last buffer"); /* take the last bytes of the jpeg data to see if there is an EOI * marker */ gst_adapter_copy (rtpjpegdepay->adapter, end, avail - 2, 2); ==============