GNOME Bugzilla – Bug 725898
Lose data when producing data faster than sendt during tunneling rtps/rtp(TCP)
Last modified: 2014-04-10 14:13:19 UTC
Created attachment 271227 [details] [review] patch Lose data when producing data faster than sendt during tunneling rtps7rtp (TCP) IF the server producing data faster than transferred to remote end then data ends up in a queue and finally the queue get full and throw away data. I attach two patches for this matter. wait.patch : This provide a method for waiting until there is room in queue. setwait.patch : This make it configurable to resend data that otherwise have been lost after waiting until there is room in queue.
Created attachment 271228 [details] [review] patch
commited with some changes: * changed function name * added macro to check for queue fullness * check for bytes limits as well * signal gcond when limits change * make function prototype like other functions with GstRTSPResult and GTimeVal timeout * handle invalid parameters like we handle them elsewhere * add to docs and defs * Add Since markers commit 0b30fdbfbe577fda0db9bcaf2d824a6cdbbf2ea1 Author: Göran Jönsson <goranjn@axis.com> Date: Thu Mar 6 13:55:17 2014 +0100 rtspconnection: gst_rtsp_watch_wait_backlog New method that wait until there is room in backlog queue. Fixes https://bugzilla.gnome.org/show_bug.cgi?id=725898
just an idea, why not doing something like gst_rtsp_connection_poll (conn, GST_RTSP_EV_READ, 20) from the server in case of GST_RTSP_ENOMEM? Wouldn't that be a better approach? This way we can also cancel the poll in case we want to, e.g. when client disconnects, and entirely skip gst_rtsp_watch_wait_backlog().
The problem sill remain. So I adding two new patches and set one the old ones as obsolete. The unblock_wait_queue.patch (gst-plugins-base) adding functionality to unblock the function gst_rtsp_watch_wait_backlog. The set_wait_queue.patch (gst-rtsp-server) is. * Using new name on function gst_rtsp_watch_wait_backlog * Unblock gst_rtsp_watch_wait_backlog when close and finalizing. * Change in while loop that resend when queue is full.
Created attachment 272843 [details] [review] patch This will be replaced
Created attachment 272844 [details] [review] patch
Created attachment 272986 [details] [review] gst-plugins-base patch
(In reply to comment #3) > just an idea, why not doing something like gst_rtsp_connection_poll (conn, > GST_RTSP_EV_READ, 20) from the server in case of GST_RTSP_ENOMEM? > Wouldn't that be a better approach? This way we can also cancel the poll in > case we want to, e.g. when client disconnects, and entirely skip > gst_rtsp_watch_wait_backlog(). It sounds like a good idea to integrate it into poll but then poll needs to take into account the backlog queue as well.
I think it is also not racefree, you usually need a flush start and stop operation because else you might unblock the wait before you start the wait operation.
commit 8d439edd7abbb9a6b30c1320734e7eb041136cba Author: Wim Taymans <wtaymans@redhat.com> Date: Fri Mar 28 09:32:20 2014 +0100 rtspconnection: add flush method Add a method to set/unset the flushing state that makes _wait_backlog() unlock. See https://bugzilla.gnome.org/show_bug.cgi?id=725898
(In reply to comment #8) > (In reply to comment #3) > > just an idea, why not doing something like gst_rtsp_connection_poll (conn, > > GST_RTSP_EV_READ, 20) from the server in case of GST_RTSP_ENOMEM? > > > Wouldn't that be a better approach? This way we can also cancel the poll in > > case we want to, e.g. when client disconnects, and entirely skip > > gst_rtsp_watch_wait_backlog(). > > It sounds like a good idea to integrate it into poll but then poll needs to > take into account the backlog queue as well. After some investigations, it doesn't sound like a good idea anymore. The backlog is at the watch level and poll at the lower connection level, best not to mix them and implement a simple flush method.
Created attachment 273399 [details] [review] gst-plugins-base patch Small corection
Created attachment 273400 [details] [review] gst-rtsp-server patch New patch in gst-rtsp-server * Change due to change in a api gstrtspconnection.h * Changes off mutexes in rtsp-stream.c not longer needed due to patch stream: release lock while pushing out packets
Added new patches for * gst-plugins-base . small correction. * gst-rtsp-server Change because of chang in api gstrtspconnection.h Change because split off mutex in rtsp-stream.c not longer needed after patch "stream: release lock while pushing out packets"
commit a483e9095505c6b0c2a07855770005ce09f74f9c Author: Göran Jönsson <goranjn@axis.com> Date: Tue Apr 1 10:38:23 2014 +0200 rtspconnection: Added gst_rtsp_watch_set_flushing to list. Added gst_rtsp_watch_set_flushing to list in file libgstrtsp.def
The gst-rtsp-server patch is wrong. pleasew do not commit.
Created attachment 273762 [details] [review] gst-rtsp-server patch Some correction done to previous gst-rtsp-server patch
it does not seem to pass make check, gst/rtspserver hangs forever.
The problem is that you set the watch to flushing before you send the teardown reply, which makes the reply never arrive at the client. What did you want to do here?
I rewrote the patch a little like this: commit 11369d38ef6bc6a054b6da521cb9e2dbe6226373 Author: Göran Jönsson <goranjn@axis.com> Date: Tue Apr 1 13:04:21 2014 +0200 client: Add drop-backlog property When we have too many messages queued for a client (currently hardcoded to 100) we overflow and drop the messages. Add a drop-backlog property to control this behaviour. Setting this property to FALSE will retry to send the messages to the client by waiting for more room in the backlog. Fixes https://bugzilla.gnome.org/show_bug.cgi?id=725898