GNOME Bugzilla – Bug 786348
omx: Ports should be disabled sequentially
Last modified: 2017-08-17 13:20:16 UTC
Created attachment 357691 [details] [review] omx{audio,video}{dec,enc}: sequentially disable ports because buffers are not shared In omx-il 1.1.2 the spec was not clear when the client has to disable ports in parallel or not. Starting from spec 1.2.0 the sequential way is mandatory except for a few specific cases.
Review of attachment 357691 [details] [review]: Looks good modulo comment. Tested on rpi and zynqultrascaleplus. ::: omx/gstomxaudioenc.c @@ +697,3 @@ + /* Disabling at the same time input port and output port is only + * required when a buffer is shared between the ports. This cannot + * be the case for a decoder because its input and output buffers it's an encoder here. So maybe say "encoder/decoder". ::: omx/gstomxvideoenc.c @@ +1006,3 @@ + /* Disabling at the same time input port and output port is only + * required when a buffer is shared between the ports. This cannot + * be the case for a decoder because its input and output buffers same here.
Comment on attachment 357691 [details] [review] omx{audio,video}{dec,enc}: sequentially disable ports because buffers are not shared Done thx ! commit eec88a965106d66e1b327b9bc15a114b6b0f262e Author: Julien Isorce <jisorce@oblong.com> Date: Thu Jul 20 09:43:19 2017 +0100 omx{audio,video}{dec,enc}: sequentially disable ports because buffers are not shared For the history, the parallel disable port has been introduced by: "00be69f omxvideodec: Disable output port when setting a new format" and then replicated to videoenc, audiodec and audioenc. This is only required to do 'parallel' if buffers are shared between ports. But for decoders and encoders the input and output buffer are of different nature by definition (bitstream vs images). So they cannot be shared. Also starting from IL 1.2.0 it is written in the spec that the parallel disable is not allowed and will return an error. Except when buffers are shared. Again here we know in advance that they are not shared so let's always do a sequential disable. Tested on Desktop, rpi and zynqultrascaleplus. https://bugzilla.gnome.org/show_bug.cgi?id=786348