On 16/07/16 09:12, Jan Schmidt wrote:
On 16/07/16 05:25, Andrew Eikum wrote:
Fix state setting of new elements - always just set them to PAUSED. gst_element_sync_state_with_parent() can be problematic, and we only need these in paused or playing anyway.
I'm not sure about this part. It makes sense to me for all members of a container to be in the same state. Why is it better for these to be PAUSED while others are PLAYING? If it's needed, this should be its own patch, too.
This is actually the part that fixes bug 40764 (although the refcounting ones are needed to avoid crashes too). You can set them to PLAYING or PAUSED - it doesn't actually matter. The only difference between playing and paused states in GStreamer is in elements that do operations on the clock, such as audio or video sinks. For videoflip and videoconvert the important thing is that they aren't still in READY state by the time they receive data, because then their pads will not have been activated.
That's the core of the race condition that's causing 40764.
Actually, I just realised this isn't true. The actual cause of 40764 is replacing the bus on decodebin which is now inside the container bin. It was OK when decodebin was the top-level element, but not when it's a child element. Setting the bus on the top-level bin is sufficient to fix 40764 reliably, which I'd fixed separately while cleaning up the ref-counting. Continuing to use gst_element_sync_state_to_parent here seems OK.
New patches coming soon.
J.