[Bug 755169] dashdemux: can we have multiple seek events at the same time?

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=755169

--- Comment #53 from Florin Apostol <florin.apostol at oregan.net> ---
Review of attachment 312126:
 --> (https://bugzilla.gnome.org/review?bug=755169&attachment=312126)

1. comment in gst_adaptive_demux_stream_download_uri is wrong saying the
function should be called with fragment_download_lock. It shouldn't because the
function grabs it.

::: gst-libs/gst/adaptivedemux/gstadaptivedemux.c
@@ +1069,3 @@
+  if (!demux->priv->exposing)
+    g_mutex_lock (&demux->priv->api_lock);
+

this has 2 problems. 

First, the exposing condition should be used only for RECONFIGURE event. All
others need to be serialized using the api_lock. Otherwise, while one thread is
exposing, a second seek gets a free ticket to bypass the locks and mess the
data :)

Second, to avoid the free ticket problem also for reconfigure events, you need
to determine if this is the thread holding the lock (and doing the exposure) or
not. And to determine that, you need the mutex to be reentrant. 
I suggest to add a reentrant lock demux->private->exposing_lock. The
gst_adaptive_demux_expose_streams function, while holding the api_lock and the
manifest_lock will also hold exposing_lock for the duration between
demux->priv->exposing = TRUE; and demux->priv->exposing = FALSE; The
gst_adaptive_demux_src_event function, for GST_EVENT_RECONFIGURE will grab this
exposing_lock, read the exposing value and release the lock. If the exposing
was TRUE, it is the same thread and it can bypass getting the api_lock and
manifest lock. If exposing was false, this is a random RECONFIGURE event
received outside exposing, so it must get api_lock and manifest lock.

@@ +2862,3 @@
 }

+/* with manifest_lock */

comment is wrong. Function is called with the lock not taken. But maybe it
should be called with the lock taken and temporarily release it while sleeping

Also the function should call g_cond_wait_until in a while loop and check for
demux->cancelled. The code is similar to the one from
gst_adaptive_demux_stream_download_loop (search for g_cond_wait_until) but
there the demux->cancelled is checked.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.