v4l: videobuf: qbuf now uses relevant v4l2_buffer fields for OUTPUT types

Message ID 1271843067-23496-1-git-send-email-p.osciak@samsung.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Pawel Osciak April 21, 2010, 9:44 a.m. UTC
  According to the V4L2 specification, applications set bytesused, field and
timestamp fields of struct v4l2_buffer when the buffer is intended for
output and memory type is MMAP. This adds proper copying of those values
to videobuf_buffer so drivers can use them.

Signed-off-by: Pawel Osciak <p.osciak@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/video/videobuf-core.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)
  

Comments

Laurent Pinchart April 22, 2010, 9:14 a.m. UTC | #1
Hi Pawel,

On Wednesday 21 April 2010 11:44:27 Pawel Osciak wrote:
> According to the V4L2 specification, applications set bytesused, field and
> timestamp fields of struct v4l2_buffer when the buffer is intended for
> output and memory type is MMAP. This adds proper copying of those values
> to videobuf_buffer so drivers can use them.

Why only for the MMAP memory type ? Don't drivers need the information for 
USERPTR buffers as well ?

> Signed-off-by: Pawel Osciak <p.osciak@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/media/video/videobuf-core.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/video/videobuf-core.c
> b/drivers/media/video/videobuf-core.c index 63d7043..e573ca7 100644
> --- a/drivers/media/video/videobuf-core.c
> +++ b/drivers/media/video/videobuf-core.c
> @@ -549,6 +549,13 @@ int videobuf_qbuf(struct videobuf_queue *q, struct
> v4l2_buffer *b) "but buffer addr is zero!\n");
>  			goto done;
>  		}
> +		if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT
> +		    || q->type == V4L2_BUF_TYPE_VBI_OUTPUT
> +		    || q->type == V4L2_BUF_TYPE_SLICED_VBI_OUTPUT) {
> +			buf->size = b->bytesused;
> +			buf->field = b->field;
> +			buf->ts = b->timestamp;
> +		}
>  		break;
>  	case V4L2_MEMORY_USERPTR:
>  		if (b->length < buf->bsize) {
  
Pawel Osciak April 22, 2010, 9:24 a.m. UTC | #2
Hi Laurent,

>Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
>> According to the V4L2 specification, applications set bytesused, field and
>> timestamp fields of struct v4l2_buffer when the buffer is intended for
>> output and memory type is MMAP. This adds proper copying of those values
>> to videobuf_buffer so drivers can use them.
>
>Why only for the MMAP memory type ? Don't drivers need the information for
>USERPTR buffers as well ?
>

It is only mentioned for the MMAP memory type:
http://linuxtv.org/downloads/video4linux/API/V4L2_API/spec-single/v4l2.html#vidioc-qbuf
although it would make sense to do this for USERPTR as well. Maybe I am trying
too hard to stay 100% faithful to the documentation, I guess it should be corrected 
as well then?


Best regards
--
Pawel Osciak
Linux Platform Group
Samsung Poland R&D Center


--
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
  
Laurent Pinchart April 22, 2010, 9:29 a.m. UTC | #3
Hi Pawel,

On Thursday 22 April 2010 11:24:52 Pawel Osciak wrote:
> >Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> >> According to the V4L2 specification, applications set bytesused, field
> >> and timestamp fields of struct v4l2_buffer when the buffer is intended
> >> for output and memory type is MMAP. This adds proper copying of those
> >> values to videobuf_buffer so drivers can use them.
> >
> >Why only for the MMAP memory type ? Don't drivers need the information for
> >USERPTR buffers as well ?
> 
> It is only mentioned for the MMAP memory type:
> http://linuxtv.org/downloads/video4linux/API/V4L2_API/spec-single/v4l2.html
> #vidioc-qbuf although it would make sense to do this for USERPTR as well.
> Maybe I am trying too hard to stay 100% faithful to the documentation, I
> guess it should be corrected as well then?

This wouldn't be the first time the spec is wrong :-) I'd like other people's 
opinion on this, but I think we should fix the spec and copy the values for 
both MMAP and USERPTR.
  
Pawel Osciak April 22, 2010, 9:41 a.m. UTC | #4
>Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
>On Thursday 22 April 2010 11:24:52 Pawel Osciak wrote:
>> >Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
>> >> According to the V4L2 specification, applications set bytesused, field
>> >> and timestamp fields of struct v4l2_buffer when the buffer is intended
>> >> for output and memory type is MMAP. This adds proper copying of those
>> >> values to videobuf_buffer so drivers can use them.
>> >
>> >Why only for the MMAP memory type ? Don't drivers need the information for
>> >USERPTR buffers as well ?
>>
>> It is only mentioned for the MMAP memory type:
>> http://linuxtv.org/downloads/video4linux/API/V4L2_API/spec-single/v4l2.html
>> #vidioc-qbuf although it would make sense to do this for USERPTR as well.
>> Maybe I am trying too hard to stay 100% faithful to the documentation, I
>> guess it should be corrected as well then?
>
>This wouldn't be the first time the spec is wrong :-) I'd like other people's
>opinion on this, but I think we should fix the spec and copy the values for
>both MMAP and USERPTR.

Yes, same here. Thanks for pointing that up. I really have to stop treating the
spec as it was somehow sacred :)

Best regards
--
Pawel Osciak
Linux Platform Group
Samsung Poland R&D Center





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

Patch

diff --git a/drivers/media/video/videobuf-core.c b/drivers/media/video/videobuf-core.c
index 63d7043..e573ca7 100644
--- a/drivers/media/video/videobuf-core.c
+++ b/drivers/media/video/videobuf-core.c
@@ -549,6 +549,13 @@  int videobuf_qbuf(struct videobuf_queue *q, struct v4l2_buffer *b)
 				   "but buffer addr is zero!\n");
 			goto done;
 		}
+		if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT
+		    || q->type == V4L2_BUF_TYPE_VBI_OUTPUT
+		    || q->type == V4L2_BUF_TYPE_SLICED_VBI_OUTPUT) {
+			buf->size = b->bytesused;
+			buf->field = b->field;
+			buf->ts = b->timestamp;
+		}
 		break;
 	case V4L2_MEMORY_USERPTR:
 		if (b->length < buf->bsize) {