[PATCH v5 0/5] Signalling last decoded frame by V4L2_BUF_FLAG_LAST and -EPIPE

[PATCH v5 0/5] Signalling last decoded frame by V4L2_BUF_FLAG_LAST and -EPIPE

[Philipp Zabel]

At the V4L2 codec API session during ELC-E 2014, we agreed that for the decoder
draining flow, after a V4L2_DEC_CMD_STOP decoder command was issued, the last
decoded buffer should get dequeued with a V4L2_BUF_FLAG_LAST set. After that,
poll should immediately return and all following VIDIOC_DQBUF should return
-EPIPE until the stream is stopped or decoding continued via V4L2_DEC_CMD_START.
(or STREAMOFF/STREAMON).

Changes since v4:
 - Split documentation changes back out into the first patch.
 - Changes according to Pawel's feedback, except for the POLLHUP suggestion.

regards
Philipp

Peter Seiderer (1):
  [media] videodev2: Add V4L2_BUF_FLAG_LAST

Philipp Zabel (4):
  [media] DocBook media: document mem2mem draining flow
  [media] videobuf2: return -EPIPE from DQBUF after the last buffer
  [media] coda: Set last buffer flag and fix EOS event
  [media] s5p-mfc: Set last buffer flag

 Documentation/DocBook/media/v4l/io.xml             | 10 ++++++++
 .../DocBook/media/v4l/vidioc-decoder-cmd.xml       |  9 +++++++-
 .../DocBook/media/v4l/vidioc-encoder-cmd.xml       |  8 ++++++-
 Documentation/DocBook/media/v4l/vidioc-qbuf.xml    |  8 +++++++
 drivers/media/platform/coda/coda-bit.c             |  4 ++--
 drivers/media/platform/coda/coda-common.c          | 27 +++++++++-------------
 drivers/media/platform/coda/coda.h                 |  3 +++
 drivers/media/platform/s5p-mfc/s5p_mfc.c           |  1 +
 drivers/media/v4l2-core/v4l2-mem2mem.c             | 10 +++++++-
 drivers/media/v4l2-core/videobuf2-core.c           | 19 ++++++++++++++-
 include/media/videobuf2-core.h                     | 13 +++++++++++
 include/trace/events/v4l2.h                        |  3 ++-
 include/uapi/linux/videodev2.h                     |  2 ++
 13 files changed, 94 insertions(+), 23 deletions(-)

-- 
2.1.4

[PATCH v5 1/5] [media] DocBook media: document mem2mem draining flow

[Philipp Zabel]

Document the interaction between VIDIOC_DECODER_CMD V4L2_DEC_CMD_STOP and
VIDIOC_ENCODER_CMD V4L2_ENC_CMD_STOP to start the draining, the V4L2_EVENT_EOS
event signalling all capture buffers are finished and ready to be dequeud,
the new V4L2_BUF_FLAG_LAST buffer flag indicating the last buffer being dequeued
from the capture queue, and the poll and VIDIOC_DQBUF ioctl return values once
the queue is drained.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
Changes since v4:
 - Split out documentation changes into a separate patch
 - Changed wording following Pawel's suggestions.
---
 Documentation/DocBook/media/v4l/io.xml                 | 10 ++++++++++
 Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml |  9 ++++++++-
 Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml |  8 +++++++-
 Documentation/DocBook/media/v4l/vidioc-qbuf.xml        |  8 ++++++++
 4 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
index 1c17f80..f561037 100644
--- a/Documentation/DocBook/media/v4l/io.xml
+++ b/Documentation/DocBook/media/v4l/io.xml
@@ -1129,6 +1129,16 @@ in this buffer has not been created by the CPU but by some DMA-capable unit,
 in which case caches have not been used.</entry>
 	  </row>
 	  <row>
+	    <entry><constant>V4L2_BUF_FLAG_LAST</constant></entry>
+	    <entry>0x00100000</entry>
+	    <entry>Last buffer produced by the hardware. mem2mem codec drivers
+set this flag on the capture queue for the last buffer when the
+<link linkend="vidioc-querybuf">VIDIOC_QUERYBUF</link> or
+<link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl is called. Any subsequent
+call to the <link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl will not block
+anymore, but return an &EPIPE;.</entry>
+	  </row>
+	  <row>
 	    <entry><constant>V4L2_BUF_FLAG_TIMESTAMP_MASK</constant></entry>
 	    <entry>0x0000e000</entry>
 	    <entry>Mask for timestamp types below. To test the
diff --git a/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml b/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml
index 9215627..6502d82 100644
--- a/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml
@@ -197,7 +197,14 @@ be muted when playing back at a non-standard speed.
 this command does nothing. This command has two flags:
 if <constant>V4L2_DEC_CMD_STOP_TO_BLACK</constant> is set, then the decoder will
 set the picture to black after it stopped decoding. Otherwise the last image will
-repeat. If <constant>V4L2_DEC_CMD_STOP_IMMEDIATELY</constant> is set, then the decoder
+repeat. mem2mem decoders will stop producing new frames altogether. They will send
+a <constant>V4L2_EVENT_EOS</constant> event when the last frame has been decoded
+and all frames are ready to be dequeued and will set the
+<constant>V4L2_BUF_FLAG_LAST</constant> buffer flag on the last buffer of the
+capture queue to indicate there will be no new buffers produced to dequeue. Once
+this flag was set, the <link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl
+will not block anymore, but return an &EPIPE;.
+If <constant>V4L2_DEC_CMD_STOP_IMMEDIATELY</constant> is set, then the decoder
 stops immediately (ignoring the <structfield>pts</structfield> value), otherwise it
 will keep decoding until timestamp >= pts or until the last of the pending data from
 its internal buffers was decoded.
diff --git a/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml b/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml
index 0619ca5..3cdb841 100644
--- a/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml
@@ -129,7 +129,13 @@ this command.</entry>
 encoding will continue until the end of the current <wordasword>Group
 Of Pictures</wordasword>, otherwise encoding will stop immediately.
 When the encoder is already stopped, this command does
-nothing.</entry>
+nothing. mem2mem encoders will send a <constant>V4L2_EVENT_EOS</constant> event
+when the last frame has been decoded and all frames are ready to be dequeued and
+will set the <constant>V4L2_BUF_FLAG_LAST</constant> buffer flag on the last
+buffer of the capture queue to indicate there will be no new buffers produced to
+dequeue. Once this flag was set, the
+<link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl will not block anymore,
+but return an &EPIPE;.</entry>
 	  </row>
 	  <row>
 	    <entry><constant>V4L2_ENC_CMD_PAUSE</constant></entry>
diff --git a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
index 3504a7f..6cfc53b 100644
--- a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
@@ -186,6 +186,14 @@ In that case the application should be able to safely reuse the buffer and
 continue streaming.
 	</para>
 	</listitem>
