* [PATCH v5 1/4] vb2: split the io_flags member of vb2_queue into a bit field
@ 2015-02-20 16:38 Kamil Debski
2015-02-20 16:38 ` [PATCH v5 2/4] vb2: add allow_zero_bytesused flag to the vb2_queue struct Kamil Debski
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Kamil Debski @ 2015-02-20 16:38 UTC (permalink / raw)
To: linux-media; +Cc: m.szyprowski, k.debski, hverkuil
This patch splits the io_flags member of vb2_queue into a bit field.
Instead of an enum with flags separate bit fields were introduced.
Signed-off-by: Kamil Debski <k.debski@samsung.com>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/media/v4l2-core/videobuf2-core.c | 17 +++++++++--------
include/media/videobuf2-core.h | 18 +++++-------------
2 files changed, 14 insertions(+), 21 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index bc08a82..5cd60bf 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -2760,7 +2760,8 @@ struct vb2_fileio_data {
unsigned int initial_index;
unsigned int q_count;
unsigned int dq_count;
- unsigned int flags;
+ unsigned read_once:1;
+ unsigned write_immediately:1;
};
/**
@@ -2798,14 +2799,16 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read)
*/
count = 1;
- dprintk(3, "setting up file io: mode %s, count %d, flags %08x\n",
- (read) ? "read" : "write", count, q->io_flags);
+ dprintk(3, "setting up file io: mode %s, count %d, read_once %d, write_immediately %d\n",
+ (read) ? "read" : "write", count, q->fileio_read_once,
+ q->fileio_write_immediately);
fileio = kzalloc(sizeof(struct vb2_fileio_data), GFP_KERNEL);
if (fileio == NULL)
return -ENOMEM;
- fileio->flags = q->io_flags;
+ fileio->read_once = q->fileio_read_once;
+ fileio->write_immediately = q->fileio_write_immediately;
/*
* Request buffers and use MMAP type to force driver
@@ -3028,13 +3031,11 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
/*
* Queue next buffer if required.
*/
- if (buf->pos == buf->size ||
- (!read && (fileio->flags & VB2_FILEIO_WRITE_IMMEDIATELY))) {
+ if (buf->pos == buf->size || (!read && fileio->write_immediately)) {
/*
* Check if this is the last buffer to read.
*/
- if (read && (fileio->flags & VB2_FILEIO_READ_ONCE) &&
- fileio->dq_count == 1) {
+ if (read && fileio->read_once && fileio->dq_count == 1) {
dprintk(3, "read limit reached\n");
return __vb2_cleanup_fileio(q);
}
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index bd2cec2..e49dc6b 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -134,17 +134,6 @@ enum vb2_io_modes {
};
/**
- * enum vb2_fileio_flags - flags for selecting a mode of the file io emulator,
- * by default the 'streaming' style is used by the file io emulator
- * @VB2_FILEIO_READ_ONCE: report EOF after reading the first buffer
- * @VB2_FILEIO_WRITE_IMMEDIATELY: queue buffer after each write() call
- */
-enum vb2_fileio_flags {
- VB2_FILEIO_READ_ONCE = (1 << 0),
- VB2_FILEIO_WRITE_IMMEDIATELY = (1 << 1),
-};
-
-/**
* enum vb2_buffer_state - current video buffer state
* @VB2_BUF_STATE_DEQUEUED: buffer under userspace control
* @VB2_BUF_STATE_PREPARING: buffer is being prepared in videobuf
@@ -346,7 +335,8 @@ struct v4l2_fh;
*
* @type: queue type (see V4L2_BUF_TYPE_* in linux/videodev2.h
* @io_modes: supported io methods (see vb2_io_modes enum)
- * @io_flags: additional io flags (see vb2_fileio_flags enum)
+ * @fileio_read_once: report EOF after reading the first buffer
+ * @fileio_write_immediately: queue buffer after each write() call
* @lock: pointer to a mutex that protects the vb2_queue struct. The
* driver can set this to a mutex to let the v4l2 core serialize
* the queuing ioctls. If the driver wants to handle locking
@@ -396,7 +386,9 @@ struct v4l2_fh;
struct vb2_queue {
enum v4l2_buf_type type;
unsigned int io_modes;
- unsigned int io_flags;
+ unsigned fileio_read_once:1;
+ unsigned fileio_write_immediately:1;
+
struct mutex *lock;
struct v4l2_fh *owner;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v5 2/4] vb2: add allow_zero_bytesused flag to the vb2_queue struct
2015-02-20 16:38 [PATCH v5 1/4] vb2: split the io_flags member of vb2_queue into a bit field Kamil Debski
@ 2015-02-20 16:38 ` Kamil Debski
2015-02-20 16:52 ` Hans Verkuil
2015-02-20 16:38 ` [PATCH v5 3/4] coda: set allow_zero_bytesused flag for vb2_queue_init Kamil Debski
2015-02-20 16:38 ` [PATCH v5 4/4] s5p-mfc: " Kamil Debski
2 siblings, 1 reply; 9+ messages in thread
From: Kamil Debski @ 2015-02-20 16:38 UTC (permalink / raw)
To: linux-media; +Cc: m.szyprowski, k.debski, hverkuil
The vb2: fix bytesused == 0 handling (8a75ffb) patch changed the behavior
of __fill_vb2_buffer function, so that if bytesused is 0 it is set to the
size of the buffer. However, bytesused set to 0 is used by older codec
drivers as as indication used to mark the end of stream.
To keep backward compatibility, this patch adds a flag passed to the
vb2_queue_init function - allow_zero_bytesused. If the flag is set upon
initialization of the queue, the videobuf2 keeps the value of bytesused
intact in the OUTPUT queue and passes it to the driver.
Reported-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Signed-off-by: Kamil Debski <k.debski@samsung.com>
---
drivers/media/v4l2-core/videobuf2-core.c | 39 +++++++++++++++++++++++++-----
include/media/videobuf2-core.h | 2 ++
2 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 5cd60bf..5d77670 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1247,6 +1247,16 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
{
unsigned int plane;
+ if (V4L2_TYPE_IS_OUTPUT(b->type)) {
+ if (WARN_ON_ONCE(b->bytesused == 0)) {
+ pr_warn_once("use of bytesused == 0 is deprecated and will be removed in 2017,\n");
+ if (vb->vb2_queue->allow_zero_bytesused)
+ pr_warn_once("use VIDIOC_DECODER_CMD(V4L2_DEC_CMD_STOP) instead.\n");
+ else
+ pr_warn_once("use the actual size instead.\n");
+ }
+ }
+
if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
if (b->memory == V4L2_MEMORY_USERPTR) {
for (plane = 0; plane < vb->num_planes; ++plane) {
@@ -1276,13 +1286,22 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
* userspace clearly never bothered to set it and
* it's a safe assumption that they really meant to
* use the full plane sizes.
+ *
+ * Some drivers, e.g. old codec drivers, use bytesused
+ * == 0 as a way to indicate that streaming is finished.
+ * In that case, the driver should use the
+ * allow_zero_bytesused flag to keep old userspace
+ * applications working.
*/
for (plane = 0; plane < vb->num_planes; ++plane) {
struct v4l2_plane *pdst = &v4l2_planes[plane];
struct v4l2_plane *psrc = &b->m.planes[plane];
- pdst->bytesused = psrc->bytesused ?
- psrc->bytesused : pdst->length;
+ if (vb->vb2_queue->allow_zero_bytesused)
+ pdst->bytesused = psrc->bytesused;
+ else
+ pdst->bytesused = psrc->bytesused ?
+ psrc->bytesused : pdst->length;
pdst->data_offset = psrc->data_offset;
}
}
@@ -1295,6 +1314,11 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
*
* If bytesused == 0 for the output buffer, then fall back
* to the full buffer size as that's a sensible default.
+ *
+ * Some drivers, e.g. old codec drivers, use bytesused * == 0 as
+ * a way to indicate that streaming is finished. In that case,
+ * the driver should use the allow_zero_bytesused flag to keep
+ * old userspace applications working.
*/
if (b->memory == V4L2_MEMORY_USERPTR) {
v4l2_planes[0].m.userptr = b->m.userptr;
@@ -1306,10 +1330,13 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
v4l2_planes[0].length = b->length;
}
- if (V4L2_TYPE_IS_OUTPUT(b->type))
- v4l2_planes[0].bytesused = b->bytesused ?
- b->bytesused : v4l2_planes[0].length;
- else
+ if (V4L2_TYPE_IS_OUTPUT(b->type)) {
+ if (vb->vb2_queue->allow_zero_bytesused)
+ v4l2_planes[0].bytesused = b->bytesused;
+ else
+ v4l2_planes[0].bytesused = b->bytesused ?
+ b->bytesused : v4l2_planes[0].length;
+ } else
v4l2_planes[0].bytesused = 0;
}
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index e49dc6b..a5790fd 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -337,6 +337,7 @@ struct v4l2_fh;
* @io_modes: supported io methods (see vb2_io_modes enum)
* @fileio_read_once: report EOF after reading the first buffer
* @fileio_write_immediately: queue buffer after each write() call
+ * @allow_zero_bytesused: allow bytesused == 0 to be passed to the driver
* @lock: pointer to a mutex that protects the vb2_queue struct. The
* driver can set this to a mutex to let the v4l2 core serialize
* the queuing ioctls. If the driver wants to handle locking
@@ -388,6 +389,7 @@ struct vb2_queue {
unsigned int io_modes;
unsigned fileio_read_once:1;
unsigned fileio_write_immediately:1;
+ unsigned allow_zero_bytesused:1;
struct mutex *lock;
struct v4l2_fh *owner;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v5 3/4] coda: set allow_zero_bytesused flag for vb2_queue_init
2015-02-20 16:38 [PATCH v5 1/4] vb2: split the io_flags member of vb2_queue into a bit field Kamil Debski
2015-02-20 16:38 ` [PATCH v5 2/4] vb2: add allow_zero_bytesused flag to the vb2_queue struct Kamil Debski
@ 2015-02-20 16:38 ` Kamil Debski
2015-02-20 16:53 ` Hans Verkuil
2015-02-20 16:38 ` [PATCH v5 4/4] s5p-mfc: " Kamil Debski
2 siblings, 1 reply; 9+ messages in thread
From: Kamil Debski @ 2015-02-20 16:38 UTC (permalink / raw)
To: linux-media; +Cc: m.szyprowski, k.debski, hverkuil
The coda driver interprets a buffer with bytesused equal to 0 as a special
case indicating end-of-stream. After vb2: fix bytesused == 0 handling
(8a75ffb) patch videobuf2 modified the value of bytesused if it was 0.
The allow_zero_bytesused flag was added to videobuf2 to keep
backward compatibility.
Signed-off-by: Kamil Debski <k.debski@samsung.com>
---
drivers/media/platform/coda/coda-common.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
index 6f32e6d..2d23f9a 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1541,6 +1541,13 @@ static int coda_queue_init(struct coda_ctx *ctx, struct vb2_queue *vq)
vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
vq->lock = &ctx->dev->dev_mutex;
+ /* One of means to indicate end-of-stream for coda is to set the
+ * bytesused == 0. However by default videobuf2 handles videobuf
+ * equal to 0 as a special case and changes its value to the size
+ * of the buffer. Set the allow_zero_bytesused flag, so
+ * that videobuf2 will keep the value of bytesused intact.
+ */
+ vq->allow_zero_bytesused = 1;
return vb2_queue_init(vq);
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v5 4/4] s5p-mfc: set allow_zero_bytesused flag for vb2_queue_init
2015-02-20 16:38 [PATCH v5 1/4] vb2: split the io_flags member of vb2_queue into a bit field Kamil Debski
2015-02-20 16:38 ` [PATCH v5 2/4] vb2: add allow_zero_bytesused flag to the vb2_queue struct Kamil Debski
2015-02-20 16:38 ` [PATCH v5 3/4] coda: set allow_zero_bytesused flag for vb2_queue_init Kamil Debski
@ 2015-02-20 16:38 ` Kamil Debski
2015-02-20 16:53 ` Hans Verkuil
2 siblings, 1 reply; 9+ messages in thread
From: Kamil Debski @ 2015-02-20 16:38 UTC (permalink / raw)
To: linux-media; +Cc: m.szyprowski, k.debski, hverkuil
The s5p-mfc driver interprets a buffer with bytesused equal to 0 as a
special case indicating end-of-stream. After vb2: fix bytesused == 0
handling (8a75ffb) patch videobuf2 modified the value of bytesused if it
was 0. The allow_zero_bytesused flag was added to videobuf2 to keep
backward compatibility.
Signed-off-by: Kamil Debski <k.debski@samsung.com>
---
drivers/media/platform/s5p-mfc/s5p_mfc.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 8e44a59..6b08488 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -843,6 +843,13 @@ static int s5p_mfc_open(struct file *file)
ret = -ENOENT;
goto err_queue_init;
}
+ /* One of means to indicate end-of-stream for MFC is to set the
+ * bytesused == 0. However by default videobuf2 handles videobuf
+ * equal to 0 as a special case and changes its value to the size
+ * of the buffer. Set the allow_zero_bytesused flag so that videobuf2
+ * will keep the value of bytesused intact.
+ */
+ q->allow_zero_bytesused = 1;
q->mem_ops = &vb2_dma_contig_memops;
q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
ret = vb2_queue_init(q);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v5 2/4] vb2: add allow_zero_bytesused flag to the vb2_queue struct
2015-02-20 16:38 ` [PATCH v5 2/4] vb2: add allow_zero_bytesused flag to the vb2_queue struct Kamil Debski
@ 2015-02-20 16:52 ` Hans Verkuil
2015-02-23 11:58 ` Kamil Debski
0 siblings, 1 reply; 9+ messages in thread
From: Hans Verkuil @ 2015-02-20 16:52 UTC (permalink / raw)
To: Kamil Debski, linux-media; +Cc: m.szyprowski
Hi Kamil,
One question and one typo below...
On 02/20/2015 05:38 PM, Kamil Debski wrote:
> The vb2: fix bytesused == 0 handling (8a75ffb) patch changed the behavior
> of __fill_vb2_buffer function, so that if bytesused is 0 it is set to the
> size of the buffer. However, bytesused set to 0 is used by older codec
> drivers as as indication used to mark the end of stream.
>
> To keep backward compatibility, this patch adds a flag passed to the
> vb2_queue_init function - allow_zero_bytesused. If the flag is set upon
> initialization of the queue, the videobuf2 keeps the value of bytesused
> intact in the OUTPUT queue and passes it to the driver.
>
> Reported-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> Signed-off-by: Kamil Debski <k.debski@samsung.com>
> ---
> drivers/media/v4l2-core/videobuf2-core.c | 39 +++++++++++++++++++++++++-----
> include/media/videobuf2-core.h | 2 ++
> 2 files changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> index 5cd60bf..5d77670 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -1247,6 +1247,16 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
> {
> unsigned int plane;
>
> + if (V4L2_TYPE_IS_OUTPUT(b->type)) {
> + if (WARN_ON_ONCE(b->bytesused == 0)) {
> + pr_warn_once("use of bytesused == 0 is deprecated and will be removed in 2017,\n");
I wonder if we should give a specific year, or just say 'in the future'.
What do you think?
> + if (vb->vb2_queue->allow_zero_bytesused)
> + pr_warn_once("use VIDIOC_DECODER_CMD(V4L2_DEC_CMD_STOP) instead.\n");
> + else
> + pr_warn_once("use the actual size instead.\n");
> + }
> + }
> +
> if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
> if (b->memory == V4L2_MEMORY_USERPTR) {
> for (plane = 0; plane < vb->num_planes; ++plane) {
> @@ -1276,13 +1286,22 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
> * userspace clearly never bothered to set it and
> * it's a safe assumption that they really meant to
> * use the full plane sizes.
> + *
> + * Some drivers, e.g. old codec drivers, use bytesused
> + * == 0 as a way to indicate that streaming is finished.
> + * In that case, the driver should use the
> + * allow_zero_bytesused flag to keep old userspace
> + * applications working.
> */
> for (plane = 0; plane < vb->num_planes; ++plane) {
> struct v4l2_plane *pdst = &v4l2_planes[plane];
> struct v4l2_plane *psrc = &b->m.planes[plane];
>
> - pdst->bytesused = psrc->bytesused ?
> - psrc->bytesused : pdst->length;
> + if (vb->vb2_queue->allow_zero_bytesused)
> + pdst->bytesused = psrc->bytesused;
> + else
> + pdst->bytesused = psrc->bytesused ?
> + psrc->bytesused : pdst->length;
> pdst->data_offset = psrc->data_offset;
> }
> }
> @@ -1295,6 +1314,11 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
> *
> * If bytesused == 0 for the output buffer, then fall back
> * to the full buffer size as that's a sensible default.
> + *
> + * Some drivers, e.g. old codec drivers, use bytesused * == 0 as
Small typo:
s/bytesused * == 0/bytesused == 0/
> + * a way to indicate that streaming is finished. In that case,
> + * the driver should use the allow_zero_bytesused flag to keep
> + * old userspace applications working.
> */
> if (b->memory == V4L2_MEMORY_USERPTR) {
> v4l2_planes[0].m.userptr = b->m.userptr;
> @@ -1306,10 +1330,13 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
> v4l2_planes[0].length = b->length;
> }
>
> - if (V4L2_TYPE_IS_OUTPUT(b->type))
> - v4l2_planes[0].bytesused = b->bytesused ?
> - b->bytesused : v4l2_planes[0].length;
> - else
> + if (V4L2_TYPE_IS_OUTPUT(b->type)) {
> + if (vb->vb2_queue->allow_zero_bytesused)
> + v4l2_planes[0].bytesused = b->bytesused;
> + else
> + v4l2_planes[0].bytesused = b->bytesused ?
> + b->bytesused : v4l2_planes[0].length;
> + } else
> v4l2_planes[0].bytesused = 0;
>
> }
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index e49dc6b..a5790fd 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -337,6 +337,7 @@ struct v4l2_fh;
> * @io_modes: supported io methods (see vb2_io_modes enum)
> * @fileio_read_once: report EOF after reading the first buffer
> * @fileio_write_immediately: queue buffer after each write() call
> + * @allow_zero_bytesused: allow bytesused == 0 to be passed to the driver
> * @lock: pointer to a mutex that protects the vb2_queue struct. The
> * driver can set this to a mutex to let the v4l2 core serialize
> * the queuing ioctls. If the driver wants to handle locking
> @@ -388,6 +389,7 @@ struct vb2_queue {
> unsigned int io_modes;
> unsigned fileio_read_once:1;
> unsigned fileio_write_immediately:1;
> + unsigned allow_zero_bytesused:1;
>
> struct mutex *lock;
> struct v4l2_fh *owner;
>
Regards,
Hans
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 3/4] coda: set allow_zero_bytesused flag for vb2_queue_init
2015-02-20 16:38 ` [PATCH v5 3/4] coda: set allow_zero_bytesused flag for vb2_queue_init Kamil Debski
@ 2015-02-20 16:53 ` Hans Verkuil
0 siblings, 0 replies; 9+ messages in thread
From: Hans Verkuil @ 2015-02-20 16:53 UTC (permalink / raw)
To: Kamil Debski, linux-media; +Cc: m.szyprowski
On 02/20/2015 05:38 PM, Kamil Debski wrote:
> The coda driver interprets a buffer with bytesused equal to 0 as a special
> case indicating end-of-stream. After vb2: fix bytesused == 0 handling
> (8a75ffb) patch videobuf2 modified the value of bytesused if it was 0.
> The allow_zero_bytesused flag was added to videobuf2 to keep
> backward compatibility.
>
> Signed-off-by: Kamil Debski <k.debski@samsung.com>
> ---
> drivers/media/platform/coda/coda-common.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
> index 6f32e6d..2d23f9a 100644
> --- a/drivers/media/platform/coda/coda-common.c
> +++ b/drivers/media/platform/coda/coda-common.c
> @@ -1541,6 +1541,13 @@ static int coda_queue_init(struct coda_ctx *ctx, struct vb2_queue *vq)
> vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> vq->lock = &ctx->dev->dev_mutex;
> + /* One of means to indicate end-of-stream for coda is to set the
s/One of means/One way/
> + * bytesused == 0. However by default videobuf2 handles videobuf
s/videobuf/bytesused/
> + * equal to 0 as a special case and changes its value to the size
> + * of the buffer. Set the allow_zero_bytesused flag, so
> + * that videobuf2 will keep the value of bytesused intact.
> + */
> + vq->allow_zero_bytesused = 1;
>
> return vb2_queue_init(vq);
> }
>
Regards,
Hans
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 4/4] s5p-mfc: set allow_zero_bytesused flag for vb2_queue_init
2015-02-20 16:38 ` [PATCH v5 4/4] s5p-mfc: " Kamil Debski
@ 2015-02-20 16:53 ` Hans Verkuil
0 siblings, 0 replies; 9+ messages in thread
From: Hans Verkuil @ 2015-02-20 16:53 UTC (permalink / raw)
To: Kamil Debski, linux-media; +Cc: m.szyprowski
On 02/20/2015 05:38 PM, Kamil Debski wrote:
> The s5p-mfc driver interprets a buffer with bytesused equal to 0 as a
> special case indicating end-of-stream. After vb2: fix bytesused == 0
> handling (8a75ffb) patch videobuf2 modified the value of bytesused if it
> was 0. The allow_zero_bytesused flag was added to videobuf2 to keep
> backward compatibility.
>
> Signed-off-by: Kamil Debski <k.debski@samsung.com>
> ---
> drivers/media/platform/s5p-mfc/s5p_mfc.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> index 8e44a59..6b08488 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> @@ -843,6 +843,13 @@ static int s5p_mfc_open(struct file *file)
> ret = -ENOENT;
> goto err_queue_init;
> }
> + /* One of means to indicate end-of-stream for MFC is to set the
> + * bytesused == 0. However by default videobuf2 handles videobuf
Same typos as in patch 3/4.
Regards,
Hans
> + * equal to 0 as a special case and changes its value to the size
> + * of the buffer. Set the allow_zero_bytesused flag so that videobuf2
> + * will keep the value of bytesused intact.
> + */
> + q->allow_zero_bytesused = 1;
> q->mem_ops = &vb2_dma_contig_memops;
> q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> ret = vb2_queue_init(q);
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v5 2/4] vb2: add allow_zero_bytesused flag to the vb2_queue struct
2015-02-20 16:52 ` Hans Verkuil
@ 2015-02-23 11:58 ` Kamil Debski
2015-02-23 11:59 ` Hans Verkuil
0 siblings, 1 reply; 9+ messages in thread
From: Kamil Debski @ 2015-02-23 11:58 UTC (permalink / raw)
To: 'Hans Verkuil', linux-media; +Cc: Marek Szyprowski
Hi,
Thank you for the review :)
> From: Hans Verkuil [mailto:hverkuil@xs4all.nl]
> Sent: Friday, February 20, 2015 5:52 PM
>
> Hi Kamil,
>
> One question and one typo below...
>
> On 02/20/2015 05:38 PM, Kamil Debski wrote:
> > The vb2: fix bytesused == 0 handling (8a75ffb) patch changed the
> > behavior of __fill_vb2_buffer function, so that if bytesused is 0 it
> > is set to the size of the buffer. However, bytesused set to 0 is used
> > by older codec drivers as as indication used to mark the end of
> stream.
> >
> > To keep backward compatibility, this patch adds a flag passed to the
> > vb2_queue_init function - allow_zero_bytesused. If the flag is set
> > upon initialization of the queue, the videobuf2 keeps the value of
> > bytesused intact in the OUTPUT queue and passes it to the driver.
> >
> > Reported-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > Signed-off-by: Kamil Debski <k.debski@samsung.com>
> > ---
> > drivers/media/v4l2-core/videobuf2-core.c | 39
> +++++++++++++++++++++++++-----
> > include/media/videobuf2-core.h | 2 ++
> > 2 files changed, 35 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> > b/drivers/media/v4l2-core/videobuf2-core.c
> > index 5cd60bf..5d77670 100644
> > --- a/drivers/media/v4l2-core/videobuf2-core.c
> > +++ b/drivers/media/v4l2-core/videobuf2-core.c
> > @@ -1247,6 +1247,16 @@ static void __fill_vb2_buffer(struct
> vb2_buffer
> > *vb, const struct v4l2_buffer *b {
> > unsigned int plane;
> >
> > + if (V4L2_TYPE_IS_OUTPUT(b->type)) {
> > + if (WARN_ON_ONCE(b->bytesused == 0)) {
> > + pr_warn_once("use of bytesused == 0 is deprecated
and
> will be
> > +removed in 2017,\n");
>
> I wonder if we should give a specific year, or just say 'in the
> future'.
>
> What do you think?
I think I would prefer to use the phrase "in the future".
If you are ok with that I will post an updated patch set soon.
>
> > + if (vb->vb2_queue->allow_zero_bytesused)
> > + pr_warn_once("use
> VIDIOC_DECODER_CMD(V4L2_DEC_CMD_STOP) instead.\n");
> > + else
> > + pr_warn_once("use the actual size
instead.\n");
> > + }
> > + }
> > +
> > if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
> > if (b->memory == V4L2_MEMORY_USERPTR) {
> > for (plane = 0; plane < vb->num_planes; ++plane) {
@@
> -1276,13
> > +1286,22 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb,
> const struct v4l2_buffer *b
> > * userspace clearly never bothered to set it and
> > * it's a safe assumption that they really meant to
> > * use the full plane sizes.
> > + *
> > + * Some drivers, e.g. old codec drivers, use
> bytesused
> > + * == 0 as a way to indicate that streaming is
> finished.
> > + * In that case, the driver should use the
> > + * allow_zero_bytesused flag to keep old userspace
> > + * applications working.
> > */
> > for (plane = 0; plane < vb->num_planes; ++plane) {
> > struct v4l2_plane *pdst =
&v4l2_planes[plane];
> > struct v4l2_plane *psrc =
&b->m.planes[plane];
> >
> > - pdst->bytesused = psrc->bytesused ?
> > - psrc->bytesused : pdst->length;
> > + if (vb->vb2_queue->allow_zero_bytesused)
> > + pdst->bytesused = psrc->bytesused;
> > + else
> > + pdst->bytesused = psrc->bytesused ?
> > + psrc->bytesused :
pdst->length;
> > pdst->data_offset = psrc->data_offset;
> > }
> > }
> > @@ -1295,6 +1314,11 @@ static void __fill_vb2_buffer(struct
> vb2_buffer *vb, const struct v4l2_buffer *b
> > *
> > * If bytesused == 0 for the output buffer, then fall back
> > * to the full buffer size as that's a sensible default.
> > + *
> > + * Some drivers, e.g. old codec drivers, use bytesused * ==
> 0 as
>
> Small typo:
>
> s/bytesused * == 0/bytesused == 0/
>
> > + * a way to indicate that streaming is finished. In that
> case,
> > + * the driver should use the allow_zero_bytesused flag to
> keep
> > + * old userspace applications working.
> > */
> > if (b->memory == V4L2_MEMORY_USERPTR) {
> > v4l2_planes[0].m.userptr = b->m.userptr; @@ -1306,10
> +1330,13 @@
> > static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct
> v4l2_buffer *b
> > v4l2_planes[0].length = b->length;
> > }
> >
> > - if (V4L2_TYPE_IS_OUTPUT(b->type))
> > - v4l2_planes[0].bytesused = b->bytesused ?
> > - b->bytesused : v4l2_planes[0].length;
> > - else
> > + if (V4L2_TYPE_IS_OUTPUT(b->type)) {
> > + if (vb->vb2_queue->allow_zero_bytesused)
> > + v4l2_planes[0].bytesused = b->bytesused;
> > + else
> > + v4l2_planes[0].bytesused = b->bytesused ?
> > + b->bytesused :
v4l2_planes[0].length;
> > + } else
> > v4l2_planes[0].bytesused = 0;
> >
> > }
> > diff --git a/include/media/videobuf2-core.h
> > b/include/media/videobuf2-core.h index e49dc6b..a5790fd 100644
> > --- a/include/media/videobuf2-core.h
> > +++ b/include/media/videobuf2-core.h
> > @@ -337,6 +337,7 @@ struct v4l2_fh;
> > * @io_modes: supported io methods (see vb2_io_modes enum)
> > * @fileio_read_once: report EOF after reading the first
> buffer
> > * @fileio_write_immediately: queue buffer after each write() call
> > + * @allow_zero_bytesused: allow bytesused == 0 to be passed to the
> driver
> > * @lock: pointer to a mutex that protects the vb2_queue
> struct. The
> > * driver can set this to a mutex to let the v4l2 core
> serialize
> > * the queuing ioctls. If the driver wants to handle locking
> > @@ -388,6 +389,7 @@ struct vb2_queue {
> > unsigned int io_modes;
> > unsigned fileio_read_once:1;
> > unsigned fileio_write_immediately:1;
> > + unsigned allow_zero_bytesused:1;
> >
> > struct mutex *lock;
> > struct v4l2_fh *owner;
> >
>
> Regards,
>
> Hans
Best wishes,
--
Kamil Debski
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 2/4] vb2: add allow_zero_bytesused flag to the vb2_queue struct
2015-02-23 11:58 ` Kamil Debski
@ 2015-02-23 11:59 ` Hans Verkuil
0 siblings, 0 replies; 9+ messages in thread
From: Hans Verkuil @ 2015-02-23 11:59 UTC (permalink / raw)
To: Kamil Debski, linux-media; +Cc: Marek Szyprowski
On 02/23/2015 12:58 PM, Kamil Debski wrote:
> Hi,
>
> Thank you for the review :)
>
>> From: Hans Verkuil [mailto:hverkuil@xs4all.nl]
>> Sent: Friday, February 20, 2015 5:52 PM
>>
>> Hi Kamil,
>>
>> One question and one typo below...
>>
>> On 02/20/2015 05:38 PM, Kamil Debski wrote:
>>> The vb2: fix bytesused == 0 handling (8a75ffb) patch changed the
>>> behavior of __fill_vb2_buffer function, so that if bytesused is 0 it
>>> is set to the size of the buffer. However, bytesused set to 0 is used
>>> by older codec drivers as as indication used to mark the end of
>> stream.
>>>
>>> To keep backward compatibility, this patch adds a flag passed to the
>>> vb2_queue_init function - allow_zero_bytesused. If the flag is set
>>> upon initialization of the queue, the videobuf2 keeps the value of
>>> bytesused intact in the OUTPUT queue and passes it to the driver.
>>>
>>> Reported-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
>>> Signed-off-by: Kamil Debski <k.debski@samsung.com>
>>> ---
>>> drivers/media/v4l2-core/videobuf2-core.c | 39
>> +++++++++++++++++++++++++-----
>>> include/media/videobuf2-core.h | 2 ++
>>> 2 files changed, 35 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
>>> b/drivers/media/v4l2-core/videobuf2-core.c
>>> index 5cd60bf..5d77670 100644
>>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>>> @@ -1247,6 +1247,16 @@ static void __fill_vb2_buffer(struct
>> vb2_buffer
>>> *vb, const struct v4l2_buffer *b {
>>> unsigned int plane;
>>>
>>> + if (V4L2_TYPE_IS_OUTPUT(b->type)) {
>>> + if (WARN_ON_ONCE(b->bytesused == 0)) {
>>> + pr_warn_once("use of bytesused == 0 is deprecated
> and
>> will be
>>> +removed in 2017,\n");
>>
>> I wonder if we should give a specific year, or just say 'in the
>> future'.
>>
>> What do you think?
>
> I think I would prefer to use the phrase "in the future".
> If you are ok with that I will post an updated patch set soon.
I agree with that.
Regards,
Hans
>
>>
>>> + if (vb->vb2_queue->allow_zero_bytesused)
>>> + pr_warn_once("use
>> VIDIOC_DECODER_CMD(V4L2_DEC_CMD_STOP) instead.\n");
>>> + else
>>> + pr_warn_once("use the actual size
> instead.\n");
>>> + }
>>> + }
>>> +
>>> if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
>>> if (b->memory == V4L2_MEMORY_USERPTR) {
>>> for (plane = 0; plane < vb->num_planes; ++plane) {
> @@
>> -1276,13
>>> +1286,22 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb,
>> const struct v4l2_buffer *b
>>> * userspace clearly never bothered to set it and
>>> * it's a safe assumption that they really meant to
>>> * use the full plane sizes.
>>> + *
>>> + * Some drivers, e.g. old codec drivers, use
>> bytesused
>>> + * == 0 as a way to indicate that streaming is
>> finished.
>>> + * In that case, the driver should use the
>>> + * allow_zero_bytesused flag to keep old userspace
>>> + * applications working.
>>> */
>>> for (plane = 0; plane < vb->num_planes; ++plane) {
>>> struct v4l2_plane *pdst =
> &v4l2_planes[plane];
>>> struct v4l2_plane *psrc =
> &b->m.planes[plane];
>>>
>>> - pdst->bytesused = psrc->bytesused ?
>>> - psrc->bytesused : pdst->length;
>>> + if (vb->vb2_queue->allow_zero_bytesused)
>>> + pdst->bytesused = psrc->bytesused;
>>> + else
>>> + pdst->bytesused = psrc->bytesused ?
>>> + psrc->bytesused :
> pdst->length;
>>> pdst->data_offset = psrc->data_offset;
>>> }
>>> }
>>> @@ -1295,6 +1314,11 @@ static void __fill_vb2_buffer(struct
>> vb2_buffer *vb, const struct v4l2_buffer *b
>>> *
>>> * If bytesused == 0 for the output buffer, then fall back
>>> * to the full buffer size as that's a sensible default.
>>> + *
>>> + * Some drivers, e.g. old codec drivers, use bytesused * ==
>> 0 as
>>
>> Small typo:
>>
>> s/bytesused * == 0/bytesused == 0/
>>
>>> + * a way to indicate that streaming is finished. In that
>> case,
>>> + * the driver should use the allow_zero_bytesused flag to
>> keep
>>> + * old userspace applications working.
>>> */
>>> if (b->memory == V4L2_MEMORY_USERPTR) {
>>> v4l2_planes[0].m.userptr = b->m.userptr; @@ -1306,10
>> +1330,13 @@
>>> static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct
>> v4l2_buffer *b
>>> v4l2_planes[0].length = b->length;
>>> }
>>>
>>> - if (V4L2_TYPE_IS_OUTPUT(b->type))
>>> - v4l2_planes[0].bytesused = b->bytesused ?
>>> - b->bytesused : v4l2_planes[0].length;
>>> - else
>>> + if (V4L2_TYPE_IS_OUTPUT(b->type)) {
>>> + if (vb->vb2_queue->allow_zero_bytesused)
>>> + v4l2_planes[0].bytesused = b->bytesused;
>>> + else
>>> + v4l2_planes[0].bytesused = b->bytesused ?
>>> + b->bytesused :
> v4l2_planes[0].length;
>>> + } else
>>> v4l2_planes[0].bytesused = 0;
>>>
>>> }
>>> diff --git a/include/media/videobuf2-core.h
>>> b/include/media/videobuf2-core.h index e49dc6b..a5790fd 100644
>>> --- a/include/media/videobuf2-core.h
>>> +++ b/include/media/videobuf2-core.h
>>> @@ -337,6 +337,7 @@ struct v4l2_fh;
>>> * @io_modes: supported io methods (see vb2_io_modes enum)
>>> * @fileio_read_once: report EOF after reading the first
>> buffer
>>> * @fileio_write_immediately: queue buffer after each write() call
>>> + * @allow_zero_bytesused: allow bytesused == 0 to be passed to the
>> driver
>>> * @lock: pointer to a mutex that protects the vb2_queue
>> struct. The
>>> * driver can set this to a mutex to let the v4l2 core
>> serialize
>>> * the queuing ioctls. If the driver wants to handle locking
>>> @@ -388,6 +389,7 @@ struct vb2_queue {
>>> unsigned int io_modes;
>>> unsigned fileio_read_once:1;
>>> unsigned fileio_write_immediately:1;
>>> + unsigned allow_zero_bytesused:1;
>>>
>>> struct mutex *lock;
>>> struct v4l2_fh *owner;
>>>
>>
>> Regards,
>>
>> Hans
>
> Best wishes,
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-02-23 12:00 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-20 16:38 [PATCH v5 1/4] vb2: split the io_flags member of vb2_queue into a bit field Kamil Debski
2015-02-20 16:38 ` [PATCH v5 2/4] vb2: add allow_zero_bytesused flag to the vb2_queue struct Kamil Debski
2015-02-20 16:52 ` Hans Verkuil
2015-02-23 11:58 ` Kamil Debski
2015-02-23 11:59 ` Hans Verkuil
2015-02-20 16:38 ` [PATCH v5 3/4] coda: set allow_zero_bytesused flag for vb2_queue_init Kamil Debski
2015-02-20 16:53 ` Hans Verkuil
2015-02-20 16:38 ` [PATCH v5 4/4] s5p-mfc: " Kamil Debski
2015-02-20 16:53 ` Hans Verkuil
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.