GNOME Bugzilla – Bug 697215
pad: discoverer timeout could flush the element pads while peer seetcaps
Last modified: 2013-07-24 09:39:06 UTC
Created attachment 240540 [details] [review] fix discoverer timeout mangling the state of an element when it is passing data between src and sink Gst discoverer timeout change the state of the pipeline thus if the discover thread has not complete its discovery processing. Here in the main thread, the timeout set the avdec_dvvideo sink pad to flushing when it pre_activate to false - GST_MODE_NONE - in gst_pad_activate_mode. This when the discover thread has not yet added the buffer pool : gst_video_decoder_negotiate_default attempts to set the caps before calling the decide_allocation handler and as the pad is flushing this returns false, decide_allocation never happens (thus get_output_buffer from avvidedec fails). Fixed by enclosing gst_pad_push_data content with the pad stream locking. My understanding is that pad_push_data pass data from one element pad to the same element peer pad. Also pad stream lock is recursive while element lock is not. Same for pad gst object lock. So locking on them on the discoverer timeout is not an option as this handler calls functions which also lock on both of them. backtrace: thread "6" is the discoverer thread , thread "1" the discoverer timeout one.
+ Trace 231720
Thread 6 (Thread 0x7fc9a3225700 (LWP 21513))
Thread 1 (Thread 0x7fc9fe6fd700 (LWP 21451))
I think this is fixed nowadays. When flushing happens everything should stop as soon as possible, which means that gst_video_decoder_negotiate() will fail and that everything should go into flushing mode asap.
Comment on attachment 240540 [details] [review] fix discoverer timeout mangling the state of an element when it is passing data between src and sink <slomo> wtay: https://bugzilla.gnome.org/show_bug.cgi?id=697215 any idea about this one? it shouldn't happen anymore because of changes elsewhere, but why can it happen that the stream-lock is not taken there already (see the patch)? <wtay> slomo, we don't take stream-lock on srcpads only on sinkpads <wtay> slomo, and srcpads in pull mode <slomo> i see <wtay> also also on srcpads when using a task <wtay> well, it does not have to be <slomo> why not? <wtay> there is no need to serialize pushes on the srcpad <wtay> only on the sinkpads <slomo> because when not using a task, it should already be protected by the sinkpad stream-lock? <wtay> well, there is no need to serialize <wtay> and yes, for shutdown of the element, we use the sinkpad lock <wtay> when that's taken, we are sure nothing is inside the element pushing on the srcpad either <slomo> ok, so that patch is not useful and only hiding another bug (that is fixed now)? <slomo> do we also take the srcpad lock for shutdown? e.g. when the srcpad has its own task running? <wtay> we do take the lock in shutdown but when in push mode, core does not take it <wtay> so if the element is using a task, core will wait for the task to complete
*** This bug has been marked as a duplicate of bug 701763 ***