Re: [RFC PATCH 2/2] [media] videobuf2: return -EPIPE from DQBUF after the last buffer

2015-02-04 Thread Philipp Zabel 
Hi Laurent,

Am Dienstag, den 03.02.2015, 13:20 +0200 schrieb Laurent Pinchart:
 On Monday 02 February 2015 15:00:51 Hans Verkuil wrote:
  On 01/22/2015 12:28 PM, 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.
  
  This looks OK to me, although I would like to see comments from others as
  well. Of course, this needs to be documented in the spec as well.
 
 I haven't followed the V4L2 codec API discussion during ELC-E. Could someone 
 briefly expose the rationale for this and the codec draining flow ? The 
 explanation should probably included in the documentation.

This is the draining flow as written down in the notes:

  The decoder draining flow starts with the application making the
  driver aware of its intentions by calling VIDIOC_DECODER_CMD with
  V4L2_DEC_CMD_STOP.

  The driver processes the remaining buffers on the OUTPUT queue.
  Once all CAPTURE buffers produced from those are ready to dequeue, it 
  sends a V4L2_EVENT_EOS.

  The application then dequeues buffers from the CAPTURE queue until it
  encounters a buffer with V4L2_BUF_FLAG_LAST set. This buffer may
  already be empty due to hardware limitations.
  Alternatively, the application may simply continue to dequeue
  (possibly empty) buffers until the VIDIOC_DQBUF ioctl returns -EPIPE.

  To resume decoding, the application can call VIDIOC_DECODER_CMD with
  V4L2_CMD_START without the need to stop streaming.

This should work the same for encoding.
The rationale for using -EPIPE was IIRC that it allows for simpler code
(DQBUF loop until error). Also it works without the newest kernel
headers that #define V4L2_BUF_FLAG_LAST.

I don't remember if we talked about what should happen to new buffers
queued via QBUF on the OUTPUT queue while in draining mode, but I
suppose the driver should just hold them until V4L2_CMD_START is called.

 Existing applications will receive -EPIPE from DQBUF now. Have potential 
 breakages been taken into account ?

We can continue to produce empty frames this way, which are currently
used to signal the last frame in s5p-mfc. I am not aware of other
unspecified mechanisms in use.

regards
Philipp

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

Re: [RFC PATCH 2/2] [media] videobuf2: return -EPIPE from DQBUF after the last buffer

2015-02-03 Thread Laurent Pinchart 
On Monday 02 February 2015 15:00:51 Hans Verkuil wrote:
 On 01/22/2015 12:28 PM, 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.
 
 This looks OK to me, although I would like to see comments from others as
 well. Of course, this needs to be documented in the spec as well.

I haven't followed the V4L2 codec API discussion during ELC-E. Could someone 
briefly expose the rationale for this and the codec draining flow ? The 
explanation should probably included in the documentation.

Existing applications will receive -EPIPE from DQBUF now. Have potential 
breakages been taken into account ?

  Signed-off-by: Philipp Zabel p.za...@pengutronix.de
  ---
  TODO: (How) should the last_buffer_dequeud flag be cleared in reaction to
  V4L2_DEC_CMD_START?
 
 I would suggest an inline function in videobuf2-core.h that clears the flag
 and that drivers can call. I don't think the vb2 core can detect when it is
 OK to clear the flag, it needs to be told by the driver (correct me if I am
 wrong).

-- 
Regards,

Laurent Pinchart

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

Re: [RFC PATCH 2/2] [media] videobuf2: return -EPIPE from DQBUF after the last buffer

2015-02-02 Thread Hans Verkuil 
On 01/22/2015 12:28 PM, 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.

This looks OK to me, although I would like to see comments from others as well.
Of course, this needs to be documented in the spec as well.

 
 Signed-off-by: Philipp Zabel p.za...@pengutronix.de
 ---
 TODO: (How) should the last_buffer_dequeud flag be cleared in reaction to
 V4L2_DEC_CMD_START?

I would suggest an inline function in videobuf2-core.h that clears the flag
and that drivers can call. I don't think the vb2 core can detect when it is
OK to clear the flag, it needs to be told by the driver (correct me if I am
wrong).

