Skip to content

v4l2: videodec: Rework dynamic resolution change handling

Nicolas Dufresne requested to merge ndufresne/gstreamer:imx8qxp into main

Since 1.22, the decoder would use the caps display width/height to setup and allocate capture buffer very early. It then relied in the event to let the decoder tell us when this guess wasn't right. In most situation, the capture queue get allocated twice, which takes a lot of time. In other situation, bug in the driver may cause the driver to "forget" to notify, as the buffers were big enough. The resulting issue is that the caps sent downstream our then inaccurate (examples are not interlaced, too big of a resolution, wrong colorimetry, etc.)

While the approach wasn't against the spec, it wasn't ideal. It was also breaking legacy support, since legacy supports requires the first bitstream buffer to be queued before we setup the capture queue.

In this MR, all the capture queue setup is moved into the capture thread. This removes a possible race around the resolution_changed atomic variable and the need for the variable itself. The GstPoll is moved back into the GstV4L2OBject like it once was, allowing to poll "just for the event" initially and when the pixel format selected requires it.

CC of active contributors: @Shengqi.Yu @HuQian @hq @m.tretter @ayaka @ming.qian

Here's some todo, this code is a still a bit fresh, and can probably have few more passes of improvement. And we need to chase after any corner cases. I would like to backport this into 1.22 as the new resolution code seems to cause regressions on non-NXP HW.

TODO:

  • Verify handling of the flushing state of the GstPoll
  • Investigate playbin stall on IMX8QXP when a lot of debug is enabled
  • Validate on Raspbery Pi (and other HW if possible, I don't have a Venus setup)
Edited by Nicolas Dufresne

Merge request reports