| Message ID | 20190808151221.24254-2-kieran.bingham@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, Thank you for the patch. On Thu, Aug 08, 2019 at 04:12:16PM +0100, Kieran Bingham wrote: > Provide a means for V4L2 device instances to set the fd_ explicitly. > This allows for V4L2Devices to operate when they must use an external open() call. Line wrap ? > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/libcamera/include/v4l2_device.h | 1 + > src/libcamera/v4l2_device.cpp | 19 +++++++++++++++++++ > 2 files changed, 20 insertions(+) > > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h > index e7e9829cb05f..7c3af5f6ebe0 100644 > --- a/src/libcamera/include/v4l2_device.h > +++ b/src/libcamera/include/v4l2_device.h > @@ -39,6 +39,7 @@ protected: > int ioctl(unsigned long request, void *argp); > > int fd() { return fd_; } > + int setFd(int fd); > > private: > void listControls(); > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > index 9a00566a532b..120f68e49d7a 100644 > --- a/src/libcamera/v4l2_device.cpp > +++ b/src/libcamera/v4l2_device.cpp > @@ -308,6 +308,25 @@ int V4L2Device::ioctl(unsigned long request, void *argp) > * \return The V4L2 device file descriptor, -1 if the device node is not open > */ > > +/** > + * \brief Set the file descriptor of a V4L2 device. s/\.$// > + * \param[in] fd The file descriptor handle > + * > + * Allow a device to set the internal file descriptor rather than opening a > + * device node directly. How about the following ? * This method allows a device to provide an already opened file descriptor * referring to the V4L2 device node, instead of opening it with open(). This * can be used for V4L2 M2M devices where a single video device node is used for * both the output and capture devices, or when receiving an open file * descriptor in a context that doesn't have permission to open the device node * itself. * * This method and the open() method are mutually exclusive, only one of the two * shall be used for a V4L2Device instance. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + * > + * \return 0 on success or a negative error code otherwise > + */ > +int V4L2Device::setFd(int fd) > +{ > + if (fd_ != -1) > + return -1; > + > + fd_ = fd; > + > + return 0; > +} > + > /* > * \brief List and store information about all controls supported by the > * V4L2 device
On 08/08/2019 21:34, Laurent Pinchart wrote: > Hi Kieran, > > Thank you for the patch. > > On Thu, Aug 08, 2019 at 04:12:16PM +0100, Kieran Bingham wrote: >> Provide a means for V4L2 device instances to set the fd_ explicitly. >> This allows for V4L2Devices to operate when they must use an external open() call. > > Line wrap ? Fixed, > >> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> --- >> src/libcamera/include/v4l2_device.h | 1 + >> src/libcamera/v4l2_device.cpp | 19 +++++++++++++++++++ >> 2 files changed, 20 insertions(+) >> >> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h >> index e7e9829cb05f..7c3af5f6ebe0 100644 >> --- a/src/libcamera/include/v4l2_device.h >> +++ b/src/libcamera/include/v4l2_device.h >> @@ -39,6 +39,7 @@ protected: >> int ioctl(unsigned long request, void *argp); >> >> int fd() { return fd_; } >> + int setFd(int fd); >> >> private: >> void listControls(); >> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp >> index 9a00566a532b..120f68e49d7a 100644 >> --- a/src/libcamera/v4l2_device.cpp >> +++ b/src/libcamera/v4l2_device.cpp >> @@ -308,6 +308,25 @@ int V4L2Device::ioctl(unsigned long request, void *argp) >> * \return The V4L2 device file descriptor, -1 if the device node is not open >> */ >> >> +/** >> + * \brief Set the file descriptor of a V4L2 device. > > s/\.$// Removed > >> + * \param[in] fd The file descriptor handle >> + * >> + * Allow a device to set the internal file descriptor rather than opening a >> + * device node directly. > > How about the following ? > > * This method allows a device to provide an already opened file descriptor > * referring to the V4L2 device node, instead of opening it with open(). This > * can be used for V4L2 M2M devices where a single video device node is used for > * both the output and capture devices, or when receiving an open file > * descriptor in a context that doesn't have permission to open the device node > * itself. > * > * This method and the open() method are mutually exclusive, only one of the two > * shall be used for a V4L2Device instance. Added verbatim. > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Collected. > >> + * >> + * \return 0 on success or a negative error code otherwise >> + */ >> +int V4L2Device::setFd(int fd) >> +{ >> + if (fd_ != -1) >> + return -1; >> + >> + fd_ = fd; >> + >> + return 0; >> +} >> + >> /* >> * \brief List and store information about all controls supported by the >> * V4L2 device >
Hello, On Thu, Aug 08, 2019 at 11:34:16PM +0300, Laurent Pinchart wrote: > Hi Kieran, > > Thank you for the patch. > > On Thu, Aug 08, 2019 at 04:12:16PM +0100, Kieran Bingham wrote: > > Provide a means for V4L2 device instances to set the fd_ explicitly. > > This allows for V4L2Devices to operate when they must use an external open() call. > > Line wrap ? > > > > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > --- > > src/libcamera/include/v4l2_device.h | 1 + > > src/libcamera/v4l2_device.cpp | 19 +++++++++++++++++++ > > 2 files changed, 20 insertions(+) > > > > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h > > index e7e9829cb05f..7c3af5f6ebe0 100644 > > --- a/src/libcamera/include/v4l2_device.h > > +++ b/src/libcamera/include/v4l2_device.h > > @@ -39,6 +39,7 @@ protected: > > int ioctl(unsigned long request, void *argp); > > > > int fd() { return fd_; } > > + int setFd(int fd); > > > > private: > > void listControls(); > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > > index 9a00566a532b..120f68e49d7a 100644 > > --- a/src/libcamera/v4l2_device.cpp > > +++ b/src/libcamera/v4l2_device.cpp > > @@ -308,6 +308,25 @@ int V4L2Device::ioctl(unsigned long request, void *argp) > > * \return The V4L2 device file descriptor, -1 if the device node is not open > > */ > > > > +/** > > + * \brief Set the file descriptor of a V4L2 device. > > s/\.$// > > > + * \param[in] fd The file descriptor handle > > + * > > + * Allow a device to set the internal file descriptor rather than opening a > > + * device node directly. > > How about the following ? > > * This method allows a device to provide an already opened file descriptor > * referring to the V4L2 device node, instead of opening it with open(). This > * can be used for V4L2 M2M devices where a single video device node is used for > * both the output and capture devices, or when receiving an open file > * descriptor in a context that doesn't have permission to open the device node > * itself. > * > * This method and the open() method are mutually exclusive, only one of the two > * shall be used for a V4L2Device instance. Thanks, this gives some more context on why this change is needed Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > + * > > + * \return 0 on success or a negative error code otherwise > > + */ > > +int V4L2Device::setFd(int fd) > > +{ > > + if (fd_ != -1) > > + return -1; > > + > > + fd_ = fd; > > + > > + return 0; > > +} > > + > > /* > > * \brief List and store information about all controls supported by the > > * V4L2 device > > -- > Regards, > > Laurent Pinchart > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h index e7e9829cb05f..7c3af5f6ebe0 100644 --- a/src/libcamera/include/v4l2_device.h +++ b/src/libcamera/include/v4l2_device.h @@ -39,6 +39,7 @@ protected: int ioctl(unsigned long request, void *argp); int fd() { return fd_; } + int setFd(int fd); private: void listControls(); diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 9a00566a532b..120f68e49d7a 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -308,6 +308,25 @@ int V4L2Device::ioctl(unsigned long request, void *argp) * \return The V4L2 device file descriptor, -1 if the device node is not open */ +/** + * \brief Set the file descriptor of a V4L2 device. + * \param[in] fd The file descriptor handle + * + * Allow a device to set the internal file descriptor rather than opening a + * device node directly. + * + * \return 0 on success or a negative error code otherwise + */ +int V4L2Device::setFd(int fd) +{ + if (fd_ != -1) + return -1; + + fd_ = fd; + + return 0; +} + /* * \brief List and store information about all controls supported by the * V4L2 device
Provide a means for V4L2 device instances to set the fd_ explicitly. This allows for V4L2Devices to operate when they must use an external open() call. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- src/libcamera/include/v4l2_device.h | 1 + src/libcamera/v4l2_device.cpp | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+)