<div dir="ltr"><div dir="ltr">Hi Laurent,<div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 26 Oct 2021 at 23:46, Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Naush,<br>
<br>
On Tue, Oct 26, 2021 at 11:48:24AM +0100, Naushir Patuck wrote:<br>
> On Tue, 26 Oct 2021 at 08:48, Naushir Patuck wrote:<br>
> > On Mon, 25 Oct 2021 at 14:58, Laurent Pinchart wrote:<br>
> >> On Fri, Oct 22, 2021 at 03:39:02PM +0100, Naushir Patuck wrote:<br>
> >> > Add a BayerFormat::toPixelFormat() member function to convert a BayerFormat to a<br>
> >> > PixelFormat type. This conversion uses the existing bayerToV4l2 conversion<br>
> >> > table.<br>
> >> ><br>
> >> > Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> >> > Reviewed-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
> >> > ---<br>
> >> >  include/libcamera/internal/bayer_format.h |  3 +++<br>
> >> >  src/libcamera/bayer_format.cpp            | 13 +++++++++++++<br>
> >> >  2 files changed, 16 insertions(+)<br>
> >> ><br>
> >> > diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h<br>
> >> > index 723382d4168d..247a60cf0e36 100644<br>
> >> > --- a/include/libcamera/internal/bayer_format.h<br>
> >> > +++ b/include/libcamera/internal/bayer_format.h<br>
> >> > @@ -10,6 +10,8 @@<br>
> >> >  #include <stdint.h><br>
> >> >  #include <string><br>
> >> ><br>
> >> > +#include <libcamera/pixel_format.h><br>
> >> > +<br>
> >> >  #include "libcamera/internal/v4l2_pixelformat.h"<br>
> >> ><br>
> >> >  namespace libcamera {<br>
> >> > @@ -50,6 +52,7 @@ public:<br>
> >> ><br>
> >> >       V4L2PixelFormat toV4L2PixelFormat() const;<br>
> >> >       static BayerFormat fromV4L2PixelFormat(V4L2PixelFormat v4l2Format);<br>
> >> > +     PixelFormat toPixelFormat() const;<br>
> >> >       BayerFormat transform(Transform t) const;<br>
> >> ><br>
> >> >       Order order;<br>
> >> > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp<br>
> >> > index 11355f144f66..c662ba36604c 100644<br>
> >> > --- a/src/libcamera/bayer_format.cpp<br>
> >> > +++ b/src/libcamera/bayer_format.cpp<br>
> >> > @@ -269,6 +269,19 @@ BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format)<br>
> >> >       return BayerFormat();<br>
> >> >  }<br>
> >> ><br>
> >> > +/**<br>
> >> > + * \brief Convert a BayerFormat into the corresponding PixelFormat<br>
> >> > + * \return The PixelFormat corresponding to this BayerFormat<br>
> >> > + */<br>
> >> > +PixelFormat BayerFormat::toPixelFormat() const<br>
> >> > +{<br>
> >> > +     const auto it = bayerToV4l2.find(*this);<br>
> >> > +     if (it != bayerToV4l2.end())<br>
> >> > +             return PixelFormat(it->second);<br>
> >><br>
> >> Should we store the PixelFormat in the bayerToV4l2 map to avoid a double<br>
> >> looking (one here, one in the PixelFormat constructor) ? The map should<br>
> >> be renamed to bayerToFormat, and be stored as either<br>
> >><br>
> >> std::map<BayerFormat, std::pair<PixelFormat, V4L2PixelFormat>,<br>
> >> BayerFormatComparator><br>
> >><br>
> >> or<br>
> >><br>
> >> struct Formats {<br>
> >>         PixelFormat pixelFormat;<br>
> >>         V4L2PixelFormat v4l2Format;<br>
> >> };<br>
> >><br>
> >> std::map<BayerFormat, Formats, BayerFormatComparator><br>
> ><br>
> > This seems reasonable.  I will update the table with a std::pair for 2-way<br>
> > conversion.<br>
> <br>
> Unfortunately this has hit an unexpected complication. DRM formats, and subsequently<br>
> PixelFormat types do not define Mono Bayer formats. The corresponding mbus codes<br>
> are MEDIA_BUS_FMT_Y8_1X8 and MEDIA_BUS_FMT_Y10_1X10.<br>
<br>
DRM and V4L2 define greyscale formats (only 8-bit for DRM with<br>
DRM_FORMAT_R8, 10- and 12-bit formats could be easily added, and<br>
V4L2_PIX_FMT_GREY, V4L2_PIX_FMT_Y10 and V4L2_PIX_FMT_Y12 for V4L2).<br>
They're not Bayer formats indeed, because Bayer filters are only for<br>
colour formats.<br></blockquote><div><br></div>I did sport the DRM_FORMAT_R8, but the comment of "/* 8 bpp Red */" make me<br>think it was not entirely correct to use that for mono sensors.  However, I am happy to do</div><div class="gmail_quote">so now that you have suggested it :-)</div><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
On a side note, I think it was a historical mistake in V4L2 to add Bayer<br>
formats, we should have defined RAW8/10/12/14/16 formats with the CFA<br>
pattern being carried out of band.<br>
<br>
> Would it be acceptable if the conversion regards these mono BayerFormat types as invalid<br>
> PixelFormat types with a todo to add appropriate support in the future? Mono sensor support<br>
> is still usable as we go from mbus code -> V4L2 4CC correctly, but the application can never<br>
> request a mono output - which it can't do today anyway!<br>
<br>
For 8-bit I think you can simply use formats::R8. For 10- and 12-bit,<br>
would <a href="https://git.libcamera.org/libcamera/pinchartl/libcamera.git/log/?h=simple/imx8" rel="noreferrer" target="_blank">https://git.libcamera.org/libcamera/pinchartl/libcamera.git/log/?h=simple/imx8</a><br>
help ?<br></blockquote><div><br></div><div>Yes it would! I am happy to cherry-pick commits 26f0ce03 and 940d4df4 that add R10 and R12 formats</div><div>to this series if you like?</div><div><br></div><div>Thanks,</div><div>Naush</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>