GNOME Bugzilla – Bug 757337
adaptivedemux: release manifest lock before sending flush stop event
Last modified: 2015-10-30 13:28:23 UTC
sending flush stop event will serialiase on stream->src_srcpad. But the _src_chain function will first get the pad lock and then the manifest lock, so we cannot hold the manifest lock while sending the flush stop event (taking locks in reverse order will lead to deadlock) In conclusion, we need to release the manifest lock before flushing.
Created attachment 314432 [details] [review] proposed patch
Vincent, can you please review and merge this?
Yes, I have them merged locally, they're about to go in, except I'm getting audio corruption which I've just tracked to https://bugzilla.gnome.org/show_bug.cgi?id=757351. I'm about to test with a slightly older -base, and will push once that's done.
Also, I was wondering whether it'd be best to have both the flush-start and flush-stop out of the lock, even though there is no hook for flush-start that really requires it.
commit e0ef33542604ba81259bd62ea6d3c1b12a85ecf7 Author: Florin Apostol <florin.apostol@oregan.net> Date: Fri Oct 30 00:04:12 2015 +0000 adaptivedemux: release manifest lock before sending flush stop event https://bugzilla.gnome.org/show_bug.cgi?id=757337
(In reply to Vincent Penquerc'h from comment #4) > Also, I was wondering whether it'd be best to have both the flush-start and > flush-stop out of the lock, even though there is no hook for flush-start > that really requires it. I took the approach to keep the lock as much as possible and document each need to release it. My assumption is that there are only a few of these exceptions so the code will be more maintainable.
In case this introduced a race, an approach used at other places is to take the stream lock, then release the internal lock, and then sending flush stop, this would imply the internal lock to never be taken after the stream lock (like in chain function) though.