Regards,

Hans

 ---
  drivers/media/v4l2-core/v4l2-mem2mem.c   | 10 +-
  drivers/media/v4l2-core/videobuf2-core.c | 18 +-
  include/media/videobuf2-core.h   |  1 +
  3 files changed, 27 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 d09a891..c2c2eac 100644
 --- a/drivers/media/v4l2-core/videobuf2-core.c
 +++ b/drivers/media/v4l2-core/videobuf2-core.c
 @@ -2046,6 +2046,10 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, 
 struct v4l2_buffer *b, bool n
   struct vb2_buffer *vb = NULL;
   int ret;
  
 + if (q-last_buffer_dequeued) {
 + dprintk(3, last buffer dequeued already\n);
 + return -EPIPE;
 + }
   if (b-type != q-type) {
   dprintk(1, invalid buffer type\n);
   return -EINVAL;
 @@ -2073,6 +2077,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 +2293,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 +2636,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 (!V4L2_TYPE_IS_OUTPUT(q-type)  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..ca337bf 100644
 --- a/include/media/videobuf2-core.h
 +++ b/include/media/videobuf2-core.h
 @@ -429,6 +429,7 @@ struct vb2_queue {
   unsigned intstart_streaming_called:1;
   unsigned interror:1;
   unsigned intwaiting_for_buffers:1;
 + unsigned intlast_buffer_dequeued:1;
  
   struct vb2_fileio_data  *fileio;
   struct vb2_threadio_data*threadio;
 

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

Re: [RFC PATCH 2/2] [media] videobuf2: return -EPIPE from DQBUF after the last buffer

2015-02-02 Thread Philipp Zabel 
Am Montag, den 02.02.2015, 15:00 +0100 schrieb Hans Verkuil:
 On 01/22/2015 12:28 PM, 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.
 
 This looks OK to me, although I would like to see comments from others as 
 well.
 Of course, this needs to be documented in the spec as well.

Thanks, I'll fix that in the next round.

  Signed-off-by: Philipp Zabel p.za...@pengutronix.de
  ---
  TODO: (How) should the last_buffer_dequeud flag be cleared in reaction to
  V4L2_DEC_CMD_START?
 
 I would suggest an inline function in videobuf2-core.h that clears the flag
 and that drivers can call. I don't think the vb2 core can detect when it is
 OK to clear the flag, it needs to be told by the driver (correct me if I am
 wrong).

No, I think you are right that this should be done explicitly. I'll add
an inline function next time.

regards
Philipp

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

[RFC PATCH 2/2] [media] videobuf2: return -EPIPE from DQBUF after the last buffer

2015-01-22 Thread 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.

Signed-off-by: Philipp Zabel p.za...@pengutronix.de
---
TODO: (How) should the last_buffer_dequeud flag be cleared in reaction to
V4L2_DEC_CMD_START?
---
 drivers/media/v4l2-core/v4l2-mem2mem.c   | 10 +-
 drivers/media/v4l2-core/videobuf2-core.c | 18 +-
 include/media/videobuf2-core.h   |  1 +
 3 files changed, 27 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 d09a891..c2c2eac 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -2046,6 +2046,10 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, 
struct v4l2_buffer *b, bool n
struct vb2_buffer *vb = NULL;
int ret;
 
+   if (q-last_buffer_dequeued) {
+   dprintk(3, last buffer dequeued already\n);
+   return -EPIPE;
+   }
if (b-type != q-type) {
dprintk(1, invalid buffer type\n);
return -EINVAL;
@@ -2073,6 +2077,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 +2293,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 +2636,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 (!V4L2_TYPE_IS_OUTPUT(q-type)  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..ca337bf 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -429,6 +429,7 @@ struct vb2_queue {
unsigned intstart_streaming_called:1;
unsigned interror:1;
unsigned intwaiting_for_buffers:1;
+   unsigned intlast_buffer_dequeued:1;
 
struct vb2_fileio_data  *fileio;
struct vb2_threadio_data*threadio;
-- 
2.1.4

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