| Message ID | 20220317104740.569310-2-umang.jain@ideasonboard.com |
|---|---|
| State | Accepted |
| Delegated to: | Umang Jain |
| Headers | show |
| Series |
|
| Related | show |
Hi Umang, Thank you for the patch. On Thu, Mar 17, 2022 at 04:17:38PM +0530, Umang Jain via libcamera-devel wrote: > The struct v4l2_buffer documentation [1] clearly states > that setting of the sequence is done by the driver. libcamera does > not really need to set this field while queuing the buffer(s). > > [1]: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/buffer.html#struct-v4l2-buffer > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/libcamera/v4l2_videodevice.cpp | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > index 5f36ee20..5580269f 100644 > --- a/src/libcamera/v4l2_videodevice.cpp > +++ b/src/libcamera/v4l2_videodevice.cpp > @@ -1600,7 +1600,6 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer) > buf.length = planes[0].length; > } > > - buf.sequence = metadata.sequence; > buf.timestamp.tv_sec = metadata.timestamp / 1000000000; > buf.timestamp.tv_usec = (metadata.timestamp / 1000) % 1000000; > }
Quoting Laurent Pinchart via libcamera-devel (2022-03-23 00:12:09) > Hi Umang, > > Thank you for the patch. > > On Thu, Mar 17, 2022 at 04:17:38PM +0530, Umang Jain via libcamera-devel wrote: > > The struct v4l2_buffer documentation [1] clearly states > > that setting of the sequence is done by the driver. libcamera does > > not really need to set this field while queuing the buffer(s). > > > > [1]: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/buffer.html#struct-v4l2-buffer > > > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> I always thought sequence numbers should be propogated like the timestamp, but if the kernel doesn't do it - then no point setting it indeed. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > --- > > src/libcamera/v4l2_videodevice.cpp | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > > index 5f36ee20..5580269f 100644 > > --- a/src/libcamera/v4l2_videodevice.cpp > > +++ b/src/libcamera/v4l2_videodevice.cpp > > @@ -1600,7 +1600,6 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer) > > buf.length = planes[0].length; > > } > > > > - buf.sequence = metadata.sequence; > > buf.timestamp.tv_sec = metadata.timestamp / 1000000000; > > buf.timestamp.tv_usec = (metadata.timestamp / 1000) % 1000000; > > } > > -- > Regards, > > Laurent Pinchart
diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 5f36ee20..5580269f 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1600,7 +1600,6 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer) buf.length = planes[0].length; } - buf.sequence = metadata.sequence; buf.timestamp.tv_sec = metadata.timestamp / 1000000000; buf.timestamp.tv_usec = (metadata.timestamp / 1000) % 1000000; }
The struct v4l2_buffer documentation [1] clearly states that setting of the sequence is done by the driver. libcamera does not really need to set this field while queuing the buffer(s). [1]: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/buffer.html#struct-v4l2-buffer Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- src/libcamera/v4l2_videodevice.cpp | 1 - 1 file changed, 1 deletion(-)