GNOME Bugzilla – Bug 796207
omx: always signal drain cond when stopping streaming loop
Last modified: 2018-06-08 07:52:02 UTC
Fix hang in some cases, see second commit for details.
Created attachment 372152 [details] [review] omxvideoenc: factor out gst_omx_video_enc_pause_loop() No semantic change. I'm going to use it in more failure cases.
Created attachment 372153 [details] [review] omxvideodec: always signal drain cond when stopping streaming loop If for some reason something goes wrong and we stop the streaming loop we may end up with other threads still waiting on the drain cond. No more buffers will be produced by the component so they were waiting forever. Fix this by always signalling this cond when stopping the streaming loop.
Created attachment 372154 [details] [review] omxvideoenc: always signal drain cond when stopping streaming loop Similar change as the one I just did in omxvideodec.
Does that relates to these two similar looking bugs ? https://bugzilla.gnome.org/show_bug.cgi?id=743386 https://bugzilla.gnome.org/show_bug.cgi?id=776167
Review of attachment 372152 [details] [review]: Good.
Review of attachment 372153 [details] [review]: Good.
Review of attachment 372154 [details] [review]: I'm fine with this, but it's not symetrical, as you didnd't refactored in a seperate patch this time.
Attachment 372152 [details] pushed as 798bbc9 - omxvideoenc: factor out gst_omx_video_enc_pause_loop() Attachment 372153 [details] pushed as fd108f4 - omxvideodec: always signal drain cond when stopping streaming loop Attachment 372154 [details] pushed as 84483f3 - omxvideoenc: always signal drain cond when stopping streaming loop
(In reply to Nicolas Dufresne (ndufresne) from comment #4) > Does that relates to these two similar looking bugs ? > > https://bugzilla.gnome.org/show_bug.cgi?id=743386 > https://bugzilla.gnome.org/show_bug.cgi?id=776167 Could be. I commented on those. (In reply to Nicolas Dufresne (ndufresne) from comment #7) > Review of attachment 372154 [details] [review] [review]: > > I'm fine with this, but it's not symetrical, as you didnd't refactored in a > seperate patch this time. Indeed. The split was mostly to ease the reviewing. Thanks for the review.