+	<term><errorcode>EPIPE</errorcode></term>
+	<listitem>
+	  <para><constant>VIDIOC_DQBUF</constant> returns this on an empty
+capture queue for mem2mem codecs if a buffer with the
+<constant>V4L2_BUF_FLAG_LAST</constant> was already dequeued and no new buffers
+are expected to become available.
+	</para>
+	</listitem>
       </varlistentry>
     </variablelist>
   </refsect1>
-- 
2.1.4

[PATCH v5 2/5] [media] videodev2: Add V4L2_BUF_FLAG_LAST

[Philipp Zabel]

From: Peter Seiderer <ps.report@gmx.net>

This v4l2_buffer flag can be used by drivers to mark a capture buffer
as the last generated buffer, for example after a V4L2_DEC_CMD_STOP
command was issued.

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
Changes since v4:
 - Split out DocBook changes into a separate patch.
---
 include/trace/events/v4l2.h    | 3 ++-
 include/uapi/linux/videodev2.h | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/trace/events/v4l2.h b/include/trace/events/v4l2.h
index b9bb1f2..32c33aa 100644
--- a/include/trace/events/v4l2.h
+++ b/include/trace/events/v4l2.h
@@ -58,7 +58,8 @@
 		{ V4L2_BUF_FLAG_TIMESTAMP_MASK,	     "TIMESTAMP_MASK" },      \
 		{ V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN,   "TIMESTAMP_UNKNOWN" },   \
 		{ V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC, "TIMESTAMP_MONOTONIC" }, \
-		{ V4L2_BUF_FLAG_TIMESTAMP_COPY,	     "TIMESTAMP_COPY" })
+		{ V4L2_BUF_FLAG_TIMESTAMP_COPY,	     "TIMESTAMP_COPY" },      \
+		{ V4L2_BUF_FLAG_LAST,                "LAST" })
 
 #define show_timecode_flags(flags)					  \
 	__print_flags(flags, "|",					  \
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index fbdc360..c642c10 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -809,6 +809,8 @@ struct v4l2_buffer {
 #define V4L2_BUF_FLAG_TSTAMP_SRC_MASK		0x00070000
 #define V4L2_BUF_FLAG_TSTAMP_SRC_EOF		0x00000000
 #define V4L2_BUF_FLAG_TSTAMP_SRC_SOE		0x00010000
+/* mem2mem encoder/decoder */
+#define V4L2_BUF_FLAG_LAST			0x00100000
 
 /**
  * struct v4l2_exportbuffer - export of video buffer as DMABUF file descriptor
-- 
2.1.4

[PATCH v5 3/5] [media] videobuf2: return -EPIPE from DQBUF after the last buffer

[Philipp Zabel]

If the last buffer was dequeued from a capture queue, let poll return
immediately and let DQBUF return -EPIPE to signal there will no more
buffers to dequeue until STREAMOFF.
The driver signals the last buffer by setting the V4L2_BUF_FLAG_LAST.
To reenable dequeuing on the capture queue, the driver must explicitly
call vb2_clear_last_buffer_queued. The last buffer queued flag is
cleared automatically during STREAMOFF.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
Changes since v4:
 - Split out DocBook changes into a separate patch.
 - Documented last_buffer_dequeued flag in vb2_queue comment.
 - Moved last_buffer_dequeued check from vb2_internal_dqbuf
   into __vb2_wait_for_done_vb.
 - Dropped superfluous check in vb2_poll.
---
 drivers/media/v4l2-core/v4l2-mem2mem.c   | 10 +++++++++-
 drivers/media/v4l2-core/videobuf2-core.c | 19 ++++++++++++++++++-
 include/media/videobuf2-core.h           | 13 +++++++++++++
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
index 80c588f..1b5b432 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -564,8 +564,16 @@ unsigned int v4l2_m2m_poll(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
 
 	if (list_empty(&src_q->done_list))
 		poll_wait(file, &src_q->done_wq, wait);
-	if (list_empty(&dst_q->done_list))
+	if (list_empty(&dst_q->done_list)) {
+		/*
+		 * If the last buffer was dequeued from the capture queue,
+		 * return immediately. DQBUF will return -EPIPE.
+		 */
+		if (dst_q->last_buffer_dequeued)
+			return rc | POLLIN | POLLRDNORM;
+
 		poll_wait(file, &dst_q->done_wq, wait);
+	}
 
 	if (m2m_ctx->m2m_dev->m2m_ops->lock)
 		m2m_ctx->m2m_dev->m2m_ops->lock(m2m_ctx->priv);
diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index cc16e76..3b2188e 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1918,6 +1918,11 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
 			return -EIO;
 		}
 
+		if (q->last_buffer_dequeued) {
+			dprintk(3, "last buffer dequeued already, will not wait for buffers\n");
+			return -EPIPE;
+		}
+
 		if (!list_empty(&q->done_list)) {
 			/*
 			 * Found a buffer that we were waiting for.
@@ -2073,6 +2078,9 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n
 	/* Remove from videobuf queue */
 	list_del(&vb->queued_entry);
 	q->queued_count--;
+	if (!V4L2_TYPE_IS_OUTPUT(q->type) &&
+	    vb->v4l2_buf.flags & V4L2_BUF_FLAG_LAST)
+		q->last_buffer_dequeued = true;
 	/* go back to dequeued state */
 	__vb2_dqbuf(vb);
 
@@ -2286,6 +2294,7 @@ static int vb2_internal_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
 	 */
 	__vb2_queue_cancel(q);
 	q->waiting_for_buffers = !V4L2_TYPE_IS_OUTPUT(q->type);
+	q->last_buffer_dequeued = false;
 
 	dprintk(3, "successful\n");
 	return 0;
@@ -2628,8 +2637,16 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait)
 	if (V4L2_TYPE_IS_OUTPUT(q->type) && q->queued_count < q->num_buffers)
 		return res | POLLOUT | POLLWRNORM;
 
-	if (list_empty(&q->done_list))
+	if (list_empty(&q->done_list)) {
+		/*
+		 * If the last buffer was dequeued from a capture queue,
+		 * return immediately. DQBUF will return -EPIPE.
+		 */
+		if (q->last_buffer_dequeued)
+			return res | POLLIN | POLLRDNORM;
+
 		poll_wait(file, &q->done_wq, wait);
+	}
 
 	/*
 	 * Take first buffer available for dequeuing.
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index bd2cec2..a689d25 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -390,6 +390,9 @@ struct v4l2_fh;
  * @waiting_for_buffers: used in poll() to check if vb2 is still waiting for
  *		buffers. Only set for capture queues if qbuf has not yet been
  *		called since poll() needs to return POLLERR in that situation.
+ * @last_buffer_dequeued: used in poll() and DQBUF to immediately return if the
+ *		last decoded buffer was already dequeued. Set for capture queues
+ *		when a buffer with the V4L2_BUF_FLAG_LAST is dequeued.
  * @fileio:	file io emulator internal data, used only if emulator is active
  * @threadio:	thread io internal data, used only if thread is active
  */
