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 783976 - More OMX IL 1.2.0 support and Add OMX.Aratelia.video_decoder.vp8 to Tizonia config
More OMX IL 1.2.0 support and Add OMX.Aratelia.video_decoder.vp8 to Tizonia c...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-omx
git master
Other Mac OS
: Normal enhancement
: 1.13.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 782800 782988
Blocks:
 
 
Reported: 2017-06-19 23:08 UTC by Julien Isorce
Modified: 2017-08-21 13:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
omxvideodec: handle IL 1.2 behavior for OMX_SetParameter (3.07 KB, patch)
2017-06-19 23:17 UTC, Julien Isorce
committed Details | Review
config: add OMX.Aratelia.video_decoder.vp8 to Tizonia config (962 bytes, patch)
2017-06-19 23:17 UTC, Julien Isorce
none Details | Review
onfig: add OMX.Aratelia.video_decoder.vp8 to Tizonia config (968 bytes, patch)
2017-07-12 13:20 UTC, Julien Isorce
committed Details | Review

Description Julien Isorce 2017-06-19 23:08:49 UTC
OMX_SetParameter now also triggers SettingsChanged event on the other port. Currently gst-omx triggers reconfiguration and it should not do it for informational events.

A good example is the vp8 software decoder from Tizonia: https://github.com/tizonia/tizonia-openmax-il/tree/master/plugins/vp8_decoder/src
Comment 1 Julien Isorce 2017-06-19 23:17:10 UTC
Created attachment 354069 [details] [review]
omxvideodec: handle IL 1.2 behavior for OMX_SetParameter
Comment 2 Julien Isorce 2017-06-19 23:17:53 UTC
Created attachment 354070 [details] [review]
config: add OMX.Aratelia.video_decoder.vp8 to Tizonia config
Comment 3 Julien Isorce 2017-07-12 13:20:10 UTC
Created attachment 355425 [details] [review]
onfig: add OMX.Aratelia.video_decoder.vp8 to Tizonia config

Update since "gstomx.conf.in" enhancement.
Comment 4 Julien Isorce 2017-07-18 22:57:29 UTC
Comment on attachment 354069 [details] [review]
omxvideodec: handle IL 1.2 behavior for OMX_SetParameter

commit 1b7d0b8599010172d8ee313930f0f9db0629572b
Author: Julien Isorce <jisorce@oblong.com>
Date:   Mon Jun 19 23:56:02 2017 +0100

    omxvideodec: handle IL 1.2 behavior for OMX_SetParameter
    
    It triggers SettingsChanged on the other port and it is up to
    the client to decide if it should lead to a port reconfiguration.
    
    Settings are propagated to the other port for fields they have
    in common. But this event is only triggered on the other port
    if it actually change a setting.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=783976
Comment 5 Julien Isorce 2017-07-18 22:57:52 UTC
Comment on attachment 355425 [details] [review]
onfig: add OMX.Aratelia.video_decoder.vp8 to Tizonia config

commit a9a30870215e8264a4b28e28c3efed9355100adf
Author: Julien Isorce <jisorce@oblong.com>
Date:   Tue Jun 20 00:13:33 2017 +0100

    config: add OMX.Aratelia.video_decoder.vp8 to Tizonia config
    
    Useful mostly for testing/debugging purpose as this is a software
    based encoder (libvpxdec) for which GStreamer provides a direct
    wrapper.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=783976
Comment 6 Sebastian Dröge (slomo) 2017-07-19 06:34:04 UTC
Review of attachment 354069 [details] [review]:

::: omx/gstomxvideodec.c
@@ +772,3 @@
         goto done;
       }
+#if OMX_VERSION_MINOR == 2

This does not seem right. You could compile against 1.2 headers but use a 1.1 OMX IL
Comment 7 Julien Isorce 2017-07-19 10:18:06 UTC
(In reply to Sebastian Dröge (slomo) from comment #6)
> Review of attachment 354069 [details] [review] [review]:
> 
> ::: omx/gstomxvideodec.c
> @@ +772,3 @@
>          goto done;
>        }
> +#if OMX_VERSION_MINOR == 2
> 
> This does not seem right. You could compile against 1.2 headers but use a
> 1.1 OMX IL

You are right, I created https://bugzilla.gnome.org/show_bug.cgi?id=785114 
Note that bumping the "Minor" number might bring a few non-backward compatibility changes according to the spec.
Comment 8 Julien Isorce 2017-08-21 13:45:14 UTC
(In reply to Julien Isorce from comment #7)
> (In reply to Sebastian Dröge (slomo) from comment #6)
> > Review of attachment 354069 [details] [review] [review] [review]:
> > 
> > ::: omx/gstomxvideodec.c
> > @@ +772,3 @@
> >          goto done;
> >        }
> > +#if OMX_VERSION_MINOR == 2
> > 
> > This does not seem right. You could compile against 1.2 headers but use a
> > 1.1 OMX IL
> 
> You are right, I created https://bugzilla.gnome.org/show_bug.cgi?id=785114 
> Note that bumping the "Minor" number might bring a few non-backward
> compatibility changes according to the spec.

No in fact this is wrong, 1.2.0 is not backward compatible with 1.1.2. From the 1.2.0 spec: "due to the nature of some of the improvements introduced, backwards compatibility with previous versions is not being maintained".
So I will close https://bugzilla.gnome.org/show_bug.cgi?id=785114 as invalid