[PATCH v2 2/2] media: imx: vdic: Introduce mem2mem VDI deinterlacer driver

[Nicolas Dufresne]

Le jeudi 03 octobre 2024 à 16:57 +0200, Marek Vasut a écrit :
> On 9/26/24 1:16 PM, Philipp Zabel wrote:
> > On Mi, 2024-09-25 at 22:45 +0200, Marek Vasut wrote:
> > [...]
> > > > The driver is not taking ownership of prev_buf, only curr_buf is guaranteed to
> > > > exist until v4l2_m2m_job_finish() is called. Usespace could streamoff, allocate
> > > > new buffers, and then an old freed buffer may endup being used.
> > > 
> > > So, what should I do about this ? Is there some way to ref the buffer to
> > > keep it around ?
> > 
> > Have a look how other deinterlacers with temporal filtering do it.
> > sunxi/sun8i-di or ti/vpe look like candidates.
> I don't see exactly what those drivers are doing differently to protect 
> the prev buffer during deinterlacing . Can you be more specific ?

drivers/media/platform/sunxi/sun8i-di/sun8i-di.c:

                src = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
                if (ctx->prev)
                        v4l2_m2m_buf_done(ctx->prev, state);
                ctx->prev = src;


What that does is that whenever a src buffer has been processed and needs to be
kept has prev, it is removed from the m2m pending queue
(v4l2_m2m_src_buf_remove()), but not marked done. At the VB2 level it means that
buffer will keep its ACTIVE/QUEUED state, meaning is currently under driver
ownership. I also expect the driver to start producing frame on the second
device run, but I didn't spend the extra time to check if that is the case for
sun8i-di driver.

As for GStreamer wrapper, since it does not support deinterlaced, it does not
always allocate this one extra buffer for prev. If the driver implement the
MIN_BUFFERS_FOR_OUTPUT CID though, it will allocate matching number of extras.
Though, this has a side effect at driver level, since start streaming will be
delayed until 2 buffers has been queued and any way you need to queue 2 buffers
before the driver will produces its first buffer.

This comes to the next reason why the wrapper will fail, since for each buffer
that is pushed, it synchronously wait for the output. So it systematically stall
on first frame. As the author of that wrapper, I'm well aware of that, but never
had a use case where I needed to fix it. I will be happy to accept support for
that, though in current mainline state, there is no generic way to actually
know. One way is to thread the transform, but then GstBasetransform class can't
be used, its a lot of work and adds complexity.

We can certainly fix gstv4l2transform.c behaviour with adding
MIN_BUFFERS_FOR_OUTPUT in upstream drivers. That would be easy to handle with
adding a matching buffering delay. These deinterlacers works for Kodi, since the
userspace code they have is not generic and have internal knowledge of the
hardware it is running on.

Nicolas