| Message ID | 20200316234649.2545-3-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, Thanks for your work. On 2020-03-17 01:46:49 +0200, Laurent Pinchart wrote: > To achieve the goal of preventing unwanted conversion between a DRM and > a V4L2 FourCC, make the V4L2PixelFormat constructor that takes an > integer value explicit. All users of V4L2 pixel formats flagged by the > compiler are fixed. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/include/v4l2_videodevice.h | 2 +- > src/libcamera/pipeline/ipu3/ipu3.cpp | 14 +++---- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +- > src/libcamera/pipeline/vimc.cpp | 2 +- > src/libcamera/v4l2_videodevice.cpp | 42 ++++++++++--------- > .../v4l2_videodevice_test.cpp | 2 +- > 6 files changed, 35 insertions(+), 31 deletions(-) > > diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h > index 49d2ca357efa..9a123ce8c50e 100644 > --- a/src/libcamera/include/v4l2_videodevice.h > +++ b/src/libcamera/include/v4l2_videodevice.h > @@ -157,7 +157,7 @@ public: > { > } > > - V4L2PixelFormat(uint32_t fourcc) > + explicit V4L2PixelFormat(uint32_t fourcc) > : fourcc_(fourcc) > { > } > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 90de7749f623..123b184023c3 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -121,7 +121,7 @@ public: > int start(); > int stop(); > > - static int mediaBusToFormat(unsigned int code); > + static V4L2PixelFormat mediaBusToFormat(unsigned int code); > > V4L2VideoDevice *output_; > V4L2Subdevice *csi2_; > @@ -1456,19 +1456,19 @@ int CIO2Device::stop() > return output_->streamOff(); > } > > -int CIO2Device::mediaBusToFormat(unsigned int code) > +V4L2PixelFormat CIO2Device::mediaBusToFormat(unsigned int code) > { > switch (code) { > case MEDIA_BUS_FMT_SBGGR10_1X10: > - return V4L2_PIX_FMT_IPU3_SBGGR10; > + return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10); > case MEDIA_BUS_FMT_SGBRG10_1X10: > - return V4L2_PIX_FMT_IPU3_SGBRG10; > + return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10); > case MEDIA_BUS_FMT_SGRBG10_1X10: > - return V4L2_PIX_FMT_IPU3_SGRBG10; > + return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10); > case MEDIA_BUS_FMT_SRGGB10_1X10: > - return V4L2_PIX_FMT_IPU3_SRGGB10; > + return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10); > default: > - return -EINVAL; > + return {}; > } > } > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index beed38956662..96ab8ea45931 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -645,13 +645,13 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) > } > > V4L2DeviceFormat paramFormat = {}; > - paramFormat.fourcc = V4L2_META_FMT_RK_ISP1_PARAMS; > + paramFormat.fourcc = V4L2PixelFormat(V4L2_META_FMT_RK_ISP1_PARAMS); > ret = param_->setFormat(¶mFormat); > if (ret) > return ret; > > V4L2DeviceFormat statFormat = {}; > - statFormat.fourcc = V4L2_META_FMT_RK_ISP1_STAT_3A; > + statFormat.fourcc = V4L2PixelFormat(V4L2_META_FMT_RK_ISP1_STAT_3A); > ret = stat_->setFormat(&statFormat); > if (ret) > return ret; > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > index eedef85866a1..83ffb5f16efa 100644 > --- a/src/libcamera/pipeline/vimc.cpp > +++ b/src/libcamera/pipeline/vimc.cpp > @@ -247,7 +247,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config) > * Format has to be set on the raw capture video node, otherwise the > * vimc driver will fail pipeline validation. > */ > - format.fourcc = V4L2_PIX_FMT_SGRBG8; > + format.fourcc = V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8); > format.size = { cfg.size.width / 3, cfg.size.height / 3 }; > > ret = data->raw_->setFormat(&format); > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > index 40396c22aa45..6f59487593ae 100644 > --- a/src/libcamera/v4l2_videodevice.cpp > +++ b/src/libcamera/v4l2_videodevice.cpp > @@ -286,6 +286,10 @@ bool V4L2BufferCache::Entry::operator==(const FrameBuffer &buffer) const > * V4L2 pixel formats. Its purpose is to prevent unintentional confusion of > * V4L2 and DRM FourCCs in code by catching implicit conversion attempts at > * compile time. > + * > + * To achieve this goal, construction of a V4L2PixelFormat from an integer value > + * is explicit. To retrieve the integer value of a V4L2PixelFormat, both the > + * explicit value() and implicit uint32_t conversion operators may be used. > */ > > /** > @@ -719,7 +723,7 @@ int V4L2VideoDevice::getFormatMeta(V4L2DeviceFormat *format) > > format->size.width = 0; > format->size.height = 0; > - format->fourcc = pix->dataformat; > + format->fourcc = V4L2PixelFormat(pix->dataformat); > format->planesCount = 1; > format->planes[0].bpl = pix->buffersize; > format->planes[0].size = pix->buffersize; > @@ -771,7 +775,7 @@ int V4L2VideoDevice::getFormatMultiplane(V4L2DeviceFormat *format) > > format->size.width = pix->width; > format->size.height = pix->height; > - format->fourcc = pix->pixelformat; > + format->fourcc = V4L2PixelFormat(pix->pixelformat); > format->planesCount = pix->num_planes; > > for (unsigned int i = 0; i < format->planesCount; ++i) { > @@ -812,7 +816,7 @@ int V4L2VideoDevice::setFormatMultiplane(V4L2DeviceFormat *format) > */ > format->size.width = pix->width; > format->size.height = pix->height; > - format->fourcc = pix->pixelformat; > + format->fourcc = V4L2PixelFormat(pix->pixelformat); > format->planesCount = pix->num_planes; > for (unsigned int i = 0; i < format->planesCount; ++i) { > format->planes[i].bpl = pix->plane_fmt[i].bytesperline; > @@ -837,7 +841,7 @@ int V4L2VideoDevice::getFormatSingleplane(V4L2DeviceFormat *format) > > format->size.width = pix->width; > format->size.height = pix->height; > - format->fourcc = pix->pixelformat; > + format->fourcc = V4L2PixelFormat(pix->pixelformat); > format->planesCount = 1; > format->planes[0].bpl = pix->bytesperline; > format->planes[0].size = pix->sizeimage; > @@ -869,7 +873,7 @@ int V4L2VideoDevice::setFormatSingleplane(V4L2DeviceFormat *format) > */ > format->size.width = pix->width; > format->size.height = pix->height; > - format->fourcc = pix->pixelformat; > + format->fourcc = V4L2PixelFormat(pix->pixelformat); > format->planesCount = 1; > format->planes[0].bpl = pix->bytesperline; > format->planes[0].size = pix->sizeimage; > @@ -913,7 +917,7 @@ std::vector<V4L2PixelFormat> V4L2VideoDevice::enumPixelformats() > if (ret) > break; > > - formats.push_back(pixelformatEnum.pixelformat); > + formats.push_back(V4L2PixelFormat(pixelformatEnum.pixelformat)); > } > > if (ret && ret != -EINVAL) { > @@ -1545,21 +1549,21 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2Fourcc(PixelFormat pixelFormat, bool mult > switch (pixelFormat.fourcc()) { > /* RGB formats. */ > case DRM_FORMAT_BGR888: > - return V4L2_PIX_FMT_RGB24; > + return V4L2PixelFormat(V4L2_PIX_FMT_RGB24); > case DRM_FORMAT_RGB888: > - return V4L2_PIX_FMT_BGR24; > + return V4L2PixelFormat(V4L2_PIX_FMT_BGR24); > case DRM_FORMAT_BGRA8888: > - return V4L2_PIX_FMT_ARGB32; > + return V4L2PixelFormat(V4L2_PIX_FMT_ARGB32); > > /* YUV packed formats. */ > case DRM_FORMAT_YUYV: > - return V4L2_PIX_FMT_YUYV; > + return V4L2PixelFormat(V4L2_PIX_FMT_YUYV); > case DRM_FORMAT_YVYU: > - return V4L2_PIX_FMT_YVYU; > + return V4L2PixelFormat(V4L2_PIX_FMT_YVYU); > case DRM_FORMAT_UYVY: > - return V4L2_PIX_FMT_UYVY; > + return V4L2PixelFormat(V4L2_PIX_FMT_UYVY); > case DRM_FORMAT_VYUY: > - return V4L2_PIX_FMT_VYUY; > + return V4L2PixelFormat(V4L2_PIX_FMT_VYUY); > > /* > * YUY planar formats. > @@ -1568,17 +1572,17 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2Fourcc(PixelFormat pixelFormat, bool mult > * also take into account the formats supported by the device. > */ > case DRM_FORMAT_NV16: > - return V4L2_PIX_FMT_NV16; > + return V4L2PixelFormat(V4L2_PIX_FMT_NV16); > case DRM_FORMAT_NV61: > - return V4L2_PIX_FMT_NV61; > + return V4L2PixelFormat(V4L2_PIX_FMT_NV61); > case DRM_FORMAT_NV12: > - return V4L2_PIX_FMT_NV12; > + return V4L2PixelFormat(V4L2_PIX_FMT_NV12); > case DRM_FORMAT_NV21: > - return V4L2_PIX_FMT_NV21; > + return V4L2PixelFormat(V4L2_PIX_FMT_NV21); > > /* Compressed formats. */ > case DRM_FORMAT_MJPEG: > - return V4L2_PIX_FMT_MJPEG; > + return V4L2PixelFormat(V4L2_PIX_FMT_MJPEG); > } > > /* > @@ -1587,7 +1591,7 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2Fourcc(PixelFormat pixelFormat, bool mult > */ > libcamera::_log(__FILE__, __LINE__, _LOG_CATEGORY(V4L2)(), LogError).stream() > << "Unsupported V4L2 pixel format " << pixelFormat.toString(); > - return 0; > + return {}; > } > > /** > diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.cpp b/test/v4l2_videodevice/v4l2_videodevice_test.cpp > index 577da4cb601c..93b9e72da5b4 100644 > --- a/test/v4l2_videodevice/v4l2_videodevice_test.cpp > +++ b/test/v4l2_videodevice/v4l2_videodevice_test.cpp > @@ -69,7 +69,7 @@ int V4L2VideoDeviceTest::init() > if (debayer_->open()) > return TestFail; > > - format.fourcc = V4L2_PIX_FMT_SBGGR8; > + format.fourcc = V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8); > > V4L2SubdeviceFormat subformat = {}; > subformat.mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8; > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Laurent, On Tue, Mar 17, 2020 at 01:46:49AM +0200, Laurent Pinchart wrote: > To achieve the goal of preventing unwanted conversion between a DRM and > a V4L2 FourCC, make the V4L2PixelFormat constructor that takes an > integer value explicit. All users of V4L2 pixel formats flagged by the > compiler are fixed. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Thanks! Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/libcamera/include/v4l2_videodevice.h | 2 +- > src/libcamera/pipeline/ipu3/ipu3.cpp | 14 +++---- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +- > src/libcamera/pipeline/vimc.cpp | 2 +- > src/libcamera/v4l2_videodevice.cpp | 42 ++++++++++--------- > .../v4l2_videodevice_test.cpp | 2 +- > 6 files changed, 35 insertions(+), 31 deletions(-) > > diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h > index 49d2ca357efa..9a123ce8c50e 100644 > --- a/src/libcamera/include/v4l2_videodevice.h > +++ b/src/libcamera/include/v4l2_videodevice.h > @@ -157,7 +157,7 @@ public: > { > } > > - V4L2PixelFormat(uint32_t fourcc) > + explicit V4L2PixelFormat(uint32_t fourcc) > : fourcc_(fourcc) > { > } > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 90de7749f623..123b184023c3 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -121,7 +121,7 @@ public: > int start(); > int stop(); > > - static int mediaBusToFormat(unsigned int code); > + static V4L2PixelFormat mediaBusToFormat(unsigned int code); > > V4L2VideoDevice *output_; > V4L2Subdevice *csi2_; > @@ -1456,19 +1456,19 @@ int CIO2Device::stop() > return output_->streamOff(); > } > > -int CIO2Device::mediaBusToFormat(unsigned int code) > +V4L2PixelFormat CIO2Device::mediaBusToFormat(unsigned int code) > { > switch (code) { > case MEDIA_BUS_FMT_SBGGR10_1X10: > - return V4L2_PIX_FMT_IPU3_SBGGR10; > + return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10); > case MEDIA_BUS_FMT_SGBRG10_1X10: > - return V4L2_PIX_FMT_IPU3_SGBRG10; > + return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10); > case MEDIA_BUS_FMT_SGRBG10_1X10: > - return V4L2_PIX_FMT_IPU3_SGRBG10; > + return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10); > case MEDIA_BUS_FMT_SRGGB10_1X10: > - return V4L2_PIX_FMT_IPU3_SRGGB10; > + return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10); > default: > - return -EINVAL; > + return {}; > } > } > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index beed38956662..96ab8ea45931 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -645,13 +645,13 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) > } > > V4L2DeviceFormat paramFormat = {}; > - paramFormat.fourcc = V4L2_META_FMT_RK_ISP1_PARAMS; > + paramFormat.fourcc = V4L2PixelFormat(V4L2_META_FMT_RK_ISP1_PARAMS); > ret = param_->setFormat(¶mFormat); > if (ret) > return ret; > > V4L2DeviceFormat statFormat = {}; > - statFormat.fourcc = V4L2_META_FMT_RK_ISP1_STAT_3A; > + statFormat.fourcc = V4L2PixelFormat(V4L2_META_FMT_RK_ISP1_STAT_3A); > ret = stat_->setFormat(&statFormat); > if (ret) > return ret; > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > index eedef85866a1..83ffb5f16efa 100644 > --- a/src/libcamera/pipeline/vimc.cpp > +++ b/src/libcamera/pipeline/vimc.cpp > @@ -247,7 +247,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config) > * Format has to be set on the raw capture video node, otherwise the > * vimc driver will fail pipeline validation. > */ > - format.fourcc = V4L2_PIX_FMT_SGRBG8; > + format.fourcc = V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8); > format.size = { cfg.size.width / 3, cfg.size.height / 3 }; > > ret = data->raw_->setFormat(&format); > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > index 40396c22aa45..6f59487593ae 100644 > --- a/src/libcamera/v4l2_videodevice.cpp > +++ b/src/libcamera/v4l2_videodevice.cpp > @@ -286,6 +286,10 @@ bool V4L2BufferCache::Entry::operator==(const FrameBuffer &buffer) const > * V4L2 pixel formats. Its purpose is to prevent unintentional confusion of > * V4L2 and DRM FourCCs in code by catching implicit conversion attempts at > * compile time. > + * > + * To achieve this goal, construction of a V4L2PixelFormat from an integer value > + * is explicit. To retrieve the integer value of a V4L2PixelFormat, both the > + * explicit value() and implicit uint32_t conversion operators may be used. > */ > > /** > @@ -719,7 +723,7 @@ int V4L2VideoDevice::getFormatMeta(V4L2DeviceFormat *format) > > format->size.width = 0; > format->size.height = 0; > - format->fourcc = pix->dataformat; > + format->fourcc = V4L2PixelFormat(pix->dataformat); > format->planesCount = 1; > format->planes[0].bpl = pix->buffersize; > format->planes[0].size = pix->buffersize; > @@ -771,7 +775,7 @@ int V4L2VideoDevice::getFormatMultiplane(V4L2DeviceFormat *format) > > format->size.width = pix->width; > format->size.height = pix->height; > - format->fourcc = pix->pixelformat; > + format->fourcc = V4L2PixelFormat(pix->pixelformat); > format->planesCount = pix->num_planes; > > for (unsigned int i = 0; i < format->planesCount; ++i) { > @@ -812,7 +816,7 @@ int V4L2VideoDevice::setFormatMultiplane(V4L2DeviceFormat *format) > */ > format->size.width = pix->width; > format->size.height = pix->height; > - format->fourcc = pix->pixelformat; > + format->fourcc = V4L2PixelFormat(pix->pixelformat); > format->planesCount = pix->num_planes; > for (unsigned int i = 0; i < format->planesCount; ++i) { > format->planes[i].bpl = pix->plane_fmt[i].bytesperline; > @@ -837,7 +841,7 @@ int V4L2VideoDevice::getFormatSingleplane(V4L2DeviceFormat *format) > > format->size.width = pix->width; > format->size.height = pix->height; > - format->fourcc = pix->pixelformat; > + format->fourcc = V4L2PixelFormat(pix->pixelformat); > format->planesCount = 1; > format->planes[0].bpl = pix->bytesperline; > format->planes[0].size = pix->sizeimage; > @@ -869,7 +873,7 @@ int V4L2VideoDevice::setFormatSingleplane(V4L2DeviceFormat *format) > */ > format->size.width = pix->width; > format->size.height = pix->height; > - format->fourcc = pix->pixelformat; > + format->fourcc = V4L2PixelFormat(pix->pixelformat); > format->planesCount = 1; > format->planes[0].bpl = pix->bytesperline; > format->planes[0].size = pix->sizeimage; > @@ -913,7 +917,7 @@ std::vector<V4L2PixelFormat> V4L2VideoDevice::enumPixelformats() > if (ret) > break; > > - formats.push_back(pixelformatEnum.pixelformat); > + formats.push_back(V4L2PixelFormat(pixelformatEnum.pixelformat)); > } > > if (ret && ret != -EINVAL) { > @@ -1545,21 +1549,21 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2Fourcc(PixelFormat pixelFormat, bool mult > switch (pixelFormat.fourcc()) { > /* RGB formats. */ > case DRM_FORMAT_BGR888: > - return V4L2_PIX_FMT_RGB24; > + return V4L2PixelFormat(V4L2_PIX_FMT_RGB24); > case DRM_FORMAT_RGB888: > - return V4L2_PIX_FMT_BGR24; > + return V4L2PixelFormat(V4L2_PIX_FMT_BGR24); > case DRM_FORMAT_BGRA8888: > - return V4L2_PIX_FMT_ARGB32; > + return V4L2PixelFormat(V4L2_PIX_FMT_ARGB32); > > /* YUV packed formats. */ > case DRM_FORMAT_YUYV: > - return V4L2_PIX_FMT_YUYV; > + return V4L2PixelFormat(V4L2_PIX_FMT_YUYV); > case DRM_FORMAT_YVYU: > - return V4L2_PIX_FMT_YVYU; > + return V4L2PixelFormat(V4L2_PIX_FMT_YVYU); > case DRM_FORMAT_UYVY: > - return V4L2_PIX_FMT_UYVY; > + return V4L2PixelFormat(V4L2_PIX_FMT_UYVY); > case DRM_FORMAT_VYUY: > - return V4L2_PIX_FMT_VYUY; > + return V4L2PixelFormat(V4L2_PIX_FMT_VYUY); > > /* > * YUY planar formats. > @@ -1568,17 +1572,17 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2Fourcc(PixelFormat pixelFormat, bool mult > * also take into account the formats supported by the device. > */ > case DRM_FORMAT_NV16: > - return V4L2_PIX_FMT_NV16; > + return V4L2PixelFormat(V4L2_PIX_FMT_NV16); > case DRM_FORMAT_NV61: > - return V4L2_PIX_FMT_NV61; > + return V4L2PixelFormat(V4L2_PIX_FMT_NV61); > case DRM_FORMAT_NV12: > - return V4L2_PIX_FMT_NV12; > + return V4L2PixelFormat(V4L2_PIX_FMT_NV12); > case DRM_FORMAT_NV21: > - return V4L2_PIX_FMT_NV21; > + return V4L2PixelFormat(V4L2_PIX_FMT_NV21); > > /* Compressed formats. */ > case DRM_FORMAT_MJPEG: > - return V4L2_PIX_FMT_MJPEG; > + return V4L2PixelFormat(V4L2_PIX_FMT_MJPEG); > } > > /* > @@ -1587,7 +1591,7 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2Fourcc(PixelFormat pixelFormat, bool mult > */ > libcamera::_log(__FILE__, __LINE__, _LOG_CATEGORY(V4L2)(), LogError).stream() > << "Unsupported V4L2 pixel format " << pixelFormat.toString(); > - return 0; > + return {}; > } > > /** > diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.cpp b/test/v4l2_videodevice/v4l2_videodevice_test.cpp > index 577da4cb601c..93b9e72da5b4 100644 > --- a/test/v4l2_videodevice/v4l2_videodevice_test.cpp > +++ b/test/v4l2_videodevice/v4l2_videodevice_test.cpp > @@ -69,7 +69,7 @@ int V4L2VideoDeviceTest::init() > if (debayer_->open()) > return TestFail; > > - format.fourcc = V4L2_PIX_FMT_SBGGR8; > + format.fourcc = V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8); > > V4L2SubdeviceFormat subformat = {}; > subformat.mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8; > -- > 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_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index 49d2ca357efa..9a123ce8c50e 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -157,7 +157,7 @@ public: { } - V4L2PixelFormat(uint32_t fourcc) + explicit V4L2PixelFormat(uint32_t fourcc) : fourcc_(fourcc) { } diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 90de7749f623..123b184023c3 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -121,7 +121,7 @@ public: int start(); int stop(); - static int mediaBusToFormat(unsigned int code); + static V4L2PixelFormat mediaBusToFormat(unsigned int code); V4L2VideoDevice *output_; V4L2Subdevice *csi2_; @@ -1456,19 +1456,19 @@ int CIO2Device::stop() return output_->streamOff(); } -int CIO2Device::mediaBusToFormat(unsigned int code) +V4L2PixelFormat CIO2Device::mediaBusToFormat(unsigned int code) { switch (code) { case MEDIA_BUS_FMT_SBGGR10_1X10: - return V4L2_PIX_FMT_IPU3_SBGGR10; + return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10); case MEDIA_BUS_FMT_SGBRG10_1X10: - return V4L2_PIX_FMT_IPU3_SGBRG10; + return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10); case MEDIA_BUS_FMT_SGRBG10_1X10: - return V4L2_PIX_FMT_IPU3_SGRBG10; + return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10); case MEDIA_BUS_FMT_SRGGB10_1X10: - return V4L2_PIX_FMT_IPU3_SRGGB10; + return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10); default: - return -EINVAL; + return {}; } } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index beed38956662..96ab8ea45931 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -645,13 +645,13 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) } V4L2DeviceFormat paramFormat = {}; - paramFormat.fourcc = V4L2_META_FMT_RK_ISP1_PARAMS; + paramFormat.fourcc = V4L2PixelFormat(V4L2_META_FMT_RK_ISP1_PARAMS); ret = param_->setFormat(¶mFormat); if (ret) return ret; V4L2DeviceFormat statFormat = {}; - statFormat.fourcc = V4L2_META_FMT_RK_ISP1_STAT_3A; + statFormat.fourcc = V4L2PixelFormat(V4L2_META_FMT_RK_ISP1_STAT_3A); ret = stat_->setFormat(&statFormat); if (ret) return ret; diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index eedef85866a1..83ffb5f16efa 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -247,7 +247,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config) * Format has to be set on the raw capture video node, otherwise the * vimc driver will fail pipeline validation. */ - format.fourcc = V4L2_PIX_FMT_SGRBG8; + format.fourcc = V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8); format.size = { cfg.size.width / 3, cfg.size.height / 3 }; ret = data->raw_->setFormat(&format); diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 40396c22aa45..6f59487593ae 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -286,6 +286,10 @@ bool V4L2BufferCache::Entry::operator==(const FrameBuffer &buffer) const * V4L2 pixel formats. Its purpose is to prevent unintentional confusion of * V4L2 and DRM FourCCs in code by catching implicit conversion attempts at * compile time. + * + * To achieve this goal, construction of a V4L2PixelFormat from an integer value + * is explicit. To retrieve the integer value of a V4L2PixelFormat, both the + * explicit value() and implicit uint32_t conversion operators may be used. */ /** @@ -719,7 +723,7 @@ int V4L2VideoDevice::getFormatMeta(V4L2DeviceFormat *format) format->size.width = 0; format->size.height = 0; - format->fourcc = pix->dataformat; + format->fourcc = V4L2PixelFormat(pix->dataformat); format->planesCount = 1; format->planes[0].bpl = pix->buffersize; format->planes[0].size = pix->buffersize; @@ -771,7 +775,7 @@ int V4L2VideoDevice::getFormatMultiplane(V4L2DeviceFormat *format) format->size.width = pix->width; format->size.height = pix->height; - format->fourcc = pix->pixelformat; + format->fourcc = V4L2PixelFormat(pix->pixelformat); format->planesCount = pix->num_planes; for (unsigned int i = 0; i < format->planesCount; ++i) { @@ -812,7 +816,7 @@ int V4L2VideoDevice::setFormatMultiplane(V4L2DeviceFormat *format) */ format->size.width = pix->width; format->size.height = pix->height; - format->fourcc = pix->pixelformat; + format->fourcc = V4L2PixelFormat(pix->pixelformat); format->planesCount = pix->num_planes; for (unsigned int i = 0; i < format->planesCount; ++i) { format->planes[i].bpl = pix->plane_fmt[i].bytesperline; @@ -837,7 +841,7 @@ int V4L2VideoDevice::getFormatSingleplane(V4L2DeviceFormat *format) format->size.width = pix->width; format->size.height = pix->height; - format->fourcc = pix->pixelformat; + format->fourcc = V4L2PixelFormat(pix->pixelformat); format->planesCount = 1; format->planes[0].bpl = pix->bytesperline; format->planes[0].size = pix->sizeimage; @@ -869,7 +873,7 @@ int V4L2VideoDevice::setFormatSingleplane(V4L2DeviceFormat *format) */ format->size.width = pix->width; format->size.height = pix->height; - format->fourcc = pix->pixelformat; + format->fourcc = V4L2PixelFormat(pix->pixelformat); format->planesCount = 1; format->planes[0].bpl = pix->bytesperline; format->planes[0].size = pix->sizeimage; @@ -913,7 +917,7 @@ std::vector<V4L2PixelFormat> V4L2VideoDevice::enumPixelformats() if (ret) break; - formats.push_back(pixelformatEnum.pixelformat); + formats.push_back(V4L2PixelFormat(pixelformatEnum.pixelformat)); } if (ret && ret != -EINVAL) { @@ -1545,21 +1549,21 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2Fourcc(PixelFormat pixelFormat, bool mult switch (pixelFormat.fourcc()) { /* RGB formats. */ case DRM_FORMAT_BGR888: - return V4L2_PIX_FMT_RGB24; + return V4L2PixelFormat(V4L2_PIX_FMT_RGB24); case DRM_FORMAT_RGB888: - return V4L2_PIX_FMT_BGR24; + return V4L2PixelFormat(V4L2_PIX_FMT_BGR24); case DRM_FORMAT_BGRA8888: - return V4L2_PIX_FMT_ARGB32; + return V4L2PixelFormat(V4L2_PIX_FMT_ARGB32); /* YUV packed formats. */ case DRM_FORMAT_YUYV: - return V4L2_PIX_FMT_YUYV; + return V4L2PixelFormat(V4L2_PIX_FMT_YUYV); case DRM_FORMAT_YVYU: - return V4L2_PIX_FMT_YVYU; + return V4L2PixelFormat(V4L2_PIX_FMT_YVYU); case DRM_FORMAT_UYVY: - return V4L2_PIX_FMT_UYVY; + return V4L2PixelFormat(V4L2_PIX_FMT_UYVY); case DRM_FORMAT_VYUY: - return V4L2_PIX_FMT_VYUY; + return V4L2PixelFormat(V4L2_PIX_FMT_VYUY); /* * YUY planar formats. @@ -1568,17 +1572,17 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2Fourcc(PixelFormat pixelFormat, bool mult * also take into account the formats supported by the device. */ case DRM_FORMAT_NV16: - return V4L2_PIX_FMT_NV16; + return V4L2PixelFormat(V4L2_PIX_FMT_NV16); case DRM_FORMAT_NV61: - return V4L2_PIX_FMT_NV61; + return V4L2PixelFormat(V4L2_PIX_FMT_NV61); case DRM_FORMAT_NV12: - return V4L2_PIX_FMT_NV12; + return V4L2PixelFormat(V4L2_PIX_FMT_NV12); case DRM_FORMAT_NV21: - return V4L2_PIX_FMT_NV21; + return V4L2PixelFormat(V4L2_PIX_FMT_NV21); /* Compressed formats. */ case DRM_FORMAT_MJPEG: - return V4L2_PIX_FMT_MJPEG; + return V4L2PixelFormat(V4L2_PIX_FMT_MJPEG); } /* @@ -1587,7 +1591,7 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2Fourcc(PixelFormat pixelFormat, bool mult */ libcamera::_log(__FILE__, __LINE__, _LOG_CATEGORY(V4L2)(), LogError).stream() << "Unsupported V4L2 pixel format " << pixelFormat.toString(); - return 0; + return {}; } /** diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.cpp b/test/v4l2_videodevice/v4l2_videodevice_test.cpp index 577da4cb601c..93b9e72da5b4 100644 --- a/test/v4l2_videodevice/v4l2_videodevice_test.cpp +++ b/test/v4l2_videodevice/v4l2_videodevice_test.cpp @@ -69,7 +69,7 @@ int V4L2VideoDeviceTest::init() if (debayer_->open()) return TestFail; - format.fourcc = V4L2_PIX_FMT_SBGGR8; + format.fourcc = V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8); V4L2SubdeviceFormat subformat = {}; subformat.mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8;
To achieve the goal of preventing unwanted conversion between a DRM and a V4L2 FourCC, make the V4L2PixelFormat constructor that takes an integer value explicit. All users of V4L2 pixel formats flagged by the compiler are fixed. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/libcamera/include/v4l2_videodevice.h | 2 +- src/libcamera/pipeline/ipu3/ipu3.cpp | 14 +++---- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +- src/libcamera/pipeline/vimc.cpp | 2 +- src/libcamera/v4l2_videodevice.cpp | 42 ++++++++++--------- .../v4l2_videodevice_test.cpp | 2 +- 6 files changed, 35 insertions(+), 31 deletions(-)