[RFC,media] mem2mem: add support for hardware buffered queue

On mem2mem decoders with a hardware bitstream ringbuffer, to drain the
buffer at the end of the stream, remaining frames might need to be decoded
without additional input buffers being provided, and after calling streamoff
on the v4l2 output queue. This also allows a driver to copy input buffers
into their bitstream ringbuffer and immediately mark them as done to be
dequeued.

The motivation for this patch is hardware assisted h.264 reordering support
in the coda driver. For high profile streams, the coda can hold back
out-of-order frames, causing a few mem2mem device runs in the beginning, that
don't produce any decompressed buffer at the v4l2 capture side. At the same
time, the last few frames can be decoded from the bitstream with mem2mem device
runs that don't need a new input buffer at the v4l2 output side. A streamoff
on the v4l2 output side can be used to put the decoder into the ringbuffer
draining end-of-stream mode.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/v4l2-core/v4l2-mem2mem.c | 26 ++++++++++++++++++++------
 include/media/v4l2-mem2mem.h           |  3 +++
 2 files changed, 23 insertions(+), 6 deletions(-)
  

On Wed 22 May 2013 12:17:36 Philipp Zabel wrote:
> On mem2mem decoders with a hardware bitstream ringbuffer, to drain the
> buffer at the end of the stream, remaining frames might need to be decoded
> without additional input buffers being provided, and after calling streamoff
> on the v4l2 output queue. This also allows a driver to copy input buffers
> into their bitstream ringbuffer and immediately mark them as done to be
> dequeued.
> 
> The motivation for this patch is hardware assisted h.264 reordering support
> in the coda driver. For high profile streams, the coda can hold back
> out-of-order frames, causing a few mem2mem device runs in the beginning, that
> don't produce any decompressed buffer at the v4l2 capture side. At the same
> time, the last few frames can be decoded from the bitstream with mem2mem device
> runs that don't need a new input buffer at the v4l2 output side. A streamoff
> on the v4l2 output side can be used to put the decoder into the ringbuffer
> draining end-of-stream mode.

This is just a pointer to a related issue: how to signal to the application
that the end-of-stream has been reached:

http://www.mail-archive.com/linux-media@vger.kernel.org/msg60916.html

How does the coda driver handle eos signalling?

Regards,

	Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Am Mittwoch, den 22.05.2013, 12:36 +0200 schrieb Hans Verkuil:
> On Wed 22 May 2013 12:17:36 Philipp Zabel wrote:
> > On mem2mem decoders with a hardware bitstream ringbuffer, to drain the
> > buffer at the end of the stream, remaining frames might need to be decoded
> > without additional input buffers being provided, and after calling streamoff
> > on the v4l2 output queue. This also allows a driver to copy input buffers
> > into their bitstream ringbuffer and immediately mark them as done to be
> > dequeued.
> > 
> > The motivation for this patch is hardware assisted h.264 reordering support
> > in the coda driver. For high profile streams, the coda can hold back
> > out-of-order frames, causing a few mem2mem device runs in the beginning, that
> > don't produce any decompressed buffer at the v4l2 capture side. At the same
> > time, the last few frames can be decoded from the bitstream with mem2mem device
> > runs that don't need a new input buffer at the v4l2 output side. A streamoff
> > on the v4l2 output side can be used to put the decoder into the ringbuffer
> > draining end-of-stream mode.
> 
> This is just a pointer to a related issue: how to signal to the application
> that the end-of-stream has been reached:
> 
> http://www.mail-archive.com/linux-media@vger.kernel.org/msg60916.html

Thank you for the pointer, this is exactly

> How does the coda driver handle eos signalling?

It does not, yet. The patches I'm currently preparing are still just
calling v4l2_event_queue_fh before v4l2_m2m_job_finish from the
interrupt handler after the device run signals that there is no more
data, but I think this needs to be done with the DQBUF instead.

I like the idea of an EOS buffer flag for the capture side.

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Hello Philip,

On 05/22/2013 12:17 PM, Philipp Zabel wrote:
> On mem2mem decoders with a hardware bitstream ringbuffer, to drain the
> buffer at the end of the stream, remaining frames might need to be decoded
> without additional input buffers being provided, and after calling streamoff
> on the v4l2 output queue. This also allows a driver to copy input buffers
> into their bitstream ringbuffer and immediately mark them as done to be
> dequeued.
> 
> The motivation for this patch is hardware assisted h.264 reordering support
> in the coda driver. For high profile streams, the coda can hold back
> out-of-order frames, causing a few mem2mem device runs in the beginning, that
> don't produce any decompressed buffer at the v4l2 capture side. At the same
> time, the last few frames can be decoded from the bitstream with mem2mem device
> runs that don't need a new input buffer at the v4l2 output side. A streamoff
> on the v4l2 output side can be used to put the decoder into the ringbuffer
> draining end-of-stream mode.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  drivers/media/v4l2-core/v4l2-mem2mem.c | 26 ++++++++++++++++++++------
>  include/media/v4l2-mem2mem.h           |  3 +++
>  2 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
> index 357efa4..52818cd 100644
> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> @@ -196,6 +196,10 @@ static void v4l2_m2m_try_run(struct v4l2_m2m_dev *m2m_dev)
>   * 2) at least one destination buffer has to be queued,
>   * 3) streaming has to be on.
>   *
> + * If a queue is buffered (for example a decoder hardware ringbuffer that has
> + * to be drained before doing streamoff), allow scheduling without v4l2 buffers
> + * on that queue and even when the queue is not streaming anymore.

