RE: [PATCH 1/2] vb2: Add VB2_FILEIO_ALLOW_ZERO_BYTESUSED flag to vb2_fileio_flags
Hi, From: Hans Verkuil [mailto:hverk...@xs4all.nl] Sent: Wednesday, February 18, 2015 10:58 AM On 02/18/15 10:42, Kamil Debski wrote: Hi Hans, From: Hans Verkuil [mailto:hverk...@xs4all.nl] Sent: Tuesday, February 17, 2015 10:11 AM Hi Kamil, On 12/16/14 12:36, 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 - VB2_FILEIO_ALLOW_ZERO_BYTESUSED. If the flag is set upon initialization of the queue, the videobuf2 keeps the value of bytesused intact and passes it to the driver. What is the status of this patch series? I have to admit that I had forgotten a bit about this patch, because of other important work. Thanks for reminding me :) Note that io_flags is really the wrong place for this flag, it should be io_modes. This flag has nothing to do with file I/O. What do you think about adding a separate flags field into the vb2_queue structure? This could be combined with changing io_flags to u8 or a bit field to save space. I think changing io_flags to a bitfield is a good idea. unsigned fileio_read_once:1; unsigned fileio_write_immediately:1; unsigned allow_zero_bytesused:1; However, going back to allow_zero_bytesused: this has been broken for quite some time without anyone complaining (other than you :-) ). If I remember correctly, it was Nicolas who reported to me the problem on the IRC. Might it not be better to just fix this properly by calling V4L2_DEC_CMD_STOP, as done here: https://www.mail-archive.com/linux- me...@vger.kernel.org/msg84916.html, and drop the support for zero bytesused to mark EOS entirely? I think it would be good to have the backward compatibility for some time. I might be too optimistic here. Or perhaps at least add a pr_warn telling users to switch to V4L2_DEC_CMD_STOP since this will be removed in 2017 or whatever. Where do you see the pr_warn? I guess it would be good if it was only displayed once and only when the app uses bytesused == 0 to signal the EOS. Do you think alike? Regards, Hans Best wishes, -- Kamil Debski Samsung RD Institute Poland -- 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: [PATCH 1/2] vb2: Add VB2_FILEIO_ALLOW_ZERO_BYTESUSED flag to vb2_fileio_flags
Hi Hans,
From: Hans Verkuil [mailto:hverk...@xs4all.nl]
Sent: Tuesday, February 17, 2015 10:11 AM
Hi Kamil,
On 12/16/14 12:36, 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 - VB2_FILEIO_ALLOW_ZERO_BYTESUSED. If the
flag
is set upon initialization of the queue, the videobuf2 keeps the
value
of bytesused intact and passes it to the driver.
What is the status of this patch series?
I have to admit that I had forgotten a bit about this patch, because of
other
important work. Thanks for reminding me :)
Note that io_flags is really the wrong place for this flag, it should
be io_modes. This flag has nothing to do with file I/O.
What do you think about adding a separate flags field into the vb2_queue
structure? This could be combined with changing io_flags to u8 or a bit
field
to save space.
Best wishes,
--
Kamil Debski
Samsung RD Institute Poland
Regards,
Hans
Reported-by: Nicolas Dufresne nicolas.dufre...@collabora.com
Signed-off-by: Kamil Debski k.deb...@samsung.com
---
drivers/media/v4l2-core/videobuf2-core.c | 33
--
include/media/videobuf2-core.h |3 +++
2 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index d09a891..1068dbb 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1276,13 +1276,23 @@ 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 following
+* io_flag VB2_FILEIO_ALLOW_ZERO_BYTESUSED 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-io_flags
+ VB2_FILEIO_ALLOW_ZERO_BYTESUSED)
+ pdst-bytesused = psrc-bytesused;
+ else
+ pdst-bytesused = psrc-bytesused ?
+ psrc-bytesused :
pdst-length;
pdst-data_offset = psrc-data_offset;
}
}
@@ -1295,6 +1305,12 @@ 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 following io_flag
+* VB2_FILEIO_ALLOW_ZERO_BYTESUSED to keep old userspace
+* applications working.
*/
if (b-memory == V4L2_MEMORY_USERPTR) {
v4l2_planes[0].m.userptr = b-m.userptr; @@ -1306,11
+1322,16 @@
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-io_flags
+ VB2_FILEIO_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;
+ }
Re: [PATCH 1/2] vb2: Add VB2_FILEIO_ALLOW_ZERO_BYTESUSED flag to vb2_fileio_flags
On 02/18/15 10:42, Kamil Debski wrote: Hi Hans, From: Hans Verkuil [mailto:hverk...@xs4all.nl] Sent: Tuesday, February 17, 2015 10:11 AM Hi Kamil, On 12/16/14 12:36, 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 - VB2_FILEIO_ALLOW_ZERO_BYTESUSED. If the flag is set upon initialization of the queue, the videobuf2 keeps the value of bytesused intact and passes it to the driver. What is the status of this patch series? I have to admit that I had forgotten a bit about this patch, because of other important work. Thanks for reminding me :) Note that io_flags is really the wrong place for this flag, it should be io_modes. This flag has nothing to do with file I/O. What do you think about adding a separate flags field into the vb2_queue structure? This could be combined with changing io_flags to u8 or a bit field to save space. I think changing io_flags to a bitfield is a good idea. unsigned fileio_read_once:1; unsigned fileio_write_immediately:1; unsigned allow_zero_bytesused:1; However, going back to allow_zero_bytesused: this has been broken for quite some time without anyone complaining (other than you :-) ). Might it not be better to just fix this properly by calling V4L2_DEC_CMD_STOP, as done here: https://www.mail-archive.com/linux-media@vger.kernel.org/msg84916.html, and drop the support for zero bytesused to mark EOS entirely? I might be too optimistic here. Or perhaps at least add a pr_warn telling users to switch to V4L2_DEC_CMD_STOP since this will be removed in 2017 or whatever. Regards, Hans -- 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: [PATCH 1/2] vb2: Add VB2_FILEIO_ALLOW_ZERO_BYTESUSED flag to vb2_fileio_flags
On 02/18/15 11:31, Kamil Debski wrote: Hi, From: Hans Verkuil [mailto:hverk...@xs4all.nl] Sent: Wednesday, February 18, 2015 10:58 AM On 02/18/15 10:42, Kamil Debski wrote: Hi Hans, From: Hans Verkuil [mailto:hverk...@xs4all.nl] Sent: Tuesday, February 17, 2015 10:11 AM Hi Kamil, On 12/16/14 12:36, 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 - VB2_FILEIO_ALLOW_ZERO_BYTESUSED. If the flag is set upon initialization of the queue, the videobuf2 keeps the value of bytesused intact and passes it to the driver. What is the status of this patch series? I have to admit that I had forgotten a bit about this patch, because of other important work. Thanks for reminding me :) Note that io_flags is really the wrong place for this flag, it should be io_modes. This flag has nothing to do with file I/O. What do you think about adding a separate flags field into the vb2_queue structure? This could be combined with changing io_flags to u8 or a bit field to save space. I think changing io_flags to a bitfield is a good idea. unsigned fileio_read_once:1; unsigned fileio_write_immediately:1; unsigned allow_zero_bytesused:1; However, going back to allow_zero_bytesused: this has been broken for quite some time without anyone complaining (other than you :-) ). If I remember correctly, it was Nicolas who reported to me the problem on the IRC. Might it not be better to just fix this properly by calling V4L2_DEC_CMD_STOP, as done here: https://www.mail-archive.com/linux- me...@vger.kernel.org/msg84916.html, and drop the support for zero bytesused to mark EOS entirely? I think it would be good to have the backward compatibility for some time. I might be too optimistic here. Or perhaps at least add a pr_warn telling users to switch to V4L2_DEC_CMD_STOP since this will be removed in 2017 or whatever. Where do you see the pr_warn? I guess it would be good if it was only displayed once and only when the app uses bytesused == 0 to signal the EOS. Do you think alike? Right, pr_warn_once() would do that. As I discussed with Pawel on irc I also think we need to do a pr_warn_once if the application doesn't set bytesused for output streams. Right now we replace bytesused by length to cater for bad apps, but I really hate that. I'd like to eventually require a proper setting for bytesused, but that means apps need to be informed that they do something wrong. Unfortunately, other than gstreamer I would expect that all those apps are closed source running on embedded systems. Nicolas, can you confirm that gstreamer is filling in bytesused for output buffers correctly? (I.e., doesn't leave it to 0). I may have mentioned this before, but have you run gstreamer with valgrind? valgrind 3.10 now supports all v4l2 ioctls. Regards, Hans Regards, Hans Best wishes, -- 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: [PATCH 1/2] vb2: Add VB2_FILEIO_ALLOW_ZERO_BYTESUSED flag to vb2_fileio_flags
Hi Kamil,
On 12/16/14 12:36, 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 - VB2_FILEIO_ALLOW_ZERO_BYTESUSED. If the flag is
set upon initialization of the queue, the videobuf2 keeps the value of
bytesused intact and passes it to the driver.
What is the status of this patch series?
Note that io_flags is really the wrong place for this flag, it should
be io_modes. This flag has nothing to do with file I/O.
Regards,
Hans
Reported-by: Nicolas Dufresne nicolas.dufre...@collabora.com
Signed-off-by: Kamil Debski k.deb...@samsung.com
---
drivers/media/v4l2-core/videobuf2-core.c | 33
--
include/media/videobuf2-core.h |3 +++
2 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index d09a891..1068dbb 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1276,13 +1276,23 @@ 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 following
+ * io_flag VB2_FILEIO_ALLOW_ZERO_BYTESUSED 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-io_flags
+ VB2_FILEIO_ALLOW_ZERO_BYTESUSED)
+ pdst-bytesused = psrc-bytesused;
+ else
+ pdst-bytesused = psrc-bytesused ?
+ psrc-bytesused : pdst-length;
pdst-data_offset = psrc-data_offset;
}
}
@@ -1295,6 +1305,12 @@ 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 following io_flag
+ * VB2_FILEIO_ALLOW_ZERO_BYTESUSED to keep old userspace
+ * applications working.
*/
if (b-memory == V4L2_MEMORY_USERPTR) {
v4l2_planes[0].m.userptr = b-m.userptr;
@@ -1306,11 +1322,16 @@ 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-io_flags
+ VB2_FILEIO_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 bd2cec2..0540bc3 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -138,10 +138,13 @@ enum vb2_io_modes {
* 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
Re: [PATCH 1/2] vb2: Add VB2_FILEIO_ALLOW_ZERO_BYTESUSED flag to vb2_fileio_flags
Hi Kamil,
2014-12-16 12:36 GMT+01:00 Kamil Debski k.deb...@samsung.com:
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 - VB2_FILEIO_ALLOW_ZERO_BYTESUSED. If the flag is
set upon initialization of the queue, the videobuf2 keeps the value of
bytesused intact and passes it to the driver.
Nice, this is something we were planning to do :).
But I would split this patch and the second which is specific to
s5p-mfc as this is core specific and should be independant.
Reported-by: Nicolas Dufresne nicolas.dufre...@collabora.com
Signed-off-by: Kamil Debski k.deb...@samsung.com
---
drivers/media/v4l2-core/videobuf2-core.c | 33
--
include/media/videobuf2-core.h |3 +++
2 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index d09a891..1068dbb 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1276,13 +1276,23 @@ 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 following
+* io_flag VB2_FILEIO_ALLOW_ZERO_BYTESUSED to keep old
+* userspace applications working.
Not sure if this comment is necessary, as this is already in the commit ?
*/
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-io_flags
+ VB2_FILEIO_ALLOW_ZERO_BYTESUSED)
+ pdst-bytesused = psrc-bytesused;
+ else
+ pdst-bytesused = psrc-bytesused ?
+ psrc-bytesused :
pdst-length;
pdst-data_offset = psrc-data_offset;
}
}
@@ -1295,6 +1305,12 @@ 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 following io_flag
+* VB2_FILEIO_ALLOW_ZERO_BYTESUSED to keep old userspace
+* applications working.
Again, not sure this is useful.
*/
if (b-memory == V4L2_MEMORY_USERPTR) {
v4l2_planes[0].m.userptr = b-m.userptr;
@@ -1306,11 +1322,16 @@ 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-io_flags
+ VB2_FILEIO_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 bd2cec2..0540bc3 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -138,10 +138,13
RE: [PATCH 1/2] vb2: Add VB2_FILEIO_ALLOW_ZERO_BYTESUSED flag to vb2_fileio_flags
Hi Jean,
From: Jean-Michel Hautbois [mailto:jhautb...@gmail.com]
Sent: Friday, December 19, 2014 3:36 PM
To: Kamil Debski
Cc: Linux Media Mailing List; m.szyprow...@samsung.com; Hans Verkuil;
Nicolas Dufresne
Subject: Re: [PATCH 1/2] vb2: Add VB2_FILEIO_ALLOW_ZERO_BYTESUSED flag
to vb2_fileio_flags
Hi Kamil,
2014-12-16 12:36 GMT+01:00 Kamil Debski k.deb...@samsung.com:
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 - VB2_FILEIO_ALLOW_ZERO_BYTESUSED. If the
flag
is set upon initialization of the queue, the videobuf2 keeps the
value
of bytesused intact and passes it to the driver.
Nice, this is something we were planning to do :).
But I would split this patch and the second which is specific to s5p-
mfc as this is core specific and should be independant.
This patch contains only changes in the videobuf2-core.c file. The next
patch in the series contains changes in the s5p-mfc driver. There is
another patch sent today that adds this flag to coda.
These are all separate patches, two of them are in a single patchset.
Actually, I would send all of them in one patchset, but initially I missed
that the coda driver should also have this change applied. (Nicolas, thank
you for reminding me to do this on IRC).
Reported-by: Nicolas Dufresne nicolas.dufre...@collabora.com
Signed-off-by: Kamil Debski k.deb...@samsung.com
---
drivers/media/v4l2-core/videobuf2-core.c | 33
--
include/media/videobuf2-core.h |3 +++
2 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index d09a891..1068dbb 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1276,13 +1276,23 @@ 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
following
+* io_flag VB2_FILEIO_ALLOW_ZERO_BYTESUSED to
keep old
+* userspace applications working.
Not sure if this comment is necessary, as this is already in the
commit ?
The comment were present in the file already, I expanded them to cover the
exception when the VB2_FILEIO_ALLOW_ZERO_BYTESUSED flag is set.
It is also explained in the commit, I agree, but in the end one usually
looks in the source code.
*/
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-io_flags
+
VB2_FILEIO_ALLOW_ZERO_BYTESUSED)
+ pdst-bytesused = psrc-
bytesused;
+ else
+ pdst-bytesused = psrc-
bytesused ?
+ psrc-bytesused :
+ pdst-length;
pdst-data_offset = psrc-data_offset;
}
}
@@ -1295,6 +1305,12 @@ 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 following io_flag
+* VB2_FILEIO_ALLOW_ZERO_BYTESUSED to keep old
userspace
+* applications working.
Again, not sure this is useful.
Same applies here, I expanded the comment to cover a new case.
*/
if (b-memory == V4L2_MEMORY_USERPTR) {
v4l2_planes[0].m.userptr = b-m.userptr
Re: [PATCH 1/2] vb2: Add VB2_FILEIO_ALLOW_ZERO_BYTESUSED flag to vb2_fileio_flags
2014-12-19 17:03 GMT+01:00 Kamil Debski k.deb...@samsung.com:
Hi Jean,
From: Jean-Michel Hautbois [mailto:jhautb...@gmail.com]
Sent: Friday, December 19, 2014 3:36 PM
To: Kamil Debski
Cc: Linux Media Mailing List; m.szyprow...@samsung.com; Hans Verkuil;
Nicolas Dufresne
Subject: Re: [PATCH 1/2] vb2: Add VB2_FILEIO_ALLOW_ZERO_BYTESUSED flag
to vb2_fileio_flags
Hi Kamil,
2014-12-16 12:36 GMT+01:00 Kamil Debski k.deb...@samsung.com:
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 - VB2_FILEIO_ALLOW_ZERO_BYTESUSED. If the
flag
is set upon initialization of the queue, the videobuf2 keeps the
value
of bytesused intact and passes it to the driver.
Nice, this is something we were planning to do :).
But I would split this patch and the second which is specific to s5p-
mfc as this is core specific and should be independant.
This patch contains only changes in the videobuf2-core.c file. The next
patch in the series contains changes in the s5p-mfc driver. There is
another patch sent today that adds this flag to coda.
These are all separate patches, two of them are in a single patchset.
Actually, I would send all of them in one patchset, but initially I missed
that the coda driver should also have this change applied. (Nicolas, thank
you for reminding me to do this on IRC).
OK, This is why I have been confused. Well, I still think that core
modification should be a separate patch, and maybe s5f-mpc and coda be
in the same patchset. Not a problem.
Reported-by: Nicolas Dufresne nicolas.dufre...@collabora.com
Signed-off-by: Kamil Debski k.deb...@samsung.com
---
drivers/media/v4l2-core/videobuf2-core.c | 33
--
include/media/videobuf2-core.h |3 +++
2 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index d09a891..1068dbb 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1276,13 +1276,23 @@ 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
following
+* io_flag VB2_FILEIO_ALLOW_ZERO_BYTESUSED to
keep old
+* userspace applications working.
Not sure if this comment is necessary, as this is already in the
commit ?
The comment were present in the file already, I expanded them to cover the
exception when the VB2_FILEIO_ALLOW_ZERO_BYTESUSED flag is set.
It is also explained in the commit, I agree, but in the end one usually
looks in the source code.
OK
*/
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-io_flags
+
VB2_FILEIO_ALLOW_ZERO_BYTESUSED)
+ pdst-bytesused = psrc-
bytesused;
+ else
+ pdst-bytesused = psrc-
bytesused ?
+ psrc-bytesused :
+ pdst-length;
pdst-data_offset = psrc-data_offset;
}
}
@@ -1295,6 +1305,12 @@ 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 following io_flag
+* VB2_FILEIO_ALLOW_ZERO_BYTESUSED to keep old
userspace
+* applications working