@@ -429,6 +432,7 @@ struct vb2_queue {
 	unsigned int			start_streaming_called:1;
 	unsigned int			error:1;
 	unsigned int			waiting_for_buffers:1;
+	unsigned int			last_buffer_dequeued:1;
 
 	struct vb2_fileio_data		*fileio;
 	struct vb2_threadio_data	*threadio;
@@ -609,6 +613,15 @@ static inline bool vb2_start_streaming_called(struct vb2_queue *q)
 	return q->start_streaming_called;
 }
 
+/**
+ * vb2_clear_last_buffer_dequeued() - clear last buffer dequeued flag of queue
+ * @q:		videobuf queue
+ */
+static inline void vb2_clear_last_buffer_dequeued(struct vb2_queue *q)
+{
+	q->last_buffer_dequeued = false;
+}
+
 /*
  * The following functions are not part of the vb2 core API, but are simple
  * helper functions that you can use in your struct v4l2_file_operations,
-- 
2.1.4

[PATCH v5 4/5] [media] coda: Set last buffer flag and fix EOS event

[Philipp Zabel]

Setting the last buffer flag causes the videobuf2 core to return -EPIPE from
DQBUF calls on the capture queue after the last buffer is dequeued.
This patch also fixes the EOS event to conform to the specification. It now is
sent right after the last buffer has been decoded instead of when the last
buffer is dequeued.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/platform/coda/coda-bit.c    |  4 ++--
 drivers/media/platform/coda/coda-common.c | 27 +++++++++++----------------
 drivers/media/platform/coda/coda.h        |  3 +++
 3 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/drivers/media/platform/coda/coda-bit.c b/drivers/media/platform/coda/coda-bit.c
index 856b542..9ae0bfa 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -1278,7 +1278,7 @@ static void coda_finish_encode(struct coda_ctx *ctx)
 	v4l2_m2m_buf_done(src_buf, VB2_BUF_STATE_DONE);
 
 	dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
-	v4l2_m2m_buf_done(dst_buf, VB2_BUF_STATE_DONE);
+	coda_m2m_buf_done(ctx, dst_buf, VB2_BUF_STATE_DONE);
 
 	ctx->gopcounter--;
 	if (ctx->gopcounter < 0)
@@ -1887,7 +1887,7 @@ static void coda_finish_decode(struct coda_ctx *ctx)
 		}
 		vb2_set_plane_payload(dst_buf, 0, payload);
 
-		v4l2_m2m_buf_done(dst_buf, ctx->frame_errors[display_idx] ?
+		coda_m2m_buf_done(ctx, dst_buf, ctx->frame_errors[display_idx] ?
 				  VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
 
 		v4l2_dbg(1, coda_debug, &dev->v4l2_dev,
diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
index 6f32e6d..f178ad3 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -705,35 +705,30 @@ static int coda_qbuf(struct file *file, void *priv,
 }
 
 static bool coda_buf_is_end_of_stream(struct coda_ctx *ctx,
-				      struct v4l2_buffer *buf)
+				      struct vb2_buffer *buf)
 {
 	struct vb2_queue *src_vq;
 
 	src_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
 
 	return ((ctx->bit_stream_param & CODA_BIT_STREAM_END_FLAG) &&
-		(buf->sequence == (ctx->qsequence - 1)));
+		(buf->v4l2_buf.sequence == (ctx->qsequence - 1)));
 }
 
-static int coda_dqbuf(struct file *file, void *priv,
-		      struct v4l2_buffer *buf)
+void coda_m2m_buf_done(struct coda_ctx *ctx, struct vb2_buffer *buf,
+		       enum vb2_buffer_state state)
 {
-	struct coda_ctx *ctx = fh_to_ctx(priv);
-	int ret;
+	const struct v4l2_event eos_event = {
+		.type = V4L2_EVENT_EOS
+	};
 
-	ret = v4l2_m2m_dqbuf(file, ctx->fh.m2m_ctx, buf);
-
-	/* If this is the last capture buffer, emit an end-of-stream event */
-	if (buf->type == V4L2_BUF_TYPE_VIDEO_CAPTURE &&
-	    coda_buf_is_end_of_stream(ctx, buf)) {
-		const struct v4l2_event eos_event = {
-			.type = V4L2_EVENT_EOS
-		};
+	if (coda_buf_is_end_of_stream(ctx, buf)) {
+		buf->v4l2_buf.flags |= V4L2_BUF_FLAG_LAST;
 
 		v4l2_event_queue_fh(&ctx->fh, &eos_event);
 	}
 
-	return ret;
+	v4l2_m2m_buf_done(buf, state);
 }
 
 static int coda_g_selection(struct file *file, void *fh,
@@ -846,7 +841,7 @@ static const struct v4l2_ioctl_ops coda_ioctl_ops = {
 
 	.vidioc_qbuf		= coda_qbuf,
 	.vidioc_expbuf		= v4l2_m2m_ioctl_expbuf,
-	.vidioc_dqbuf		= coda_dqbuf,
+	.vidioc_dqbuf		= v4l2_m2m_ioctl_dqbuf,
 	.vidioc_create_bufs	= v4l2_m2m_ioctl_create_bufs,
 
 	.vidioc_streamon	= v4l2_m2m_ioctl_streamon,
diff --git a/drivers/media/platform/coda/coda.h b/drivers/media/platform/coda/coda.h
index 0c35cd5..420de18 100644
--- a/drivers/media/platform/coda/coda.h
+++ b/drivers/media/platform/coda/coda.h
@@ -291,6 +291,9 @@ static inline int coda_get_bitstream_payload(struct coda_ctx *ctx)
 
 void coda_bit_stream_end_flag(struct coda_ctx *ctx);
 
+void coda_m2m_buf_done(struct coda_ctx *ctx, struct vb2_buffer *buf,
+		       enum vb2_buffer_state state);
+
 int coda_h264_padding(int size, char *p);
 
 bool coda_jpeg_check_buffer(struct coda_ctx *ctx, struct vb2_buffer *vb);
-- 
2.1.4

[PATCH v5 5/5] [media] s5p-mfc: Set last buffer flag

[Philipp Zabel]

Setting the last buffer flag causes the videobuf2 core to return -EPIPE from
DQBUF calls on the capture queue after the last buffer is dequeued.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 98374e8..04dbeef 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -211,6 +211,7 @@ static void s5p_mfc_handle_frame_all_extracted(struct s5p_mfc_ctx *ctx)
 			dst_buf->b->v4l2_buf.field = V4L2_FIELD_NONE;
 		else
 			dst_buf->b->v4l2_buf.field = V4L2_FIELD_INTERLACED;
+		dst_buf->b->v4l2_buf.flags |= V4L2_BUF_FLAG_LAST;
 
 		ctx->dec_dst_flag &= ~(1 << dst_buf->b->v4l2_buf.index);
 		vb2_buffer_done(dst_buf->b, VB2_BUF_STATE_DONE);
-- 
2.1.4

Re: [PATCH v5 1/5] [media] DocBook media: document mem2mem draining flow

[Hans Verkuil]

Hi Philipp,

I finally got around to reviewing this patch series. Sorry for the delay, but
here are my comments:

On 04/20/2015 10:28 AM, Philipp Zabel wrote:
> Document the interaction between VIDIOC_DECODER_CMD V4L2_DEC_CMD_STOP and
> VIDIOC_ENCODER_CMD V4L2_ENC_CMD_STOP to start the draining, the V4L2_EVENT_EOS
> event signalling all capture buffers are finished and ready to be dequeud,
> the new V4L2_BUF_FLAG_LAST buffer flag indicating the last buffer being dequeued
> from the capture queue, and the poll and VIDIOC_DQBUF ioctl return values once
> the queue is drained.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
> Changes since v4:
>  - Split out documentation changes into a separate patch
>  - Changed wording following Pawel's suggestions.
> ---
>  Documentation/DocBook/media/v4l/io.xml                 | 10 ++++++++++
>  Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml |  9 ++++++++-
>  Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml |  8 +++++++-
>  Documentation/DocBook/media/v4l/vidioc-qbuf.xml        |  8 ++++++++
>  4 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
> index 1c17f80..f561037 100644
> --- a/Documentation/DocBook/media/v4l/io.xml
> +++ b/Documentation/DocBook/media/v4l/io.xml
> @@ -1129,6 +1129,16 @@ in this buffer has not been created by the CPU but by some DMA-capable unit,
>  in which case caches have not been used.</entry>
>  	  </row>
>  	  <row>
> +	    <entry><constant>V4L2_BUF_FLAG_LAST</constant></entry>
> +	    <entry>0x00100000</entry>
> +	    <entry>Last buffer produced by the hardware. mem2mem codec drivers
> +set this flag on the capture queue for the last buffer when the
> +<link linkend="vidioc-querybuf">VIDIOC_QUERYBUF</link> or
> +<link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl is called. Any subsequent
> +call to the <link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl will not block
> +anymore, but return an &EPIPE;.</entry>

As Kamil mentioned in his review, we should allow for bytesused == 0 here.

> +	  </row>
> +	  <row>
>  	    <entry><constant>V4L2_BUF_FLAG_TIMESTAMP_MASK</constant></entry>
>  	    <entry>0x0000e000</entry>
>  	    <entry>Mask for timestamp types below. To test the
> diff --git a/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml b/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml
> index 9215627..6502d82 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml
> @@ -197,7 +197,14 @@ be muted when playing back at a non-standard speed.
>  this command does nothing. This command has two flags:
>  if <constant>V4L2_DEC_CMD_STOP_TO_BLACK</constant> is set, then the decoder will
>  set the picture to black after it stopped decoding. Otherwise the last image will
> -repeat. If <constant>V4L2_DEC_CMD_STOP_IMMEDIATELY</constant> is set, then the decoder
> +repeat. mem2mem decoders will stop producing new frames altogether. They will send
> +a <constant>V4L2_EVENT_EOS</constant> event when the last frame has been decoded
> +and all frames are ready to be dequeued and will set the
> +<constant>V4L2_BUF_FLAG_LAST</constant> buffer flag on the last buffer of the

Make a note here as well that the last buffer might be an empty buffer.

> +capture queue to indicate there will be no new buffers produced to dequeue. Once
> +this flag was set, the <link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl
> +will not block anymore, but return an &EPIPE;.
> +If <constant>V4L2_DEC_CMD_STOP_IMMEDIATELY</constant> is set, then the decoder
>  stops immediately (ignoring the <structfield>pts</structfield> value), otherwise it
>  will keep decoding until timestamp >= pts or until the last of the pending data from
>  its internal buffers was decoded.
> diff --git a/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml b/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml
> index 0619ca5..3cdb841 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml
> @@ -129,7 +129,13 @@ this command.</entry>
>  encoding will continue until the end of the current <wordasword>Group
>  Of Pictures</wordasword>, otherwise encoding will stop immediately.
>  When the encoder is already stopped, this command does
> -nothing.</entry>
> +nothing. mem2mem encoders will send a <constant>V4L2_EVENT_EOS</constant> event
> +when the last frame has been decoded and all frames are ready to be dequeued and
> +will set the <constant>V4L2_BUF_FLAG_LAST</constant> buffer flag on the last
> +buffer of the capture queue to indicate there will be no new buffers produced to

Ditto.

> +dequeue. Once this flag was set, the
> +<link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl will not block anymore,
> +but return an &EPIPE;.</entry>
>  	  </row>
>  	  <row>
>  	    <entry><constant>V4L2_ENC_CMD_PAUSE</constant></entry>
> diff --git a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
> index 3504a7f..6cfc53b 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
> @@ -186,6 +186,14 @@ In that case the application should be able to safely reuse the buffer and
>  continue streaming.
>  	</para>
>  	</listitem>
> +	<term><errorcode>EPIPE</errorcode></term>
> +	<listitem>
> +	  <para><constant>VIDIOC_DQBUF</constant> returns this on an empty
> +capture queue for mem2mem codecs if a buffer with the
> +<constant>V4L2_BUF_FLAG_LAST</constant> was already dequeued and no new buffers
> +are expected to become available.
> +	</para>
> +	</listitem>
>        </varlistentry>
>      </variablelist>
>    </refsect1>
> 

Regards,

	Hans

Re: [PATCH v5 1/5] [media] DocBook media: document mem2mem draining flow

[Hans Verkuil]

On 04/20/2015 10:28 AM, Philipp Zabel wrote:
> Document the interaction between VIDIOC_DECODER_CMD V4L2_DEC_CMD_STOP and
> VIDIOC_ENCODER_CMD V4L2_ENC_CMD_STOP to start the draining, the V4L2_EVENT_EOS
> event signalling all capture buffers are finished and ready to be dequeud,
> the new V4L2_BUF_FLAG_LAST buffer flag indicating the last buffer being dequeued
> from the capture queue, and the poll and VIDIOC_DQBUF ioctl return values once
> the queue is drained.

Actually, we document the m2m draining flow, but the same thing is true for
MPEG capture devices like hdpvr that support ENCODER_CMD. So I don't think this
is specific to m2m devices.

Regards,

	Hans

> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
> Changes since v4:
>  - Split out documentation changes into a separate patch
>  - Changed wording following Pawel's suggestions.
> ---
>  Documentation/DocBook/media/v4l/io.xml                 | 10 ++++++++++
>  Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml |  9 ++++++++-
>  Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml |  8 +++++++-
>  Documentation/DocBook/media/v4l/vidioc-qbuf.xml        |  8 ++++++++
>  4 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
> index 1c17f80..f561037 100644
> --- a/Documentation/DocBook/media/v4l/io.xml
> +++ b/Documentation/DocBook/media/v4l/io.xml
> @@ -1129,6 +1129,16 @@ in this buffer has not been created by the CPU but by some DMA-capable unit,
>  in which case caches have not been used.</entry>
>  	  </row>
>  	  <row>
> +	    <entry><constant>V4L2_BUF_FLAG_LAST</constant></entry>
> +	    <entry>0x00100000</entry>
> +	    <entry>Last buffer produced by the hardware. mem2mem codec drivers
> +set this flag on the capture queue for the last buffer when the
> +<link linkend="vidioc-querybuf">VIDIOC_QUERYBUF</link> or
> +<link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl is called. Any subsequent
> +call to the <link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl will not block
> +anymore, but return an &EPIPE;.</entry>
> +	  </row>
> +	  <row>
>  	    <entry><constant>V4L2_BUF_FLAG_TIMESTAMP_MASK</constant></entry>
>  	    <entry>0x0000e000</entry>
>  	    <entry>Mask for timestamp types below. To test the
> diff --git a/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml b/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml
> index 9215627..6502d82 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml
> @@ -197,7 +197,14 @@ be muted when playing back at a non-standard speed.
>  this command does nothing. This command has two flags:
>  if <constant>V4L2_DEC_CMD_STOP_TO_BLACK</constant> is set, then the decoder will
>  set the picture to black after it stopped decoding. Otherwise the last image will
> -repeat. If <constant>V4L2_DEC_CMD_STOP_IMMEDIATELY</constant> is set, then the decoder
> +repeat. mem2mem decoders will stop producing new frames altogether. They will send
> +a <constant>V4L2_EVENT_EOS</constant> event when the last frame has been decoded
> +and all frames are ready to be dequeued and will set the
> +<constant>V4L2_BUF_FLAG_LAST</constant> buffer flag on the last buffer of the
> +capture queue to indicate there will be no new buffers produced to dequeue. Once
> +this flag was set, the <link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl
> +will not block anymore, but return an &EPIPE;.
> +If <constant>V4L2_DEC_CMD_STOP_IMMEDIATELY</constant> is set, then the decoder
>  stops immediately (ignoring the <structfield>pts</structfield> value), otherwise it
>  will keep decoding until timestamp >= pts or until the last of the pending data from
>  its internal buffers was decoded.
> diff --git a/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml b/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml
> index 0619ca5..3cdb841 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml
> @@ -129,7 +129,13 @@ this command.</entry>
>  encoding will continue until the end of the current <wordasword>Group
>  Of Pictures</wordasword>, otherwise encoding will stop immediately.
>  When the encoder is already stopped, this command does
> -nothing.</entry>
> +nothing. mem2mem encoders will send a <constant>V4L2_EVENT_EOS</constant> event
> +when the last frame has been decoded and all frames are ready to be dequeued and
> +will set the <constant>V4L2_BUF_FLAG_LAST</constant> buffer flag on the last
> +buffer of the capture queue to indicate there will be no new buffers produced to
> +dequeue. Once this flag was set, the
> +<link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl will not block anymore,
> +but return an &EPIPE;.</entry>
>  	  </row>
>  	  <row>
>  	    <entry><constant>V4L2_ENC_CMD_PAUSE</constant></entry>
> diff --git a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
> index 3504a7f..6cfc53b 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
> @@ -186,6 +186,14 @@ In that case the application should be able to safely reuse the buffer and
>  continue streaming.
>  	</para>
>  	</listitem>
> +	<term><errorcode>EPIPE</errorcode></term>
> +	<listitem>
> +	  <para><constant>VIDIOC_DQBUF</constant> returns this on an empty
> +capture queue for mem2mem codecs if a buffer with the
> +<constant>V4L2_BUF_FLAG_LAST</constant> was already dequeued and no new buffers
> +are expected to become available.
> +	</para>
> +	</listitem>
>        </varlistentry>
>      </variablelist>
>    </refsect1>
> 

Re: [PATCH v5 2/5] [media] videodev2: Add V4L2_BUF_FLAG_LAST

[Hans Verkuil]

On 04/20/2015 10:28 AM, Philipp Zabel wrote:
> From: Peter Seiderer <ps.report@gmx.net>
> 
> This v4l2_buffer flag can be used by drivers to mark a capture buffer
> as the last generated buffer, for example after a V4L2_DEC_CMD_STOP
> command was issued.
> 
> Signed-off-by: Peter Seiderer <ps.report@gmx.net>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Thanks!

	Hans

> ---
> Changes since v4:
>  - Split out DocBook changes into a separate patch.
> ---
>  include/trace/events/v4l2.h    | 3 ++-
>  include/uapi/linux/videodev2.h | 2 ++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/include/trace/events/v4l2.h b/include/trace/events/v4l2.h
> index b9bb1f2..32c33aa 100644
> --- a/include/trace/events/v4l2.h
> +++ b/include/trace/events/v4l2.h
> @@ -58,7 +58,8 @@
>  		{ V4L2_BUF_FLAG_TIMESTAMP_MASK,	     "TIMESTAMP_MASK" },      \
>  		{ V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN,   "TIMESTAMP_UNKNOWN" },   \
>  		{ V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC, "TIMESTAMP_MONOTONIC" }, \
> -		{ V4L2_BUF_FLAG_TIMESTAMP_COPY,	     "TIMESTAMP_COPY" })
> +		{ V4L2_BUF_FLAG_TIMESTAMP_COPY,	     "TIMESTAMP_COPY" },      \
> +		{ V4L2_BUF_FLAG_LAST,                "LAST" })
>  
>  #define show_timecode_flags(flags)					  \
>  	__print_flags(flags, "|",					  \
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index fbdc360..c642c10 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -809,6 +809,8 @@ struct v4l2_buffer {
>  #define V4L2_BUF_FLAG_TSTAMP_SRC_MASK		0x00070000
>  #define V4L2_BUF_FLAG_TSTAMP_SRC_EOF		0x00000000
>  #define V4L2_BUF_FLAG_TSTAMP_SRC_SOE		0x00010000
> +/* mem2mem encoder/decoder */
> +#define V4L2_BUF_FLAG_LAST			0x00100000
>  
>  /**
>   * struct v4l2_exportbuffer - export of video buffer as DMABUF file descriptor
> 

Re: [PATCH v5 3/5] [media] videobuf2: return -EPIPE from DQBUF after the last buffer

[Hans Verkuil]

On 04/20/2015 10:28 AM, Philipp Zabel wrote:
> If the last buffer was dequeued from a capture queue, let poll return
> immediately and let DQBUF return -EPIPE to signal there will no more
> buffers to dequeue until STREAMOFF.
> The driver signals the last buffer by setting the V4L2_BUF_FLAG_LAST.
> To reenable dequeuing on the capture queue, the driver must explicitly
> call vb2_clear_last_buffer_queued. The last buffer queued flag is
> cleared automatically during STREAMOFF.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Thanks!

	Hans


> ---
> Changes since v4:
>  - Split out DocBook changes into a separate patch.
>  - Documented last_buffer_dequeued flag in vb2_queue comment.
>  - Moved last_buffer_dequeued check from vb2_internal_dqbuf
>    into __vb2_wait_for_done_vb.
>  - Dropped superfluous check in vb2_poll.
> ---
>  drivers/media/v4l2-core/v4l2-mem2mem.c   | 10 +++++++++-
>  drivers/media/v4l2-core/videobuf2-core.c | 19 ++++++++++++++++++-
>  include/media/videobuf2-core.h           | 13 +++++++++++++
>  3 files changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
> index 80c588f..1b5b432 100644
> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> @@ -564,8 +564,16 @@ unsigned int v4l2_m2m_poll(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
>  
>  	if (list_empty(&src_q->done_list))
>  		poll_wait(file, &src_q->done_wq, wait);
> -	if (list_empty(&dst_q->done_list))
> +	if (list_empty(&dst_q->done_list)) {
> +		/*
> +		 * If the last buffer was dequeued from the capture queue,
> +		 * return immediately. DQBUF will return -EPIPE.
> +		 */
> +		if (dst_q->last_buffer_dequeued)
> +			return rc | POLLIN | POLLRDNORM;
> +
>  		poll_wait(file, &dst_q->done_wq, wait);
> +	}
>  
>  	if (m2m_ctx->m2m_dev->m2m_ops->lock)
>  		m2m_ctx->m2m_dev->m2m_ops->lock(m2m_ctx->priv);
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> index cc16e76..3b2188e 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -1918,6 +1918,11 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
>  			return -EIO;
>  		}
>  
> +		if (q->last_buffer_dequeued) {
> +			dprintk(3, "last buffer dequeued already, will not wait for buffers\n");
> +			return -EPIPE;
> +		}
> +
>  		if (!list_empty(&q->done_list)) {
>  			/*
>  			 * Found a buffer that we were waiting for.
> @@ -2073,6 +2078,9 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n
>  	/* Remove from videobuf queue */
>  	list_del(&vb->queued_entry);
>  	q->queued_count--;
> +	if (!V4L2_TYPE_IS_OUTPUT(q->type) &&
> +	    vb->v4l2_buf.flags & V4L2_BUF_FLAG_LAST)
> +		q->last_buffer_dequeued = true;
>  	/* go back to dequeued state */
>  	__vb2_dqbuf(vb);
>  
> @@ -2286,6 +2294,7 @@ static int vb2_internal_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
>  	 */
>  	__vb2_queue_cancel(q);
>  	q->waiting_for_buffers = !V4L2_TYPE_IS_OUTPUT(q->type);
> +	q->last_buffer_dequeued = false;
>  
>  	dprintk(3, "successful\n");
>  	return 0;
> @@ -2628,8 +2637,16 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait)
>  	if (V4L2_TYPE_IS_OUTPUT(q->type) && q->queued_count < q->num_buffers)
>  		return res | POLLOUT | POLLWRNORM;
>  
> -	if (list_empty(&q->done_list))
> +	if (list_empty(&q->done_list)) {
> +		/*
> +		 * If the last buffer was dequeued from a capture queue,
> +		 * return immediately. DQBUF will return -EPIPE.
> +		 */
> +		if (q->last_buffer_dequeued)
> +			return res | POLLIN | POLLRDNORM;
> +
>  		poll_wait(file, &q->done_wq, wait);
> +	}
>  
>  	/*
>  	 * Take first buffer available for dequeuing.
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index bd2cec2..a689d25 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -390,6 +390,9 @@ struct v4l2_fh;
>   * @waiting_for_buffers: used in poll() to check if vb2 is still waiting for
>   *		buffers. Only set for capture queues if qbuf has not yet been
>   *		called since poll() needs to return POLLERR in that situation.
> + * @last_buffer_dequeued: used in poll() and DQBUF to immediately return if the
> + *		last decoded buffer was already dequeued. Set for capture queues
> + *		when a buffer with the V4L2_BUF_FLAG_LAST is dequeued.
>   * @fileio:	file io emulator internal data, used only if emulator is active
>   * @threadio:	thread io internal data, used only if thread is active
>   */
> @@ -429,6 +432,7 @@ struct vb2_queue {
>  	unsigned int			start_streaming_called:1;
>  	unsigned int			error:1;
>  	unsigned int			waiting_for_buffers:1;
> +	unsigned int			last_buffer_dequeued:1;
>  
>  	struct vb2_fileio_data		*fileio;
>  	struct vb2_threadio_data	*threadio;
> @@ -609,6 +613,15 @@ static inline bool vb2_start_streaming_called(struct vb2_queue *q)
>  	return q->start_streaming_called;
>  }
>  
> +/**
> + * vb2_clear_last_buffer_dequeued() - clear last buffer dequeued flag of queue
> + * @q:		videobuf queue
> + */
> +static inline void vb2_clear_last_buffer_dequeued(struct vb2_queue *q)
> +{
> +	q->last_buffer_dequeued = false;
> +}
> +
>  /*
>   * The following functions are not part of the vb2 core API, but are simple
>   * helper functions that you can use in your struct v4l2_file_operations,
> 

Re: [PATCH v5 1/5] [media] DocBook media: document mem2mem draining flow

[Philipp Zabel]

Hi Hans, Kamil,

thank you for your comments.

Am Montag, den 27.04.2015, 15:23 +0200 schrieb Hans Verkuil:
> Hi Philipp,
> 
> I finally got around to reviewing this patch series. Sorry for the delay, but
> here are my comments:
>
> On 04/20/2015 10:28 AM, Philipp Zabel wrote:
> > Document the interaction between VIDIOC_DECODER_CMD V4L2_DEC_CMD_STOP and
> > VIDIOC_ENCODER_CMD V4L2_ENC_CMD_STOP to start the draining, the V4L2_EVENT_EOS
> > event signalling all capture buffers are finished and ready to be dequeud,
> > the new V4L2_BUF_FLAG_LAST buffer flag indicating the last buffer being dequeued
> > from the capture queue, and the poll and VIDIOC_DQBUF ioctl return values once
> > the queue is drained.
> > 
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > ---
> > Changes since v4:
> >  - Split out documentation changes into a separate patch
> >  - Changed wording following Pawel's suggestions.
> > ---
> >  Documentation/DocBook/media/v4l/io.xml                 | 10 ++++++++++
> >  Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml |  9 ++++++++-
> >  Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml |  8 +++++++-
> >  Documentation/DocBook/media/v4l/vidioc-qbuf.xml        |  8 ++++++++
> >  4 files changed, 33 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
> > index 1c17f80..f561037 100644
> > --- a/Documentation/DocBook/media/v4l/io.xml
> > +++ b/Documentation/DocBook/media/v4l/io.xml
> > @@ -1129,6 +1129,16 @@ in this buffer has not been created by the CPU but by some DMA-capable unit,
> >  in which case caches have not been used.</entry>
> >  	  </row>
> >  	  <row>
> > +	    <entry><constant>V4L2_BUF_FLAG_LAST</constant></entry>
> > +	    <entry>0x00100000</entry>
> > +	    <entry>Last buffer produced by the hardware. mem2mem codec drivers
> > +set this flag on the capture queue for the last buffer when the
> > +<link linkend="vidioc-querybuf">VIDIOC_QUERYBUF</link> or
> > +<link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl is called. Any subsequent
> > +call to the <link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl will not block
> > +anymore, but return an &EPIPE;.</entry>
> 
> As Kamil mentioned in his review, we should allow for bytesused == 0 here.

How about:

@@ -1134,9 +1134,11 @@ in which case caches have not been used.</entry>
            <entry>Last buffer produced by the hardware. mem2mem codec drivers
 set this flag on the capture queue for the last buffer when the
 <link linkend="vidioc-querybuf">VIDIOC_QUERYBUF</link> or
-<link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl is called. Any subsequent
-call to the <link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl will not block
-anymore, but return an &EPIPE;.</entry>
+<link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl is called. Due to hardware
+limitations, the last buffer may be empty. In this case the driver will set the
+<structfield>bytesused</structfield> field to 0, regardless of the format. Any
+Any subsequent call to the <link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl
+will not block anymore, but return an &EPIPE;.</entry>
          </row>
          <row>
            <entry><constant>V4L2_BUF_FLAG_TIMESTAMP_MASK</constant></entry>

> > +	  </row>
> > +	  <row>
> >  	    <entry><constant>V4L2_BUF_FLAG_TIMESTAMP_MASK</constant></entry>
> >  	    <entry>0x0000e000</entry>
> >  	    <entry>Mask for timestamp types below. To test the
> > diff --git a/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml b/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml
> > index 9215627..6502d82 100644
> > --- a/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml
> > +++ b/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml
> > @@ -197,7 +197,14 @@ be muted when playing back at a non-standard speed.
> >  this command does nothing. This command has two flags:
> >  if <constant>V4L2_DEC_CMD_STOP_TO_BLACK</constant> is set, then the decoder will
> >  set the picture to black after it stopped decoding. Otherwise the last image will
> > -repeat. If <constant>V4L2_DEC_CMD_STOP_IMMEDIATELY</constant> is set, then the decoder
> > +repeat. mem2mem decoders will stop producing new frames altogether. They will send
> > +a <constant>V4L2_EVENT_EOS</constant> event when the last frame has been decoded
> > +and all frames are ready to be dequeued and will set the
> > +<constant>V4L2_BUF_FLAG_LAST</constant> buffer flag on the last buffer of the
> 
> Make a note here as well that the last buffer might be an empty buffer.

@@ -201,9 +201,12 @@ repeat. mem2mem decoders will stop producing new frames altogether. They will se
 a <constant>V4L2_EVENT_EOS</constant> event when the last frame has been decoded
 and all frames are ready to be dequeued and will set the
 <constant>V4L2_BUF_FLAG_LAST</constant> buffer flag on the last buffer of the
-capture queue to indicate there will be no new buffers produced to dequeue. Once
-this flag was set, the <link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl
-will not block anymore, but return an &EPIPE;.
+capture queue to indicate there will be no new buffers produced to dequeue. This
+buffer may be empty, indicated by the driver setting the
+<structfield>bytesused</structfield> field to 0. Once the
+<constant>V4L2_BUF_FLAG_LAST</constant> flag was set, the
+<link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl will not block anymore,
+but return an &EPIPE;.
 If <constant>V4L2_DEC_CMD_STOP_IMMEDIATELY</constant> is set, then the decoder
 stops immediately (ignoring the <structfield>pts</structfield> value), otherwise it
 will keep decoding until timestamp >= pts or until the last of the pending data from

> > +capture queue to indicate there will be no new buffers produced to dequeue. Once
> > +this flag was set, the <link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl
> > +will not block anymore, but return an &EPIPE;.
> > +If <constant>V4L2_DEC_CMD_STOP_IMMEDIATELY</constant> is set, then the decoder
> >  stops immediately (ignoring the <structfield>pts</structfield> value), otherwise it
> >  will keep decoding until timestamp >= pts or until the last of the pending data from
> >  its internal buffers was decoded.
> > diff --git a/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml b/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml
> > index 0619ca5..3cdb841 100644
> > --- a/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml
> > +++ b/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml
> > @@ -129,7 +129,13 @@ this command.</entry>
> >  encoding will continue until the end of the current <wordasword>Group
> >  Of Pictures</wordasword>, otherwise encoding will stop immediately.
> >  When the encoder is already stopped, this command does
> > -nothing.</entry>
> > +nothing. mem2mem encoders will send a <constant>V4L2_EVENT_EOS</constant> event
> > +when the last frame has been decoded and all frames are ready to be dequeued and
> > +will set the <constant>V4L2_BUF_FLAG_LAST</constant> buffer flag on the last
> > +buffer of the capture queue to indicate there will be no new buffers produced to
> 
> Ditto.

@@ -133,7 +133,9 @@ nothing. mem2mem encoders will send a <constant>V4L2_EVENT_EOS</constant> event
 when the last frame has been decoded and all frames are ready to be dequeued and
 will set the <constant>V4L2_BUF_FLAG_LAST</constant> buffer flag on the last
 buffer of the capture queue to indicate there will be no new buffers produced to
-dequeue. Once this flag was set, the
+dequeue. This buffer may be empty, indicated by the driver setting the
+<structfield>bytesused</structfield> field to 0. Once the
+<constant>V4L2_BUF_FLAG_LAST</constant> flag was set, the
 <link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl will not block anymore,
 but return an &EPIPE;.</entry>
          </row>

regards
Philipp

Re: [PATCH v5 1/5] [media] DocBook media: document mem2mem draining flow

[Hans Verkuil]

On 05/04/2015 12:21 PM, Philipp Zabel wrote:
> Hi Hans, Kamil,
> 
> thank you for your comments.
> 
> Am Montag, den 27.04.2015, 15:23 +0200 schrieb Hans Verkuil:
>> Hi Philipp,
>>
>> I finally got around to reviewing this patch series. Sorry for the delay, but
>> here are my comments:
>>
>> On 04/20/2015 10:28 AM, Philipp Zabel wrote:
>>> Document the interaction between VIDIOC_DECODER_CMD V4L2_DEC_CMD_STOP and
>>> VIDIOC_ENCODER_CMD V4L2_ENC_CMD_STOP to start the draining, the V4L2_EVENT_EOS
>>> event signalling all capture buffers are finished and ready to be dequeud,
>>> the new V4L2_BUF_FLAG_LAST buffer flag indicating the last buffer being dequeued
>>> from the capture queue, and the poll and VIDIOC_DQBUF ioctl return values once
>>> the queue is drained.
>>>
>>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>>> ---
>>> Changes since v4:
>>>  - Split out documentation changes into a separate patch
>>>  - Changed wording following Pawel's suggestions.
>>> ---
>>>  Documentation/DocBook/media/v4l/io.xml                 | 10 ++++++++++
>>>  Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml |  9 ++++++++-
>>>  Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml |  8 +++++++-
>>>  Documentation/DocBook/media/v4l/vidioc-qbuf.xml        |  8 ++++++++
>>>  4 files changed, 33 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
>>> index 1c17f80..f561037 100644
>>> --- a/Documentation/DocBook/media/v4l/io.xml
>>> +++ b/Documentation/DocBook/media/v4l/io.xml
>>> @@ -1129,6 +1129,16 @@ in this buffer has not been created by the CPU but by some DMA-capable unit,
>>>  in which case caches have not been used.</entry>
>>>  	  </row>
>>>  	  <row>
>>> +	    <entry><constant>V4L2_BUF_FLAG_LAST</constant></entry>
>>> +	    <entry>0x00100000</entry>
>>> +	    <entry>Last buffer produced by the hardware. mem2mem codec drivers
>>> +set this flag on the capture queue for the last buffer when the
>>> +<link linkend="vidioc-querybuf">VIDIOC_QUERYBUF</link> or
>>> +<link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl is called. Any subsequent
>>> +call to the <link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl will not block
>>> +anymore, but return an &EPIPE;.</entry>
>>
>> As Kamil mentioned in his review, we should allow for bytesused == 0 here.
> 
> How about:
> 
> @@ -1134,9 +1134,11 @@ in which case caches have not been used.</entry>
>             <entry>Last buffer produced by the hardware. mem2mem codec drivers
>  set this flag on the capture queue for the last buffer when the
>  <link linkend="vidioc-querybuf">VIDIOC_QUERYBUF</link> or
> -<link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl is called. Any subsequent
> -call to the <link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl will not block
> -anymore, but return an &EPIPE;.</entry>
> +<link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl is called. Due to hardware
> +limitations, the last buffer may be empty. In this case the driver will set the
> +<structfield>bytesused</structfield> field to 0, regardless of the format. Any
> +Any subsequent call to the <link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl
> +will not block anymore, but return an &EPIPE;.</entry>
>           </row>
>           <row>
>             <entry><constant>V4L2_BUF_FLAG_TIMESTAMP_MASK</constant></entry>
> 
>>> +	  </row>
>>> +	  <row>
>>>  	    <entry><constant>V4L2_BUF_FLAG_TIMESTAMP_MASK</constant></entry>
>>>  	    <entry>0x0000e000</entry>
>>>  	    <entry>Mask for timestamp types below. To test the
>>> diff --git a/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml b/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml
>>> index 9215627..6502d82 100644
>>> --- a/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml
>>> +++ b/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml
>>> @@ -197,7 +197,14 @@ be muted when playing back at a non-standard speed.
>>>  this command does nothing. This command has two flags:
>>>  if <constant>V4L2_DEC_CMD_STOP_TO_BLACK</constant> is set, then the decoder will
>>>  set the picture to black after it stopped decoding. Otherwise the last image will
>>> -repeat. If <constant>V4L2_DEC_CMD_STOP_IMMEDIATELY</constant> is set, then the decoder
>>> +repeat. mem2mem decoders will stop producing new frames altogether. They will send
>>> +a <constant>V4L2_EVENT_EOS</constant> event when the last frame has been decoded
>>> +and all frames are ready to be dequeued and will set the
>>> +<constant>V4L2_BUF_FLAG_LAST</constant> buffer flag on the last buffer of the
>>
>> Make a note here as well that the last buffer might be an empty buffer.
> 
> @@ -201,9 +201,12 @@ repeat. mem2mem decoders will stop producing new frames altogether. They will se
>  a <constant>V4L2_EVENT_EOS</constant> event when the last frame has been decoded
>  and all frames are ready to be dequeued and will set the
>  <constant>V4L2_BUF_FLAG_LAST</constant> buffer flag on the last buffer of the
> -capture queue to indicate there will be no new buffers produced to dequeue. Once
> -this flag was set, the <link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl
> -will not block anymore, but return an &EPIPE;.
> +capture queue to indicate there will be no new buffers produced to dequeue. This
> +buffer may be empty, indicated by the driver setting the
> +<structfield>bytesused</structfield> field to 0. Once the
> +<constant>V4L2_BUF_FLAG_LAST</constant> flag was set, the
> +<link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl will not block anymore,
> +but return an &EPIPE;.
>  If <constant>V4L2_DEC_CMD_STOP_IMMEDIATELY</constant> is set, then the decoder
>  stops immediately (ignoring the <structfield>pts</structfield> value), otherwise it
>  will keep decoding until timestamp >= pts or until the last of the pending data from
> 
>>> +capture queue to indicate there will be no new buffers produced to dequeue. Once
>>> +this flag was set, the <link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl
>>> +will not block anymore, but return an &EPIPE;.
>>> +If <constant>V4L2_DEC_CMD_STOP_IMMEDIATELY</constant> is set, then the decoder
>>>  stops immediately (ignoring the <structfield>pts</structfield> value), otherwise it
>>>  will keep decoding until timestamp >= pts or until the last of the pending data from
>>>  its internal buffers was decoded.
>>> diff --git a/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml b/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml
>>> index 0619ca5..3cdb841 100644
>>> --- a/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml
>>> +++ b/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml
>>> @@ -129,7 +129,13 @@ this command.</entry>
>>>  encoding will continue until the end of the current <wordasword>Group
>>>  Of Pictures</wordasword>, otherwise encoding will stop immediately.
>>>  When the encoder is already stopped, this command does
>>> -nothing.</entry>
>>> +nothing. mem2mem encoders will send a <constant>V4L2_EVENT_EOS</constant> event
>>> +when the last frame has been decoded and all frames are ready to be dequeued and
>>> +will set the <constant>V4L2_BUF_FLAG_LAST</constant> buffer flag on the last
>>> +buffer of the capture queue to indicate there will be no new buffers produced to
>>
>> Ditto.
> 
> @@ -133,7 +133,9 @@ nothing. mem2mem encoders will send a <constant>V4L2_EVENT_EOS</constant> event
>  when the last frame has been decoded and all frames are ready to be dequeued and
>  will set the <constant>V4L2_BUF_FLAG_LAST</constant> buffer flag on the last
>  buffer of the capture queue to indicate there will be no new buffers produced to
> -dequeue. Once this flag was set, the
> +dequeue. This buffer may be empty, indicated by the driver setting the
> +<structfield>bytesused</structfield> field to 0. Once the
> +<constant>V4L2_BUF_FLAG_LAST</constant> flag was set, the
>  <link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl will not block anymore,
>  but return an &EPIPE;.</entry>
>           </row>

Looks good to me. With this change:

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

> 
> 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
>