GNOME Bugzilla – Bug 784094
gst-rtsp-server: huge delay for the client to connect sometimes
Last modified: 2017-07-25 09:17:54 UTC
Aproximatively 1 time over 100, the client can take up to 1 min to connect. I found this is a regression introduced by a old commit https://cgit.freedesktop.org/gstreamer/gst-rtsp-server/commit/?id=852cc09f542af5cadd79ffd7fe79d6475cf57e14 I noticed that the seqnum retrieved from the udpsink can be totally wrong, for example it will be 40000 whereas the seqnum from the payloader is 100. Most of the time these 2 values are very close, i.e. the diff is around 10 or 20 at maximum. I suspect that the last-sample is just obsolete, from a previous run or something, just a guess. I am not sure what would be the best way to check the validity of this last-sample, I tought the gstbasesink would clear it when restarting or something like that.
Created attachment 354251 [details] [review] WIP rtsp-stream: use payloader if last-sample is obsolete This is a temp workaround useful to show the problem. No delay with it.
Created attachment 354569 [details] [review] rtsp-stream: fix connection delay due to wrong assumption on last-sample
Created attachment 354571 [details] [review] rtsp-stream: fix connection delay due to wrong assumption on last-sample unmap and unref
Review of attachment 354571 [details] [review]: Looks correct, can you explain how one could setup a server to reproduce this. This is clearly a candidate for gst-validate which gained RTSP support recently.
(In reply to Nicolas Dufresne (stormer) from comment #4) > Review of attachment 354571 [details] [review] [review]: > > Looks correct, can you explain how one could setup a server to reproduce > this. This is clearly a candidate for gst-validate which gained RTSP support > recently. I was investigating this issue on a production app. But now that we know the root case I guess this can be reproduced by using rtx elements. Because GstRtpRtxSend works in SSRC-multiplexed mode. So either using some existings tests in gst-plugins-good that use rtx elements. Or some existing tests in gst-rtsp-server because it looks like it enable retransmisson but not sure if this is by default or not.
While trying to make a unit test I found that the issue happens only if the aux-rtp buffer is the one buffer used to generate the rtsp url. Basically the rtsp server get the seqnum of the current buffer to write it in the rtsp url, ex: url=rtsp://0.0.0.0:34605/test/stream=0;seq=24345;rtptime=2832971436 . I suppose the client will parse this seqnum and wait for rtp buffers starting from this seqnum. So if the seqnum error/diff is really big then the client will wait a lot before accepting the rtp buffers. So it cannot be reproduced with the rtx elements because it will necessarly not be the first packets sent. In the production app, the issue happened because the first packet sent was a FEC rtp packet, with a different ssrc and a different seqnum base. And the rtsp server was using this seqnum to generate the rtsp url. For a unit test, one can try to inject, into the payloader, a fake rtp packet with a magic seqnum. And then the test will fail if the this magic seqnum appears in the rtsp url. The attached patch allow the rtsp server to ignore this packet and use the payloader's infos instead. The test can be added in gst-rtsp-server/test/checks/gst/rtspserver.c and run with: cd gst-rtsp-server/test/checks && make gst/rtspserver.check
Review of attachment 354571 [details] [review]: Let's get this one in. It's a very special case, we'd need a queue to do something better here.
Comment on attachment 354571 [details] [review] rtsp-stream: fix connection delay due to wrong assumption on last-sample commit d72284bdf8700c40e6eaca5e060cbb850a59288e Author: Julien Isorce <julien.isorce@gmail.com> Date: Thu Jun 22 07:25:07 2017 -0700 rtsp-stream: fix connection delay due to wrong assumption on last-sample Commit 852cc09f542af5cadd79ffd7fe79d6475cf57e14 assumed that multiudpsink's last-sample always comes from the payloader. Which is wrong if auxiliary streams are multiplexed in the same stream. So check the buffer's ssrc against the caps'ssrc before to use its seqnum. If not the same ssrc just use the payloader as done prior the commit above or when there is no last-sample yet. https://bugzilla.gnome.org/show_bug.cgi?id=784094
Hi, can I import the fix into 1.12 branch ? ( https://cgit.freedesktop.org/gstreamer/gst-rtsp-server/commit/?id=d72284bdf8700c40e6eaca5e060cbb850a59288e )
Sure.
commit 674faa5e5e8c8276c6dc9fff654fcdef77c1c688 Author: Julien Isorce <julien.isorce@gmail.com> AuthorDate: Thu Jun 22 07:25:07 2017 -0700 Commit: Julien Isorce <jisorce@oblong.com> CommitDate: Tue Jul 25 10:12:42 2017 +0100 rtsp-stream: fix connection delay due to wrong assumption on last-sample Commit 852cc09f542af5cadd79ffd7fe79d6475cf57e14 assumed that multiudpsink's last-sample always comes from the payloader. Which is wrong if auxiliary streams are multiplexed in the same stream. So check the buffer's ssrc against the caps'ssrc before to use its seqnum. If not the same ssrc just use the payloader as done prior the commit above or when there is no last-sample yet. https://bugzilla.gnome.org/show_bug.cgi?id=784094