[omx][Regression] Fw versions after 911147a3276beee fail to encode h264 when input buffers are configured before port is enabled

[drott]

The discussion in gst-omx bug 785967 shows that in firmware versions at least after 911147a3276beee gst-omx fails when input buffers are configured before the OMX encoder port is enabled.

This used to work in older firmware versions, unfortunately, we do not have an exact firmware hash for which it worked.

CC @thiblahute

[popcornmix]

Ping @6by9

[6by9]

Will investigate.
I think I may now apply extra sanity checking on the port definition than before which may be tripping it up. OMX is a right pain as to exactly which fields need validating vs the component dictating the value.

[6by9]

For info, rpi-update hashes:

• 5315c7a was the last release before I started tweaking video_encode
• ec9d84e required the stride being a multiple of 32.
• 911147a drops back to the software path if the stride is only a multiple of 16.

[drott]

For info, rpi-update hashes:

• 5315c7a was the last release before I started tweaking video_encode

Thanks for listing those. I can confirm that the same gst-omx tip of tree version works for encoding h264 with 5315c7a2f806c123e9e4ee26980f0019c43b9f69.

[6by9]

OK, as expected it's the stride and sliceheight parameters.
Using gst-launch-1.0 videotestsrc ! omxh264enc ! fakesink -v it's setting the port definition to

nFrameWidth = 320 (0x140)
nFrameHeight = 240 (0xf0)
nStride = 160 (0xa0)
nSliceHeight = 64 (0x40)

which the component is sanity checking and rejecting.
It looks like gst-omx has taken the default of 160x64 nStride 160, nSliceHeight 64 and updated only nFrame[Width|Height].

The old code always ignored the user supplied nSliceHeight and set it as nFrameHeight aligned up to 16., and set the pitch to the required value if nStride was less than the minimum required.
Having integrated the ISP for the format conversion I was expecting sane values to always be passed in, and also allowing the user to set their own strides/sliceheights should they wish for extra padding. I'll amend it to take the old behaviour when below the minimum required.

[popcornmix]

Fix should be in latest rpi-update firmware.

[ndufresne]

@6by9 thanks for investigating this. We will also check the provided stride on our side as we where expecting to pass a valid default stride (not half the width at least). We even have code to align from port definition requirement. For Tizonia and Xilinx stack, we have a partial OMX 1.2 draft dynamic buffer implemented for buffer management, so code path is totally different.

[6by9]

No problems @ndufresne. Sorry the issue crept in.

It's yet another of those awkward things in IL that the spec says that nStride gets validated on state change, so then you get yet another random reason for a state change failure with no obvious reason.
And as for nSliceHeight being read only and set by the component, it almost makes you wonder why they bothered in the first place. Why worry about a low memory footprint sliced mode if you can't control when it's used?

I think I tripped over the same thing when trying something else. IIRC there are two places in the code that called SetParameter for the PortDefinition. One updated nStride and nSliceHeight to sensible values before setting (gst_omx_video_enc_configure_input_buffer), the other didn't (gst_omx_video_enc_set_format). I'd initially put that down as something quirky in gst-omx instead of me having changed the behaviour :-(

We are almost at the position of merging a V4L2 M2M driver for video encode and decode so that may reduce the requirement for gst-omx. It's working fairly nicely with the GStreamer components, and supports optimisations such as dmabufs for zero copy.
If I can drop support for OpenMax then I will be one very happy chap!

[ndufresne]

We are almost at the position of merging a V4L2 M2M driver for video encode and decode so that may reduce the requirement for gst-omx. It's working fairly nicely with the GStreamer components, and supports optimisations such as dmabufs for zero copy.
If I can drop support for OpenMax then I will be one very happy chap!

That's is a really good news!

[drott]

Fix should be in latest rpi-update firmware.

I can confirm that the original issue is fixed and running a basic pipeline with omxh264enc works after updating to e0a0ba809ef11f6bed8a00d34dda961239ea432c using rpi-update. Thanks!