Does it mean you want to be able to queue buffers on e.g. OUTPUT queue, while
this queue is in STREAM OFF state ?

Or do you really want to be able to to queue/dequeue buffers on CAPTURE queue,
while the OUTPUT queue is in STREAM OFF state ?

>   * There may also be additional, custom requirements. In such case the driver
>   * should supply a custom callback (job_ready in v4l2_m2m_ops) that should
>   * return 1 if the instance is ready.
> @@ -210,7 +214,7 @@ static void v4l2_m2m_try_schedule(struct v4l2_m2m_ctx *m2m_ctx)
>  	m2m_dev = m2m_ctx->m2m_dev;
>  	dprintk("Trying to schedule a job for m2m_ctx: %p\n", m2m_ctx);
>  
> -	if (!m2m_ctx->out_q_ctx.q.streaming
> +	if ((!m2m_ctx->out_q_ctx.q.streaming && !m2m_ctx->out_q_ctx.buffered)

This seems a bit asymmetric. Even if a driver sets 'buffered' on the capture
queue nothing really changes, right ?

Thanks,
Sylwester

>  	    || !m2m_ctx->cap_q_ctx.q.streaming) {
>  		dprintk("Streaming needs to be on for both queues\n");
>  		return;
> @@ -224,7 +228,7 @@ static void v4l2_m2m_try_schedule(struct v4l2_m2m_ctx *m2m_ctx)
>  	}
>  
>  	spin_lock_irqsave(&m2m_ctx->out_q_ctx.rdy_spinlock, flags);
> -	if (list_empty(&m2m_ctx->out_q_ctx.rdy_queue)) {
> +	if (list_empty(&m2m_ctx->out_q_ctx.rdy_queue) && !m2m_ctx->out_q_ctx.buffered) {
>  		spin_unlock_irqrestore(&m2m_ctx->out_q_ctx.rdy_spinlock, flags);
>  		spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags_job);
>  		dprintk("No input buffers available\n");
> @@ -434,9 +438,11 @@ int v4l2_m2m_streamoff(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
>  
>  	m2m_dev = m2m_ctx->m2m_dev;
>  	spin_lock_irqsave(&m2m_dev->job_spinlock, flags_job);
> -	/* We should not be scheduled anymore, since we're dropping a queue. */
> -	INIT_LIST_HEAD(&m2m_ctx->queue);
> -	m2m_ctx->job_flags = 0;gmane.linux.drivers.video-input-infrastructure	if (list_empty(&m2m_ctx->cap_q_ctx.rdy_queue))
> +	if (!q_ctx->buffered) {
> +		/* We should not be scheduled anymore, since we're dropping a queue. */
> +		INIT_LIST_HEAD(&m2m_ctx->queue);
> +		m2m_ctx->job_flags = 0;
> +	}
>  
>  	spin_lock_irqsave(&q_ctx->rdy_spinlock, flags);
>  	/* Drop queue, since streamoff returns device to the same state as after
> @@ -444,7 +450,7 @@ int v4l2_m2m_streamoff(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
>  	INIT_LIST_HEAD(&q_ctx->rdy_queue);
>  	spin_unlock_irqrestore(&q_ctx->rdy_spinlock, flags);
>  
> -	if (m2m_dev->curr_ctx == m2m_ctx) {
> +	if (!q_ctx->buffered && (m2m_dev->curr_ctx == m2m_ctx)) {
>  		m2m_dev->curr_ctx = NULL;
>  		wake_up(&m2m_ctx->finished);
>  	}
> @@ -640,6 +646,14 @@ err:
>  }	if (list_empty(&m2m_ctx->cap_q_ctx.rdy_queue))
>  EXPORT_SYMBOL_GPL(v4l2_m2m_ctx_init);
>  
> +void v4l2_m2m_queue_set_buffered(struct vb2_queue *vq)
> +{
> +	struct v4l2_m2m_queue_ctx *q_ctx = container_of(vq, struct v4l2_m2m_queue_ctx, q);
> +
> +	q_ctx->buffered = true;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_queue_set_buffered);
> +
>  /**
>   * v4l2_m2m_ctx_release() - release m2m context
>   *
> diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
> index 0f4555b..3415845 100644
> --- a/include/media/v4l2-mem2mem.h
> +++ b/include/media/v4l2-mem2mem.h
> @@ -60,6 +60,7 @@ struct v4l2_m2m_queue_ctx {
>  	struct list_head	rdy_queue;
>  	spinlock_t		rdy_spinlock;
>  	u8			num_rdy;
> +	bool			buffered;
>  };
>  
>  struct v4l2_m2m_ctx {
> @@ -134,6 +135,8 @@ struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev *m2m_dev,
>  		void *drv_priv,
>  		int (*queue_init)(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq));
>  
> +void v4l2_m2m_queue_set_buffered(struct vb2_queue *vq);
> +
>  void v4l2_m2m_ctx_release(struct v4l2_m2m_ctx *m2m_ctx);
>  
>  void v4l2_m2m_buf_queue(struct v4l2_m2m_ctx *m2m_ctx, struct vb2_buffer *vb);
  

Hi Philipp, Hans,

> On mem2mem decoders with a hardware bitstream ringbuffer, to drain the
> buffer at the end of the stream, remaining frames might need to be
> decoded without additional input buffers being provided, and after
> calling streamoff on the v4l2 output queue. This also allows a driver
> to copy input buffers into their bitstream ringbuffer and immediately
> mark them as done to be dequeued.
> 
> The motivation for this patch is hardware assisted h.264 reordering
> support in the coda driver. For high profile streams, the coda can hold
> back out-of-order frames, causing a few mem2mem device runs in the
> beginning, that don't produce any decompressed buffer at the v4l2
> capture side. At the same time, the last few frames can be decoded from
> the bitstream with mem2mem device runs that don't need a new input
> buffer at the v4l2 output side. A streamoff on the v4l2 output side can
> be used to put the decoder into the ringbuffer draining end-of-stream
> mode.

If I remember correctly the main goal of introducing the m2m framework
was to support simple mem2mem devices where one input buffer = one output
buffer. In other cases m2m was not to be used. 

An example of such mem2mem driver, which does not use m2m framework is
MFC. It uses videobuf2 directly and it is wholly up to the driver how
will it control the queues, stream on/off and so on. You can then have
one OUTPUT buffer generate multiple CAPTURE buffer, multiple OUTPUT
buffers generate a single CAPTURE buffer. Consume OUTPUT buffer without
generating CAPTURE buffer (e.g. when starting decoding) and produce
CAPTURE buffers without consuming OUTPUT buffers (e.g. when finishing
decoding).

I think that stream off should not be used to signal EOS. For this we
have EOS event. You mentioned the EOS buffer flag. This is the idea
originally proposed by Andrzej Hajda, after some lengthy discussions
with v4l community this idea was changed to use an EOS event.

I was all for the EOS buffer flag, but after discussion with Mauro I
understood his arguments. We can get back to this discussion, if we
are sure that events are not enough. Please also note that we need to
keep backward compatibility.

Original EOS buffer flag patches by Andrzej and part of the discussion
can be found here:
1) https://linuxtv.org/patch/10624/
2) https://linuxtv.org/patch/11373/

Best wishes,
Kamil Debski


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Hi Sylwester,

Am Mittwoch, den 29.05.2013, 11:32 +0200 schrieb Sylwester Nawrocki:
> Hello Philip,
> 
> On 05/22/2013 12:17 PM, Philipp Zabel wrote:
> > On mem2mem decoders with a hardware bitstream ringbuffer, to drain the
> > buffer at the end of the stream, remaining frames might need to be decoded
> > without additional input buffers being provided, and after calling streamoff
> > on the v4l2 output queue. This also allows a driver to copy input buffers
> > into their bitstream ringbuffer and immediately mark them as done to be
> > dequeued.
> > 
> > The motivation for this patch is hardware assisted h.264 reordering support
> > in the coda driver. For high profile streams, the coda can hold back
> > out-of-order frames, causing a few mem2mem device runs in the beginning, that
> > don't produce any decompressed buffer at the v4l2 capture side. At the same
> > time, the last few frames can be decoded from the bitstream with mem2mem device
> > runs that don't need a new input buffer at the v4l2 output side. A streamoff
> > on the v4l2 output side can be used to put the decoder into the ringbuffer
> > draining end-of-stream mode.
> >
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > ---
> >  drivers/media/v4l2-core/v4l2-mem2mem.c | 26 ++++++++++++++++++++------
> >  include/media/v4l2-mem2mem.h           |  3 +++
> >  2 files changed, 23 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
> > index 357efa4..52818cd 100644
> > --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> > +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> > @@ -196,6 +196,10 @@ static void v4l2_m2m_try_run(struct v4l2_m2m_dev *m2m_dev)
> >   * 2) at least one destination buffer has to be queued,
> >   * 3) streaming has to be on.
> >   *
> > + * If a queue is buffered (for example a decoder hardware ringbuffer that has
> > + * to be drained before doing streamoff), allow scheduling without v4l2 buffers
> > + * on that queue and even when the queue is not streaming anymore.
> 
> Does it mean you want to be able to queue buffers on e.g. OUTPUT queue, while
> this queue is in STREAM OFF state ?

No.

> Or do you really want to be able to to queue/dequeue buffers on CAPTURE queue,
> while the OUTPUT queue is in STREAM OFF state ?

Yes.

The CODA decoder needs the encoded OUTPUT frames to be copied into a
single bitstream ringbuffer. The OUTPUT buffers can be copied and
dequeued immediately, as long as there is enough space in the bitstream
buffer. Depending on the bitstream type, a few buffers need to be in the
bitstream for the CODA to be able to run. To be able to drain the
bitstream buffer and extract the last few frames, a stream-end bit needs
to be set, after which no new frames can be added to the bitstream
ringbuffer.

To get the last few frames of a stream decoded, I wanted to use
STREAMOFF on the OUTPUT queue (which sets stream-end mode in hardware)
and then continue to call DQBUF on the CAPTURE queue to have the driver
decode the remaining frames. When DQBUF is called on the last frame, the
driver sends the EOS event.

Kamil doesn't want STREAMOFF on the OUTPUT side to be used to put a
decoder into stream-end mode, I'll address that in the other mail.

> >   * There may also be additional, custom requirements. In such case the driver
> >   * should supply a custom callback (job_ready in v4l2_m2m_ops) that should
> >   * return 1 if the instance is ready.
> > @@ -210,7 +214,7 @@ static void v4l2_m2m_try_schedule(struct v4l2_m2m_ctx *m2m_ctx)
> >  	m2m_dev = m2m_ctx->m2m_dev;
> >  	dprintk("Trying to schedule a job for m2m_ctx: %p\n", m2m_ctx);
> >  
> > -	if (!m2m_ctx->out_q_ctx.q.streaming
> > +	if ((!m2m_ctx->out_q_ctx.q.streaming && !m2m_ctx->out_q_ctx.buffered)
> 
> This seems a bit asymmetric. Even if a driver sets 'buffered' on the capture
> queue nothing really changes, right ?

That's only because I had no need of (and no way to test) buffered
capture queues. It should probably be made symmetric.

regards
Philipp

> Thanks,
> Sylwester
> 
> >  	    || !m2m_ctx->cap_q_ctx.q.streaming) {
> >  		dprintk("Streaming needs to be on for both queues\n");
> >  		return;
> > @@ -224,7 +228,7 @@ static void v4l2_m2m_try_schedule(struct v4l2_m2m_ctx *m2m_ctx)
> >  	}
> >  
> >  	spin_lock_irqsave(&m2m_ctx->out_q_ctx.rdy_spinlock, flags);
> > -	if (list_empty(&m2m_ctx->out_q_ctx.rdy_queue)) {
> > +	if (list_empty(&m2m_ctx->out_q_ctx.rdy_queue) && !m2m_ctx->out_q_ctx.buffered) {
> >  		spin_unlock_irqrestore(&m2m_ctx->out_q_ctx.rdy_spinlock, flags);
> >  		spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags_job);
> >  		dprintk("No input buffers available\n");
> > @@ -434,9 +438,11 @@ int v4l2_m2m_streamoff(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
> >  
> >  	m2m_dev = m2m_ctx->m2m_dev;
> >  	spin_lock_irqsave(&m2m_dev->job_spinlock, flags_job);
> > -	/* We should not be scheduled anymore, since we're dropping a queue. */
> > -	INIT_LIST_HEAD(&m2m_ctx->queue);
> > -	m2m_ctx->job_flags = 0;gmane.linux.drivers.video-input-infrastructure	if (list_empty(&m2m_ctx->cap_q_ctx.rdy_queue))
> > +	if (!q_ctx->buffered) {
> > +		/* We should not be scheduled anymore, since we're dropping a queue. */
> > +		INIT_LIST_HEAD(&m2m_ctx->queue);
> > +		m2m_ctx->job_flags = 0;
> > +	}
> >  
> >  	spin_lock_irqsave(&q_ctx->rdy_spinlock, flags);
> >  	/* Drop queue, since streamoff returns device to the same state as after
> > @@ -444,7 +450,7 @@ int v4l2_m2m_streamoff(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
> >  	INIT_LIST_HEAD(&q_ctx->rdy_queue);
> >  	spin_unlock_irqrestore(&q_ctx->rdy_spinlock, flags);
> >  
> > -	if (m2m_dev->curr_ctx == m2m_ctx) {
> > +	if (!q_ctx->buffered && (m2m_dev->curr_ctx == m2m_ctx)) {
> >  		m2m_dev->curr_ctx = NULL;
> >  		wake_up(&m2m_ctx->finished);
> >  	}
> > @@ -640,6 +646,14 @@ err:
> >  }	if (list_empty(&m2m_ctx->cap_q_ctx.rdy_queue))
> >  EXPORT_SYMBOL_GPL(v4l2_m2m_ctx_init);
> >  
> > +void v4l2_m2m_queue_set_buffered(struct vb2_queue *vq)
> > +{
> > +	struct v4l2_m2m_queue_ctx *q_ctx = container_of(vq, struct v4l2_m2m_queue_ctx, q);
> > +
> > +	q_ctx->buffered = true;
> > +}
> > +EXPORT_SYMBOL_GPL(v4l2_m2m_queue_set_buffered);
> > +
> >  /**
> >   * v4l2_m2m_ctx_release() - release m2m context
> >   *
> > diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
> > index 0f4555b..3415845 100644
> > --- a/include/media/v4l2-mem2mem.h
> > +++ b/include/media/v4l2-mem2mem.h
> > @@ -60,6 +60,7 @@ struct v4l2_m2m_queue_ctx {
> >  	struct list_head	rdy_queue;
> >  	spinlock_t		rdy_spinlock;
> >  	u8			num_rdy;
> > +	bool			buffered;
> >  };
> >  
> >  struct v4l2_m2m_ctx {
> > @@ -134,6 +135,8 @@ struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev *m2m_dev,
> >  		void *drv_priv,
> >  		int (*queue_init)(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq));
> >  
> > +void v4l2_m2m_queue_set_buffered(struct vb2_queue *vq);
> > +
> >  void v4l2_m2m_ctx_release(struct v4l2_m2m_ctx *m2m_ctx);
> >  
> >  void v4l2_m2m_buf_queue(struct v4l2_m2m_ctx *m2m_ctx, struct vb2_buffer *vb);
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Hi Kamil,

Am Mittwoch, den 29.05.2013, 11:54 +0200 schrieb Kamil Debski:
> Hi Philipp, Hans,
> 
> > On mem2mem decoders with a hardware bitstream ringbuffer, to drain the
> > buffer at the end of the stream, remaining frames might need to be
> > decoded without additional input buffers being provided, and after
> > calling streamoff on the v4l2 output queue. This also allows a driver
> > to copy input buffers into their bitstream ringbuffer and immediately
> > mark them as done to be dequeued.
> > 
> > The motivation for this patch is hardware assisted h.264 reordering
> > support in the coda driver. For high profile streams, the coda can hold
> > back out-of-order frames, causing a few mem2mem device runs in the
> > beginning, that don't produce any decompressed buffer at the v4l2
> > capture side. At the same time, the last few frames can be decoded from
> > the bitstream with mem2mem device runs that don't need a new input
> > buffer at the v4l2 output side. A streamoff on the v4l2 output side can
> > be used to put the decoder into the ringbuffer draining end-of-stream
> > mode.
> 
> If I remember correctly the main goal of introducing the m2m framework
> was to support simple mem2mem devices where one input buffer = one output
> buffer. In other cases m2m was not to be used. 

The m2m context / queue handling and job scheduling are very useful even
for drivers that don't always produce one CAPTURE buffer from one OUTPUT
buffer, just as you drescribe below.
The CODA encoder path fits the m2m model perfectly. I'd prefer not to
duplicate most of mem2mem just because the decoder doesn't.

There's two things that this patch allows me to do:
a) running mem2mem device_run with an empty OUTPUT queue, which is
   something I'd really like to make possible.
b) running mem2mem device_run with the OUTPUT queue in STREAM OFF, which
   I needed to get the remaining buffers out. But maybe there is a
   better way to do this while keeping the output queue streaming.

> An example of such mem2mem driver, which does not use m2m framework is
> MFC. It uses videobuf2 directly and it is wholly up to the driver how
> will it control the queues, stream on/off and so on. You can then have
> one OUTPUT buffer generate multiple CAPTURE buffer, multiple OUTPUT
> buffers generate a single CAPTURE buffer. Consume OUTPUT buffer without
> generating CAPTURE buffer (e.g. when starting decoding) and produce
> CAPTURE buffers without consuming OUTPUT buffers (e.g. when finishing
> decoding).
>
> I think that stream off should not be used to signal EOS. For this we
> have EOS event. You mentioned the EOS buffer flag. This is the idea
> originally proposed by Andrzej Hajda, after some lengthy discussions
> with v4l community this idea was changed to use an EOS event.

I'm not set on using STREAMOFF to signal the stream-end condition to the
hardware, but after switching to stream-end mode, no new frames should
be queued, so I thought it fit quite well. It also allows to prepare the
OUTPUT buffers (S_FMT/REQBUFS) for the next STREAMON while the CAPTURE
side is still draining the bitstream, although that's probably not a
very useful feature.
I could instead have userspace signal the driver via an EOS buffer flag
or any other mechanism. Then the OUTPUT queue would be kept streaming,
but hold back all buffers queued via QBUF until the last buffer is
dequeued from the CAPTURE queue.

> I was all for the EOS buffer flag, but after discussion with Mauro I
> understood his arguments. We can get back to this discussion, if we
> are sure that events are not enough. Please also note that we need to
> keep backward compatibility.

Maybe I've missed something, but I thought the EOS signal is only for
the driver to signal to userspace that the currently dequeued frame is
the last one?
I need userspace to signal to the driver that it won't queue any new
OUTPUT buffers, but still wants to dequeue the remaining CAPTURE buffers
until the bitstream buffer is empty.

> Original EOS buffer flag patches by Andrzej and part of the discussion
> can be found here:
> 1) https://linuxtv.org/patch/10624/
> 2) https://linuxtv.org/patch/11373/
> 
> Best wishes,
> Kamil Debski

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Hi Philipp,

On 05/29/2013 01:13 PM, Philipp Zabel wrote:
> Hi Kamil,
>
> Am Mittwoch, den 29.05.2013, 11:54 +0200 schrieb Kamil Debski:
>> Hi Philipp, Hans,
>>
>>> On mem2mem decoders with a hardware bitstream ringbuffer, to drain the
>>> buffer at the end of the stream, remaining frames might need to be
>>> decoded without additional input buffers being provided, and after
>>> calling streamoff on the v4l2 output queue. This also allows a driver
>>> to copy input buffers into their bitstream ringbuffer and immediately
>>> mark them as done to be dequeued.
>>>
>>> The motivation for this patch is hardware assisted h.264 reordering
>>> support in the coda driver. For high profile streams, the coda can hold
>>> back out-of-order frames, causing a few mem2mem device runs in the
>>> beginning, that don't produce any decompressed buffer at the v4l2
>>> capture side. At the same time, the last few frames can be decoded from
>>> the bitstream with mem2mem device runs that don't need a new input
>>> buffer at the v4l2 output side. A streamoff on the v4l2 output side can
>>> be used to put the decoder into the ringbuffer draining end-of-stream
>>> mode.
>> If I remember correctly the main goal of introducing the m2m framework
>> was to support simple mem2mem devices where one input buffer = one output
>> buffer. In other cases m2m was not to be used. 
> The m2m context / queue handling and job scheduling are very useful even
> for drivers that don't always produce one CAPTURE buffer from one OUTPUT
> buffer, just as you drescribe below.
> The CODA encoder path fits the m2m model perfectly. I'd prefer not to
> duplicate most of mem2mem just because the decoder doesn't.
>
> There's two things that this patch allows me to do:
> a) running mem2mem device_run with an empty OUTPUT queue, which is
>    something I'd really like to make possible.
> b) running mem2mem device_run with the OUTPUT queue in STREAM OFF, which
>    I needed to get the remaining buffers out. But maybe there is a
>    better way to do this while keeping the output queue streaming.
>
>> An example of such mem2mem driver, which does not use m2m framework is
>> MFC. It uses videobuf2 directly and it is wholly up to the driver how
>> will it control the queues, stream on/off and so on. You can then have
>> one OUTPUT buffer generate multiple CAPTURE buffer, multiple OUTPUT
>> buffers generate a single CAPTURE buffer. Consume OUTPUT buffer without
>> generating CAPTURE buffer (e.g. when starting decoding) and produce
>> CAPTURE buffers without consuming OUTPUT buffers (e.g. when finishing
>> decoding).
>>
>> I think that stream off should not be used to signal EOS. For this we
>> have EOS event. You mentioned the EOS buffer flag. This is the idea
>> originally proposed by Andrzej Hajda, after some lengthy discussions
>> with v4l community this idea was changed to use an EOS event.
> I'm not set on using STREAMOFF to signal the stream-end condition to the
> hardware, but after switching to stream-end mode, no new frames should
> be queued, so I thought it fit quite well. It also allows to prepare the
> OUTPUT buffers (S_FMT/REQBUFS) for the next STREAMON while the CAPTURE
> side is still draining the bitstream, although that's probably not a
> very useful feature.
> I could instead have userspace signal the driver via an EOS buffer flag
> or any other mechanism. Then the OUTPUT queue would be kept streaming,
> but hold back all buffers queued via QBUF until the last buffer is
> dequeued from the CAPTURE queue.
>
>> I was all for the EOS buffer flag, but after discussion with Mauro I
>> understood his arguments. We can get back to this discussion, if we
>> are sure that events are not enough. Please also note that we need to
>> keep backward compatibility.
> Maybe I've missed something, but I thought the EOS signal is only for
> the driver to signal to userspace that the currently dequeued frame is
> the last one?
> I need userspace to signal to the driver that it won't queue any new
> OUTPUT buffers, but still wants to dequeue the remaining CAPTURE buffers
> until the bitstream buffer is empty.
In MFC encoder I have used:
- event V4L2_EVENT_EOS by driver to signal EOS to user,
- VIDIOC_ENCODER_CMD with cmd=V4L2_ENC_CMD_STOP by
user to signal EOS to driver.
It works, but IMO it would look much natural/simpler with EOS buffer flag.

>> Original EOS buffer flag patches by Andrzej and part of the discussion
>> can be found here:
>> 1) https://linuxtv.org/patch/10624/
>> 2) https://linuxtv.org/patch/11373/
>>
>> Best wishes,
>> Kamil Debski
> regards
> Philipp
>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Hi,

> -----Original Message-----
> From: Philipp Zabel [mailto:p.zabel@pengutronix.de]
> Sent: Wednesday, May 29, 2013 1:13 PM
> To: Kamil Debski
> Cc: linux-media@vger.kernel.org; 'Mauro Carvalho Chehab'; 'Pawel
> Osciak'; 'John Sheu'; 'Hans Verkuil'; Marek Szyprowski; Andrzej Hajda
> Subject: Re: [RFC] [media] mem2mem: add support for hardware buffered
> queue
> 
> Hi Kamil,
> 
> Am Mittwoch, den 29.05.2013, 11:54 +0200 schrieb Kamil Debski:
> > Hi Philipp, Hans,
> >
> > > On mem2mem decoders with a hardware bitstream ringbuffer, to drain
> > > the buffer at the end of the stream, remaining frames might need to
> > > be decoded without additional input buffers being provided, and
> > > after calling streamoff on the v4l2 output queue. This also allows
> a
> > > driver to copy input buffers into their bitstream ringbuffer and
> > > immediately mark them as done to be dequeued.
> > >
> > > The motivation for this patch is hardware assisted h.264 reordering
> > > support in the coda driver. For high profile streams, the coda can
> > > hold back out-of-order frames, causing a few mem2mem device runs in
> > > the beginning, that don't produce any decompressed buffer at the
> > > v4l2 capture side. At the same time, the last few frames can be
> > > decoded from the bitstream with mem2mem device runs that don't need
> > > a new input buffer at the v4l2 output side. A streamoff on the v4l2
> > > output side can be used to put the decoder into the ringbuffer
> > > draining end-of-stream mode.
> >
> > If I remember correctly the main goal of introducing the m2m
> framework
> > was to support simple mem2mem devices where one input buffer = one
> > output buffer. In other cases m2m was not to be used.
> 
> The m2m context / queue handling and job scheduling are very useful
> even for drivers that don't always produce one CAPTURE buffer from one
> OUTPUT buffer, just as you drescribe below.
> The CODA encoder path fits the m2m model perfectly. I'd prefer not to
> duplicate most of mem2mem just because the decoder doesn't.
> 
> There's two things that this patch allows me to do:
> a) running mem2mem device_run with an empty OUTPUT queue, which is
>    something I'd really like to make possible.
> b) running mem2mem device_run with the OUTPUT queue in STREAM OFF,
> which
>    I needed to get the remaining buffers out. But maybe there is a
>    better way to do this while keeping the output queue streaming.
> 
> > An example of such mem2mem driver, which does not use m2m framework
> is
> > MFC. It uses videobuf2 directly and it is wholly up to the driver how
> > will it control the queues, stream on/off and so on. You can then
> have
> > one OUTPUT buffer generate multiple CAPTURE buffer, multiple OUTPUT
> > buffers generate a single CAPTURE buffer. Consume OUTPUT buffer
> > without generating CAPTURE buffer (e.g. when starting decoding) and
> > produce CAPTURE buffers without consuming OUTPUT buffers (e.g. when
> > finishing decoding).
> >
> > I think that stream off should not be used to signal EOS. For this we
> > have EOS event. You mentioned the EOS buffer flag. This is the idea
> > originally proposed by Andrzej Hajda, after some lengthy discussions
> > with v4l community this idea was changed to use an EOS event.
> 
> I'm not set on using STREAMOFF to signal the stream-end condition to
> the hardware, but after switching to stream-end mode, no new frames
> should be queued, so I thought it fit quite well. It also allows to
> prepare the OUTPUT buffers (S_FMT/REQBUFS) for the next STREAMON while
> the CAPTURE side is still draining the bitstream, although that's
> probably not a very useful feature.
> I could instead have userspace signal the driver via an EOS buffer flag
> or any other mechanism. Then the OUTPUT queue would be kept streaming,
> but hold back all buffers queued via QBUF until the last buffer is
> dequeued from the CAPTURE queue.
> 
> > I was all for the EOS buffer flag, but after discussion with Mauro I
> > understood his arguments. We can get back to this discussion, if we
> > are sure that events are not enough. Please also note that we need to
> > keep backward compatibility.
> 
> Maybe I've missed something, but I thought the EOS signal is only for
> the driver to signal to userspace that the currently dequeued frame is
> the last one?
> I need userspace to signal to the driver that it won't queue any new
> OUTPUT buffers, but still wants to dequeue the remaining CAPTURE
> buffers until the bitstream buffer is empty.

Right, I mixed that with EOS command. EOS command should be used to
signal end of stream. Also queueing a buffer with bytesused = 0 can
signal that to the driver (this is kept for backward compatibility).

> 
> > Original EOS buffer flag patches by Andrzej and part of the
> discussion
> > can be found here:
> > 1) https://linuxtv.org/patch/10624/
> > 2) https://linuxtv.org/patch/11373/
> >
> > Best wishes,
> > Kamil Debski
> 
> regards
> Philipp

Best wishes,
Kamil


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Hi Andrzej,

Am Mittwoch, den 29.05.2013, 14:05 +0200 schrieb Andrzej Hajda:
> Hi Philipp,
> 
> On 05/29/2013 01:13 PM, Philipp Zabel wrote:
> > Hi Kamil,
> >
> > Am Mittwoch, den 29.05.2013, 11:54 +0200 schrieb Kamil Debski:
> >> Hi Philipp, Hans,
> >>
> >>> On mem2mem decoders with a hardware bitstream ringbuffer, to drain the
> >>> buffer at the end of the stream, remaining frames might need to be
> >>> decoded without additional input buffers being provided, and after
> >>> calling streamoff on the v4l2 output queue. This also allows a driver
> >>> to copy input buffers into their bitstream ringbuffer and immediately
> >>> mark them as done to be dequeued.
> >>>
> >>> The motivation for this patch is hardware assisted h.264 reordering
> >>> support in the coda driver. For high profile streams, the coda can hold
> >>> back out-of-order frames, causing a few mem2mem device runs in the
> >>> beginning, that don't produce any decompressed buffer at the v4l2
> >>> capture side. At the same time, the last few frames can be decoded from
> >>> the bitstream with mem2mem device runs that don't need a new input
> >>> buffer at the v4l2 output side. A streamoff on the v4l2 output side can
> >>> be used to put the decoder into the ringbuffer draining end-of-stream
> >>> mode.
> >> If I remember correctly the main goal of introducing the m2m framework
> >> was to support simple mem2mem devices where one input buffer = one output
> >> buffer. In other cases m2m was not to be used. 
> > The m2m context / queue handling and job scheduling are very useful even
> > for drivers that don't always produce one CAPTURE buffer from one OUTPUT
> > buffer, just as you drescribe below.
> > The CODA encoder path fits the m2m model perfectly. I'd prefer not to
> > duplicate most of mem2mem just because the decoder doesn't.
> >
> > There's two things that this patch allows me to do:
> > a) running mem2mem device_run with an empty OUTPUT queue, which is
> >    something I'd really like to make possible.
> > b) running mem2mem device_run with the OUTPUT queue in STREAM OFF, which
> >    I needed to get the remaining buffers out. But maybe there is a
> >    better way to do this while keeping the output queue streaming.
> >
> >> An example of such mem2mem driver, which does not use m2m framework is
> >> MFC. It uses videobuf2 directly and it is wholly up to the driver how
> >> will it control the queues, stream on/off and so on. You can then have
> >> one OUTPUT buffer generate multiple CAPTURE buffer, multiple OUTPUT
> >> buffers generate a single CAPTURE buffer. Consume OUTPUT buffer without
> >> generating CAPTURE buffer (e.g. when starting decoding) and produce
> >> CAPTURE buffers without consuming OUTPUT buffers (e.g. when finishing
> >> decoding).
> >>
> >> I think that stream off should not be used to signal EOS. For this we
> >> have EOS event. You mentioned the EOS buffer flag. This is the idea
> >> originally proposed by Andrzej Hajda, after some lengthy discussions
> >> with v4l community this idea was changed to use an EOS event.
> > I'm not set on using STREAMOFF to signal the stream-end condition to the
> > hardware, but after switching to stream-end mode, no new frames should
> > be queued, so I thought it fit quite well. It also allows to prepare the
> > OUTPUT buffers (S_FMT/REQBUFS) for the next STREAMON while the CAPTURE
> > side is still draining the bitstream, although that's probably not a
> > very useful feature.
> > I could instead have userspace signal the driver via an EOS buffer flag
> > or any other mechanism. Then the OUTPUT queue would be kept streaming,
> > but hold back all buffers queued via QBUF until the last buffer is
> > dequeued from the CAPTURE queue.
> >
> >> I was all for the EOS buffer flag, but after discussion with Mauro I
> >> understood his arguments. We can get back to this discussion, if we
> >> are sure that events are not enough. Please also note that we need to
> >> keep backward compatibility.
> > Maybe I've missed something, but I thought the EOS signal is only for
> > the driver to signal to userspace that the currently dequeued frame is
> > the last one?
> > I need userspace to signal to the driver that it won't queue any new
> > OUTPUT buffers, but still wants to dequeue the remaining CAPTURE buffers
> > until the bitstream buffer is empty.
> In MFC encoder I have used:
> - event V4L2_EVENT_EOS by driver to signal EOS to user,
> - VIDIOC_ENCODER_CMD with cmd=V4L2_ENC_CMD_STOP by
> user to signal EOS to driver.
> It works, but IMO it would look much natural/simpler with EOS buffer flag.

Ok, thank you. I agree the buffer flag feels more natural, but this will
work. I'll use VIDIOC_DECODER_CMD with cmd=V4L2_DEC_CMD_STOP and
V4L2_EVENT_EOS for this.

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html