GNOME Bugzilla – Bug 730989
omxdec: Get stuck while doing ctrl+c during preroll
Last modified: 2014-07-23 08:14:13 UTC
Created attachment 277529 [details] Trace omx*:6 getting stuck in preroll I can get the omxdecoder stuck in acquire_buffer() by doing ctrl+c during the preroll state with the following pipeline: $ gst-launch-1.0 filesrc location=Sintel.2010.720p.mkv ! matroskademux ! h264parse ! omxh264dec ! fakesink Setting pipeline to PAUSED ... Pipeline is PREROLLING ... handling interrupt. Interrupt: Stopping pipeline ... Setting pipeline to NULL ... Caught SIGQUIT I can reproduce it on my OMX platform and on RPi. Howver the issue does not always happened. I attached a log of the above pipeline on RPi with --gst-debug=*:3,omx*:6 I think it's a race condition between port flushing, pipeline stop and component reconfiguration.
A stack trace from gdb (thread apply all bt) would also be helpful.
Created attachment 277537 [details] Trace omx*:6 getting stuck in preroll Update gstlog for following backtrace.
Created attachment 277538 [details] Threads backtrace All threads backtrace
Just a note, I've had to fix the same pattern in v4l2videodec (didn't check OMX). The loop thread is a pad task, hence it's holding on the stream lock. We cannot rely on GstVideoDecoder::stop() as it's called after trying to deactivate the pad. When we try to deactivate the pad, the loop is still holding on the stream lock, so deactivate will block on that: gstpad.c:957 GST_PAD_STREAM_LOCK (pad); <-- ** Here ** GST_DEBUG_OBJECT (pad, "stopped streaming"); GST_OBJECT_LOCK (pad); remove_events (pad); GST_OBJECT_UNLOCK (pad); GST_PAD_STREAM_UNLOCK (pad); So what I did, is to override the state change virtual, and before chaining to the base class, I do what's needed to make by loop exit. The base class could be improved, but this fixed it.
(In reply to comment #4) > Just a note, I've had to fix the same pattern in v4l2videodec (didn't check > OMX). The loop thread is a pad task, hence it's holding on the stream lock. We > cannot rely on GstVideoDecoder::stop() as it's called after trying to > deactivate the pad. When we try to deactivate the pad, the loop is still > holding on the stream lock, so deactivate will block on that: > > gstpad.c:957 > GST_PAD_STREAM_LOCK (pad); <-- ** Here ** > GST_DEBUG_OBJECT (pad, "stopped streaming"); > GST_OBJECT_LOCK (pad); > remove_events (pad); > GST_OBJECT_UNLOCK (pad); > GST_PAD_STREAM_UNLOCK (pad); > > So what I did, is to override the state change virtual, and before chaining to > the base class, I do what's needed to make by loop exit. The base class could > be improved, but this fixed it. I take a look and try the same solution that you did in v4l2videodec.
Note that the same change will be necessary in the audio encoders/decoders and the video encoders. And in the androidmedia plugin :)
Note this code here http://cgit.freedesktop.org/gstreamer/gst-omx/tree/omx/gstomxvideodec.c#n362 which in theory should unblock this here and stop the loop http://cgit.freedesktop.org/gstreamer/gst-omx/tree/omx/gstomxvideodec.c#n1186 (the FLUSHING case). That's exactly what you describe Nicolas, right? The problem according to the debug output here is that the flushing setting happens while in this code here http://cgit.freedesktop.org/gstreamer/gst-omx/tree/omx/gstomxvideodec.c#n1206 I wonder why the acquire_buffer() afterwards does not immediately return flushing... I think it's because of http://cgit.freedesktop.org/gstreamer/gst-omx/tree/omx/gstomx.c#n2148 which is called in the output port reconfiguration. The enabled-flushing and flushing-flushing probably needs to be tracked separately, but handled the same in many other places. I'll try to give you a patch for testing in a bit.
Created attachment 277621 [details] [review] omx: Don't handle disabling/enabling ports exactly like flushing Otherwise we might abort a flush operation in another thread when enabling/disabling ports, leading to deadlocks sometimes.
Aurélien, does this fix it for you? This definitely needs some further testing
(In reply to comment #9) > Aurélien, does this fix it for you? This definitely needs some further testing I tested it on both RPi and on my OMX platform. On RPi after applying the patch, I didn't succeed to reproduce the issue. However on my OMX platform, I ended up with a deadlock involving gstvideodecoder stream_lock due to a missing unlock in an error path, I will add a patch to this bug report to add it. Finally, with your patch, I can't reproduce the bug so I think it fix it.
Created attachment 277723 [details] [review] omxvideodec: add missing stream unlock in error path
Thanks for testing and for your patch, I pushed both patches now. Which platforms is your OMX platform btw?
(In reply to comment #12) > Thanks for testing and for your patch, No problem :) > Which platforms is your OMX platform btw? It's a SoC made by my company with HW decoders.
(In reply to comment #13) > > Which platforms is your OMX platform btw? > It's a SoC made by my company with HW decoders. If it's not all secret and you're interested, we could add a config file for that platform to gst-omx to make it easier for people to use your hardware
(In reply to comment #14) > (In reply to comment #13) > If it's not all secret and you're interested, we could add a config file for > that platform to gst-omx to make it easier for people to use your hardware Unfortunately, hardware is not intended for end-user :(, but thanks for the proposal.