| Message ID | 20220815141637.4159-2-david.plowman@raspberrypi.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi David, Thank you for your work. On Mon, 15 Aug 2022 at 15:16, David Plowman via libcamera-devel < libcamera-devel@lists.libcamera.org> wrote: > The "R" pixel formats (R8, R10, R10_CSI2P etc.) record the associated > colour space as being YUV rather than RAW, meaning that the code was > not detecting them as being raw formats. > > In the case of Raspberry Pi, we deal only with raw formats, so the > revised test must work correctly for both these and the standard Bayer > formats. > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > Reviewed-by: Naushir Patuck <naush@raspberrypi.com> > --- > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 14 ++------------ > 1 file changed, 2 insertions(+), 12 deletions(-) > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index e895584d..fae8e45e 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -105,18 +105,8 @@ V4L2DeviceFormat toV4L2DeviceFormat(const > V4L2VideoDevice *dev, > > bool isRaw(const PixelFormat &pixFmt) > { > - /* > - * The isRaw test might be redundant right now the pipeline > handler only > - * supports RAW sensors. Leave it in for now, just as a sanity > check. > - */ > - if (!pixFmt.isValid()) > - return false; > - > - const PixelFormatInfo &info = PixelFormatInfo::info(pixFmt); > - if (!info.isValid()) > - return false; > - > - return info.colourEncoding == PixelFormatInfo::ColourEncodingRAW; > + /* This test works for both Bayer and raw mono formats. */ > + return BayerFormat::fromPixelFormat(pixFmt).isValid(); > } > > double scoreFormat(double desired, double actual) > -- > 2.30.2 > >
Hi Just wanted to remind everyone about this patch, could we progress it so that it can be merged? Thanks David On Tue, 30 Aug 2022 at 12:44, Naushir Patuck <naush@raspberrypi.com> wrote: > > Hi David, > > Thank you for your work. > > On Mon, 15 Aug 2022 at 15:16, David Plowman via libcamera-devel <libcamera-devel@lists.libcamera.org> wrote: >> >> The "R" pixel formats (R8, R10, R10_CSI2P etc.) record the associated >> colour space as being YUV rather than RAW, meaning that the code was >> not detecting them as being raw formats. >> >> In the case of Raspberry Pi, we deal only with raw formats, so the >> revised test must work correctly for both these and the standard Bayer >> formats. >> >> Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > > > Reviewed-by: Naushir Patuck <naush@raspberrypi.com> > >> >> --- >> src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 14 ++------------ >> 1 file changed, 2 insertions(+), 12 deletions(-) >> >> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp >> index e895584d..fae8e45e 100644 >> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp >> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp >> @@ -105,18 +105,8 @@ V4L2DeviceFormat toV4L2DeviceFormat(const V4L2VideoDevice *dev, >> >> bool isRaw(const PixelFormat &pixFmt) >> { >> - /* >> - * The isRaw test might be redundant right now the pipeline handler only >> - * supports RAW sensors. Leave it in for now, just as a sanity check. >> - */ >> - if (!pixFmt.isValid()) >> - return false; >> - >> - const PixelFormatInfo &info = PixelFormatInfo::info(pixFmt); >> - if (!info.isValid()) >> - return false; >> - >> - return info.colourEncoding == PixelFormatInfo::ColourEncodingRAW; >> + /* This test works for both Bayer and raw mono formats. */ >> + return BayerFormat::fromPixelFormat(pixFmt).isValid(); >> } >> >> double scoreFormat(double desired, double actual) >> -- >> 2.30.2 >>
Hi David, Thank you for the patch. On Mon, Aug 15, 2022 at 03:16:37PM +0100, David Plowman via libcamera-devel wrote: > The "R" pixel formats (R8, R10, R10_CSI2P etc.) record the associated > colour space as being YUV rather than RAW, meaning that the code was > not detecting them as being raw formats. > > In the case of Raspberry Pi, we deal only with raw formats, so the > revised test must work correctly for both these and the standard Bayer > formats. That seems fine to me. We'll still need a more generic solution for platforms that can capture processed greyscale images, but that's for later. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > --- > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 14 ++------------ > 1 file changed, 2 insertions(+), 12 deletions(-) > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index e895584d..fae8e45e 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -105,18 +105,8 @@ V4L2DeviceFormat toV4L2DeviceFormat(const V4L2VideoDevice *dev, > > bool isRaw(const PixelFormat &pixFmt) > { > - /* > - * The isRaw test might be redundant right now the pipeline handler only > - * supports RAW sensors. Leave it in for now, just as a sanity check. > - */ > - if (!pixFmt.isValid()) > - return false; > - > - const PixelFormatInfo &info = PixelFormatInfo::info(pixFmt); > - if (!info.isValid()) > - return false; > - > - return info.colourEncoding == PixelFormatInfo::ColourEncodingRAW; > + /* This test works for both Bayer and raw mono formats. */ > + return BayerFormat::fromPixelFormat(pixFmt).isValid(); > } > > double scoreFormat(double desired, double actual)
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index e895584d..fae8e45e 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -105,18 +105,8 @@ V4L2DeviceFormat toV4L2DeviceFormat(const V4L2VideoDevice *dev, bool isRaw(const PixelFormat &pixFmt) { - /* - * The isRaw test might be redundant right now the pipeline handler only - * supports RAW sensors. Leave it in for now, just as a sanity check. - */ - if (!pixFmt.isValid()) - return false; - - const PixelFormatInfo &info = PixelFormatInfo::info(pixFmt); - if (!info.isValid()) - return false; - - return info.colourEncoding == PixelFormatInfo::ColourEncodingRAW; + /* This test works for both Bayer and raw mono formats. */ + return BayerFormat::fromPixelFormat(pixFmt).isValid(); } double scoreFormat(double desired, double actual)
The "R" pixel formats (R8, R10, R10_CSI2P etc.) record the associated colour space as being YUV rather than RAW, meaning that the code was not detecting them as being raw formats. In the case of Raspberry Pi, we deal only with raw formats, so the revised test must work correctly for both these and the standard Bayer formats. Signed-off-by: David Plowman <david.plowman@raspberrypi.com> --- src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-)