GNOME Bugzilla – Bug 749536
rtspsrc: handle gap in tcp mode
Last modified: 2015-08-16 13:39:35 UTC
Created attachment 303507 [details] sample pcap that show gap in sequence number In tcp mode several cameras can generate gap in sequence number for example if there is no enough bandwidth between client and server (in this scenario udp is useless, almost all packets are late, with tcp you can at least receive some seconds, than a gap and then other few seconds and so on). GStreamer should properly handle the gap in tcp mode (based on rtptime???)
See what I wrote on the mailing list about this. RTP timestamps are not sufficient here, what is needed is the capture times together with the RTP timestamps. And we don't have the capture times for TCP except for the very first packet.
Created attachment 303917 [details] sample file produced by gstreamer 1.4 I limited the bandwidth on my pc using wondershaper to 256kbps this is a sample file captured with gstreamer 1.4 and this pipeline: rtspsrc location="rtspt:...." ! rtph264depay ! h264parse ! matroskamux streamable=true ! filesink location=/tmp/test.mkv please take a look at the date/time overlay and the timestamps inside the file. gstreamer 1.4 seems to handle gap in tcp mode correctly. With git master the pipeline fails after few seconds with the error Received packet without DTS after a gap this seems cleary a regression
Created attachment 303918 [details] traffic dump while receving rtsp over tcp in low bandwidth condition
Created attachment 303920 [details] sample file produced using rtsp over udp in the same network conditions this file show that udp basically does not work in low bandwith conditions (same results with 1.4 and git master).
reverting this patch http://cgit.freedesktop.org/gstreamer/gst-plugins-good/commit/?id=87c8c163a876f45024a817e17af6dfdc52e71257 seems to restore the 1.4 behaviour
(In reply to Nicola from comment #5) > reverting this patch > > http://cgit.freedesktop.org/gstreamer/gst-plugins-good/commit/ > ?id=87c8c163a876f45024a817e17af6dfdc52e71257 > > seems to restore the 1.4 behaviour Obviously :P That's the change that introduces this error. The problem is that without this change, the calculations later will run into overflows/underflows and calculate things with GST_CLOCK_TIME_NONE. Just reverting this is not going to help much, other than hiding the problem for the cases where the wrong calculations don't hurt... and causing hard to debug problems otherwise (like completely wrong timestamps without an error anywhere). The real fix for this would be to somehow get the capture time of the RTP packet after the gap, which we currently can't for TCP (we only set a timestamp on the very first packet).
just in case someone want to reproduce the error here is a public rtsp url with bandwidth limited to 512kbps, you can try this pipeline: gst-launch-1.0 -v rtspsrc location="rtspt://service:password@93.63.189.11/rtsp_tunnel?h26x=4&line=1&inst=1" ! fakesink silent=false it fails after few seconds with "Received packet without DTS after a gap" error, additionally there is also another new critical error (gst-launch-1.0:4248): GStreamer-CRITICAL **: Trying to dispose element pipeline0, but it is in PLAYING instead of the NULL state. You need to explicitly set elements to the NULL state before dropping the final reference, to allow them to clean up. This problem may also be caused by a refcounting bug in the application or some element.
I can confirm that this stream reproduces the problem. We could maybe use the previous packet spacing to estimate a timestamp
commit 243730ced44e61997f5338007ccc3c3dae6f337a Author: Sebastian Dröge <sebastian@centricular.com> Date: Wed Jul 8 15:13:17 2015 +0300 rtpjitterbuffer: Handle seqnum gaps in TCP streams without erroring out or overflowing calculations That is, handle DTS==GST_CLOCK_TIME_NONE correctly. https://bugzilla.gnome.org/show_bug.cgi?id=749536
after this patch seems that dts and pts both none after a gap, so basically you cannot save the stream in a container ecc..
commit 4e23481d9fd09863be91cb88d1d3d33760ed8fad Author: Sebastian Dröge <sebastian@centricular.com> Date: Wed Jul 8 17:02:05 2015 +0300 rtpjitterbuffer: Calculate receive time if we don't have any This is required to properly schedule packet loss timers and make sure all our calculations work properly. https://bugzilla.gnome.org/show_bug.cgi?id=749536
it is better now, thanks! However still happen "sometime" to have buffer without timestamp /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (1923 bytes, dts: 0:00:21.328776087, pts: 0:00:21.328776087, duration: none, offset: -1, offset_end: -1, flags: 00002000 delta-unit ) 0x7f79d8064bd0 /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (2033 bytes, dts: 0:00:21.362153864, pts: 0:00:21.362153864, duration: none, offset: -1, offset_end: -1, flags: 00002000 delta-unit ) 0x7f79c40065b0 /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (3236 bytes, dts: 0:00:21.395531642, pts: 0:00:21.395531642, duration: none, offset: -1, offset_end: -1, flags: 00002000 delta-unit ) 0x7f79d8057cd0 /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (919 bytes, dts: none, pts: none, duration: none, offset: -1, offset_end: -1, flags: 00002040 discont delta-unit ) 0x7f79d8077410 /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (1047 bytes, dts: none, pts: none, duration: none, offset: -1, offset_end: -1, flags: 00002000 delta-unit ) 0x7f79d8057cd0 /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (1462 bytes, dts: none, pts: none, duration: none, offset: -1, offset_end: -1, flags: 00002000 delta-unit ) 0x7f79d80771f0 /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (2477 bytes, dts: none, pts: none, duration: none, offset: -1, offset_end: -1, flags: 00002000 delta-unit ) 0x7f79d809b120 /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (2629 bytes, dts: none, pts: none, duration: none, offset: -1, offset_end: -1, flags: 00002000 delta-unit ) 0x7f79d807d240 and after that all buffers will have no timestamps
Can you get me a debug log with GST_DEBUG=2,rtpjitterbufer:6 ?
here is the log 0:00:12.484344491 8052 0x1cc2590 WARN rtpsource rtpsource.c:1120:update_receiver_stats: unacceptable seqnum received this happen with the provided test pipeline if you keep it running for a minute or so
Here is a proper log file (there is a typo in rtpjitterbuffer log directive above) http://195.250.34.59/temp/749536.log.tar.bz2
Ok, that means that we got a discont of more than 3000 seqnums, which then causes everything to reset... not sure what to do about that one
(In reply to Sebastian Dröge (slomo) from comment #16) > Ok, that means that we got a discont of more than 3000 seqnums, which then > causes everything to reset... not sure what to do about that one maybe we can reset the timestamp to something like: last_timestamp + elapsed time from last received timestamp, what do you think about? Sending buffer without ts should be avoided, for example matroskamux will silently discard all buffers whitout timestamp, I don't think that existing apps handle this case (at least not mine)
Well, that would be one option, yes. Can you check how well that works? You could get the current clock time minus base time in gstrtpjitterbuffer.c in the "First buffer" case in the chain function if dts == NONE.
And with base time I mean gst_element_get_base_time().
Ok, I'll read the code and try to write a patch tomorrow, thanks!
Created attachment 307098 [details] [review] rtpjitterbuffer: Calculate DTS from the clock if we had none for the first packet after a reset
Seems to work here, let me know if it also works for you. This stream is borderline broken though :)
Thanks! Here is a file produced with rtspsrc ! rtph264depay ! h264parse ! matroskamux ! filesink http://195.250.34.59/temp/749536.mkv duration is more than one hour, if you play it with totem and compare the running time with the video overlay date/time is quite good!
Thanks for confirming! commit 6e7c724afadd6a4d04d6e68938d314e1be570f9e Author: Sebastian Dröge <sebastian@centricular.com> Date: Wed Jul 8 19:59:13 2015 +0300 rtpjitterbuffer: Calculate DTS from the clock if we had none for the first packet after a reset https://bugzilla.gnome.org/show_bug.cgi?id=749536
That's actually not complete yet, going to fix that now.
ok, the test stream is available again if you need it
commit ae8acc0973a3da6546c2158ef8f61e3450155e4c Author: Sebastian Dröge <sebastian@centricular.com> Date: Thu Jul 9 23:59:10 2015 +0300 rtpjitterbuffer: Always estimate DTS from the current clock time Estimating it from the RTP time will give us the PTS, so in cases of PTS!=DTS we would produce wrong DTS. As now the estimated DTS is based on the clock, don't store it in the jitterbuffer items as it would otherwise be used in the skew calculations and would influence the results. We only really need the DTS for timer calculations. https://bugzilla.gnome.org/show_bug.cgi?id=749536
the latest patch seems to cause buffers without timestamp, you can reproduce with the public stream (please keep running for some minutes) gst-launch-1.0 -v rtspsrc location="rtspt://service:password@93.63.189.11/rtsp_tunnel?h26x=4&line=1&inst=1" ! fakesink silent=false
I think this particular bug is solved. The fact that buffers don't always have timestamps is not necessarily incorrect, we might not be able to make up timestamps for all buffers. And I don't think this is a regression either, but could have happened before too? My guess is that your issue from comment #28 is basically bug #659489. In any case, it should probably be handled in a different bug.
(In reply to Tim-Philipp Müller from comment #29) > I think this particular bug is solved. > > The fact that buffers don't always have timestamps is not necessarily > incorrect, we might not be able to make up timestamps for all buffers. And I > don't think this is a regression either, but could have happened before too? > in 1.4 seems to not happen > My guess is that your issue from comment #28 is basically bug #659489. In > any case, it should probably be handled in a different bug. seems something different since it happen with the pipeline in comment 28: no h264parse in my app I use the patches from this bug but not the latest one and I have no buffers without timestamp
(In reply to Tim-Philipp Müller from comment #29) > I think this particular bug is solved. > > The fact that buffers don't always have timestamps is not necessarily > incorrect, we might not be able to make up timestamps for all buffers. And I > don't think this is a regression either, but could have happened before too? > > My guess is that your issue from comment #28 is basically bug #659489. In > any case, it should probably be handled in a different bug. To be more clear: 1) the problem does not happen using the patch in comment 21 but not the one in comment 27. 2) the problem happen applying the patch in comment 27 too 3) With the patch in comment 27 all buffers will have no timestamp after some time and not only some buffer 4) no h264parse here only rtspsrc ! fakesink
Ok, so we'll have to find yet another solution for this.
(In reply to Sebastian Dröge (slomo) from comment #32) > Ok, so we'll have to find yet another solution for this. what's wrong in reverting patch in comment 27 and keep only the one in comment 21? Is what I'm using and it seems to work fine
It's explained in the commit message.
I can confirm that your stream ends up without PTS after a few minutes
commit 68a9209408c51d822d2a28239a86d6311a4bbd22 Author: Sebastian Dröge <sebastian@centricular.com> Date: Thu Aug 13 16:32:55 2015 +0200 rtpjitterbuffer: Keep the DTS estimate if we got no DTS after a jitterbuffer reset Otherwise we will just output buffers without timestamps after a reset if no timestamps are provided by upstream, e.g. when using RTSP over TCP. https://bugzilla.gnome.org/show_bug.cgi?id=749536