After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 726038 - omxvideodec: Multiple issues during seeks
omxvideodec: Multiple issues during seeks
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-omx
git master
Other Linux
: Normal major
: 1.2.0
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-03-10 15:11 UTC by Josep Torra Valles
Modified: 2014-07-23 08:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Ensure to populate the most downstream port (2.44 KB, patch)
2014-03-25 15:02 UTC, Josep Torra Valles
committed Details | Review
Use flush because reset is deprecated. (2.59 KB, patch)
2014-03-25 15:04 UTC, Josep Torra Valles
committed Details | Review
Fixes races while seeking (4.60 KB, patch)
2014-03-25 15:11 UTC, Josep Torra Valles
committed Details | Review

Description Josep Torra Valles 2014-03-10 15:11:20 UTC
There's several issues triggered during seeks when omxvideodec uses a shared pool.

Those are reproducible with eglglessink and testegl example, and also triggered with ximagesink when the resizer component is used.

Initial fixes were included in bug #722686 but those are independent of resize component functionality so here there's another bug focused on the seeking issues.

The final fix set can be found at http://cgit.freedesktop.org/~adn770/gst-omx/log/?h=seeking

The first two patches are cherry picked from bug #722686.

The last patch fixes the race issue and supersedes all other related patches in bug #722686.
Comment 1 Josep Torra Valles 2014-03-25 15:02:38 UTC
Created attachment 272857 [details] [review]
Ensure to populate the most downstream port

Cherry picked patch from 722686
Comment 2 Josep Torra Valles 2014-03-25 15:04:40 UTC
Created attachment 272858 [details] [review]
Use flush because reset is deprecated.

Cherry picked patch from 722686.

Just to align with the current state of base video decoder class.
Comment 3 Josep Torra Valles 2014-03-25 15:11:34 UTC
Created attachment 272859 [details] [review]
Fixes races while seeking

The flush had several race behaviours due omx components being in executing state while flushing and _loop task not being stopped/started in the right times.

The restart task had been moved to the _chain in order to ensure that the new segment event after flush and other base class stuff is properly handled before we start pushing decoded frames again.
Comment 4 Josep Torra Valles 2014-03-25 15:19:13 UTC
Review of attachment 272857 [details] [review]:

Clearly there was a bug in the code as decoder output port was populated unconditionally.
The fix seems correct to me.
Comment 5 Josep Torra Valles 2014-03-25 15:21:47 UTC
Review of attachment 272858 [details] [review]:

This is just an enhancement to align with current base class API and seems a trivial change.
I'm fine with that change being made upstream.
Comment 6 Josep Torra Valles 2014-03-25 15:31:00 UTC
Review of attachment 272858 [details] [review]:

I've just forgot change the status to reviewed.
Comment 7 Josep Torra Valles 2014-03-25 15:31:32 UTC
Review of attachment 272857 [details] [review]:

I've just forgot change the status to reviewed.
Comment 8 Julien Isorce 2014-03-25 15:51:35 UTC
Review of attachment 272859 [details] [review]:

Tested and works fine. It fixes the "ERROR_gstomxvideodec.c:1314:gst_omx_video_dec_loop:<omxh264dec-omxh264dec0> No corresponding frame found" we get each time we seek when using eglglessink or testegl example. And seeking works without races.
Comment 9 Julien Isorce 2014-03-25 16:09:59 UTC
commit 73d83f311c23429f86fc2377f2b2828db5af9898
Author: Josep Torra <n770galaxy@gmail.com>
Date:   Fri Mar 7 20:08:05 2014 +0100

    omxvideodec: fixes race condition during seeks
    
    Acording 6.1.3 Seek Event Sequence in the OpenMAX IL 1.1.2 spec
    document in order to flush the component it needs to be in
    paused state.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=726038

commit 39ca9f980eeca0610cdc3fec2774681514a16ff2
Author: Julien Isorce <julien.isorce@collabora.co.uk>
Date:   Wed Jan 29 18:31:26 2014 +0000

    omxvideodec: use flush because reset is deprecated
    
    https://bugzilla.gnome.org/show_bug.cgi?id=726038

commit 777411c286dafd1be7024c238a8a5c851ddd3e8e
Author: Julien Isorce <julien.isorce@collabora.co.uk>
Date:   Mon Jan 27 17:03:50 2014 +0000

    omxvideodec: populate the most downstream output port on reset
    
    Make seeking work when using egl_render component
    
    https://bugzilla.gnome.org/show_bug.cgi?id=726038