Re: [REVIEW PATCH 06/11] vb2: set timestamp when using write()
On 04/07/2014 10:32 AM, Pawel Osciak wrote: > On Tue, Mar 11, 2014 at 6:20 AM, Hans Verkuil wrote: >> From: Hans Verkuil >> >> When using write() to write data to an output video node the vb2 core >> should set timestamps if V4L2_BUF_FLAG_TIMESTAMP_COPY is set. Nobody > > I'm confused. Shouldn't we be saving the existing timestamp from the buffer if > V4L2_BUF_FLAG_TIMESTAMP_COPY is true, instead of getting it from > v4l2_get_timestamp()? When using the write() file operation the application has no way of setting the timestamp. So it is uninitialized and the reader on the other side receives an uninitialized (or 0, I'm not sure) timestamp. So __vb2_perform_fileio() has to fill in a valid timestamp instead. It's a corner case. Regards, Hans > >> else is able to provide this information with the write() operation. >> >> Signed-off-by: Hans Verkuil >> --- >> drivers/media/v4l2-core/videobuf2-core.c | 6 ++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/media/v4l2-core/videobuf2-core.c >> b/drivers/media/v4l2-core/videobuf2-core.c >> index e38b45e..afd1268 100644 >> --- a/drivers/media/v4l2-core/videobuf2-core.c >> +++ b/drivers/media/v4l2-core/videobuf2-core.c >> @@ -22,6 +22,7 @@ >> #include >> #include >> #include >> +#include >> #include >> >> static int debug; >> @@ -2767,6 +2768,9 @@ static size_t __vb2_perform_fileio(struct vb2_queue >> *q, char __user *data, size_ >> { >> struct vb2_fileio_data *fileio; >> struct vb2_fileio_buf *buf; >> + bool set_timestamp = !read && >> + (q->timestamp_flags & V4L2_BUF_FLAG_TIMESTAMP_MASK) == >> + V4L2_BUF_FLAG_TIMESTAMP_COPY; >> int ret, index; >> >> dprintk(3, "file io: mode %s, offset %ld, count %zd, %sblocking\n", >> @@ -2868,6 +2872,8 @@ static size_t __vb2_perform_fileio(struct vb2_queue >> *q, char __user *data, size_ >> fileio->b.memory = q->memory; >> fileio->b.index = index; >> fileio->b.bytesused = buf->pos; >> + if (set_timestamp) >> + v4l2_get_timestamp(&fileio->b.timestamp); >> ret = vb2_internal_qbuf(q, &fileio->b); >> dprintk(5, "file io: vb2_internal_qbuf result: %d\n", ret); >> if (ret) >> -- >> 1.9.0 >> > > > -- 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: [REVIEW PATCH 06/11] vb2: set timestamp when using write()
On Tue, Mar 11, 2014 at 6:20 AM, Hans Verkuil wrote: > From: Hans Verkuil > > When using write() to write data to an output video node the vb2 core > should set timestamps if V4L2_BUF_FLAG_TIMESTAMP_COPY is set. Nobody I'm confused. Shouldn't we be saving the existing timestamp from the buffer if V4L2_BUF_FLAG_TIMESTAMP_COPY is true, instead of getting it from v4l2_get_timestamp()? > else is able to provide this information with the write() operation. > > Signed-off-by: Hans Verkuil > --- > drivers/media/v4l2-core/videobuf2-core.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/media/v4l2-core/videobuf2-core.c > b/drivers/media/v4l2-core/videobuf2-core.c > index e38b45e..afd1268 100644 > --- a/drivers/media/v4l2-core/videobuf2-core.c > +++ b/drivers/media/v4l2-core/videobuf2-core.c > @@ -22,6 +22,7 @@ > #include > #include > #include > +#include > #include > > static int debug; > @@ -2767,6 +2768,9 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, > char __user *data, size_ > { > struct vb2_fileio_data *fileio; > struct vb2_fileio_buf *buf; > + bool set_timestamp = !read && > + (q->timestamp_flags & V4L2_BUF_FLAG_TIMESTAMP_MASK) == > + V4L2_BUF_FLAG_TIMESTAMP_COPY; > int ret, index; > > dprintk(3, "file io: mode %s, offset %ld, count %zd, %sblocking\n", > @@ -2868,6 +2872,8 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, > char __user *data, size_ > fileio->b.memory = q->memory; > fileio->b.index = index; > fileio->b.bytesused = buf->pos; > + if (set_timestamp) > + v4l2_get_timestamp(&fileio->b.timestamp); > ret = vb2_internal_qbuf(q, &fileio->b); > dprintk(5, "file io: vb2_internal_qbuf result: %d\n", ret); > if (ret) > -- > 1.9.0 > -- Best regards, Pawel Osciak -- 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
[REVIEW PATCH 06/11] vb2: set timestamp when using write()
From: Hans Verkuil When using write() to write data to an output video node the vb2 core should set timestamps if V4L2_BUF_FLAG_TIMESTAMP_COPY is set. Nobody else is able to provide this information with the write() operation. Signed-off-by: Hans Verkuil --- drivers/media/v4l2-core/videobuf2-core.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index e38b45e..afd1268 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -22,6 +22,7 @@ #include #include #include +#include #include static int debug; @@ -2767,6 +2768,9 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ { struct vb2_fileio_data *fileio; struct vb2_fileio_buf *buf; + bool set_timestamp = !read && + (q->timestamp_flags & V4L2_BUF_FLAG_TIMESTAMP_MASK) == + V4L2_BUF_FLAG_TIMESTAMP_COPY; int ret, index; dprintk(3, "file io: mode %s, offset %ld, count %zd, %sblocking\n", @@ -2868,6 +2872,8 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ fileio->b.memory = q->memory; fileio->b.index = index; fileio->b.bytesused = buf->pos; + if (set_timestamp) + v4l2_get_timestamp(&fileio->b.timestamp); ret = vb2_internal_qbuf(q, &fileio->b); dprintk(5, "file io: vb2_internal_qbuf result: %d\n", ret); if (ret) -- 1.9.0 -- 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