GNOME Bugzilla – Bug 723851
omxvideoenc/dec: Fix for a startup race condition
Last modified: 2014-07-23 08:24:38 UTC
Created attachment 268414 [details] [review] Fix. Hi guys, the attached patch is a simple fix for a race condition we encountered at startup. The basic problem seems to be that the port who should get disabled because of reconfigure is still in the process of being enabled. Please review and apply if approved, Christian.
Comment on attachment 268414 [details] [review] Fix. Which other code was enabling the port if you get to this point? That line is only called if the decoder was not configured before, and in that case no other code should've enabled or disabled the output port yet.
(In reply to comment #1) > (From update of attachment 268414 [details] [review]) > Which other code was enabling the port if you get to this point? I don't have the slightest idea. We just got random "enabled/disabled pending" errors at startup from the following gst_omx_port_set_enabled call and adding this wait fixed it.
Can you get a debug log with GST_DEBUG=omx*:6 for such a case?
I will try, but trying to get a log always changed the timing for me and so made the bug hard to reproduce. Leo was the only one who could reproduce the bug and get a trace log at the same time and so figured out where it came from. The only thing I can assure you that after applying that patch we never had the issue again.
Can you get me that log? Or at provide a way to reproduce it (what hardware are you using?)? Just the patch alone does not make much sense to me, I don't see how this situation could happen with the current code.
Let's drop this patch for now. It looks like we can't reproduce the issue anymore. If I can get it to reproduce once more I will open up this bugreport again.
Created attachment 270274 [details] Log of failure. Took me a while, but I was able to reproduce this bug again. Requested gst log with omx*:6 is attached.
Sorry, forget that log. That was a leftover from another run. I'm still trying to reproduce the problem.
Created attachment 270294 [details] Log of failure. OK. Finally a clean log of the problem.
Created attachment 270634 [details] [review] Updated fix for the decoder. I figured out what the root cause of the problem is. gst_omx_video_dec_reset and gst_omx_video_enc_reset doesn't check the state of the component before starting the src pad loop. This results is that _set_format is trying to enable/disable the output port at the same time as the other task.
Created attachment 270635 [details] [review] Same fix for the encoder.
commit 4e4f093319e9f28d242f42b4fee3a1ae1157920d Author: Christian König <christian.koenig@amd.com> Date: Sat Mar 1 18:49:41 2014 +0100 omxvideoenc: fix startup race condition The reset function shouldn't start the src pad loop if it wasn't started before. Signed-off-by: Christian König <christian.koenig@amd.com> commit 0a8cfcde877b50a06a64f7ded4807707edeb1cd9 Author: Christian König <christian.koenig@amd.com> Date: Sat Mar 1 18:48:17 2014 +0100 omxvideodec: fix startup race condition The reset function shouldn't start the src pad loop if it wasn't started before. Signed-off-by: Christian König <christian.koenig@amd.com>
Comment on attachment 270635 [details] [review] Same fix for the encoder. Thanks for the patches and debugging this annoying race condition :)