| Message ID | 20220804104550.4600-2-david.plowman@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi David, Thank you for the patch. On Thu, Aug 04, 2022 at 11:45:49AM +0100, David Plowman via libcamera-devel wrote: > These are being used for raw monochrome sensors and so the colour > encoding should be "raw". Ouch. This is something I didn't foresee, but we have a problem here. These can refer to both raw data from monochrome sensors, but also greyscale data processed by an ISP. We currently don't consider as RAW formats, which means that - The CameraConfiguration::validateColorSpaces() will accept other color spaces that ColorSpace::Raw. - The IPU3 and Raspberry Pi pipeline handlers, and the Android HAL, will consider them as processed streams, not raw streams. This patch will change that, quite likely introducing breakages. Now, the interesting question is how to support both raw monochrome data and processed monochrome data. Any idea ? :-) > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > --- > src/libcamera/formats.cpp | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp > index 283ecb3d..7b98fef2 100644 > --- a/src/libcamera/formats.cpp > +++ b/src/libcamera/formats.cpp > @@ -531,7 +531,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > .multi = V4L2PixelFormat(), > }, > .bitsPerPixel = 8, > - .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > .packed = false, > .pixelsPerGroup = 1, > .planes = {{ { 1, 1 }, { 0, 0 }, { 0, 0 } }}, > @@ -544,7 +544,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > .multi = V4L2PixelFormat(), > }, > .bitsPerPixel = 10, > - .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > .packed = false, > .pixelsPerGroup = 1, > .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > @@ -557,7 +557,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > .multi = V4L2PixelFormat(), > }, > .bitsPerPixel = 12, > - .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > .packed = false, > .pixelsPerGroup = 1, > .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > @@ -570,7 +570,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > .multi = V4L2PixelFormat(), > }, > .bitsPerPixel = 10, > - .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > .packed = true, > .pixelsPerGroup = 4, > .planes = {{ { 5, 1 }, { 0, 0 }, { 0, 0 } }},
Hi Laurent Thanks for the feedback. On Thu, 4 Aug 2022 at 19:25, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi David, > > Thank you for the patch. > > On Thu, Aug 04, 2022 at 11:45:49AM +0100, David Plowman via libcamera-devel wrote: > > These are being used for raw monochrome sensors and so the colour > > encoding should be "raw". > > Ouch. This is something I didn't foresee, but we have a problem here. > These can refer to both raw data from monochrome sensors, but also > greyscale data processed by an ISP. We currently don't consider as RAW > formats, which means that > > - The CameraConfiguration::validateColorSpaces() will accept other color > spaces that ColorSpace::Raw. > - The IPU3 and Raspberry Pi pipeline handlers, and the Android HAL, will > consider them as processed streams, not raw streams. > > This patch will change that, quite likely introducing breakages. > > Now, the interesting question is how to support both raw monochrome data > and processed monochrome data. Any idea ? :-) Hmm. That's all a bit awkward. Am I right in thinking that this comes fundamentally from the V4L2 drivers which tell us a format (e.g. "10-bit greyscale") but not whether it's raw or processed? Which presumably makes it a bit awkward to fix "properly". Alternatively, I suppose we could add (slightly ugly) workarounds in the Pi pipeline handler, along the lines of "oh, it says R10, I know that's really a raw format", because that's true for us. I don't really think that I'm seeing anything better than this at the moment... Thanks! David > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > > --- > > src/libcamera/formats.cpp | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp > > index 283ecb3d..7b98fef2 100644 > > --- a/src/libcamera/formats.cpp > > +++ b/src/libcamera/formats.cpp > > @@ -531,7 +531,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > .multi = V4L2PixelFormat(), > > }, > > .bitsPerPixel = 8, > > - .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > .packed = false, > > .pixelsPerGroup = 1, > > .planes = {{ { 1, 1 }, { 0, 0 }, { 0, 0 } }}, > > @@ -544,7 +544,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > .multi = V4L2PixelFormat(), > > }, > > .bitsPerPixel = 10, > > - .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > .packed = false, > > .pixelsPerGroup = 1, > > .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > @@ -557,7 +557,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > .multi = V4L2PixelFormat(), > > }, > > .bitsPerPixel = 12, > > - .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > .packed = false, > > .pixelsPerGroup = 1, > > .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > @@ -570,7 +570,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > .multi = V4L2PixelFormat(), > > }, > > .bitsPerPixel = 10, > > - .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > .packed = true, > > .pixelsPerGroup = 4, > > .planes = {{ { 5, 1 }, { 0, 0 }, { 0, 0 } }}, > > -- > Regards, > > Laurent Pinchart
Hi again On Mon, 8 Aug 2022 at 12:18, David Plowman <david.plowman@raspberrypi.com> wrote: > > Hi Laurent > > Thanks for the feedback. > > On Thu, 4 Aug 2022 at 19:25, Laurent Pinchart > <laurent.pinchart@ideasonboard.com> wrote: > > > > Hi David, > > > > Thank you for the patch. > > > > On Thu, Aug 04, 2022 at 11:45:49AM +0100, David Plowman via libcamera-devel wrote: > > > These are being used for raw monochrome sensors and so the colour > > > encoding should be "raw". > > > > Ouch. This is something I didn't foresee, but we have a problem here. > > These can refer to both raw data from monochrome sensors, but also > > greyscale data processed by an ISP. We currently don't consider as RAW > > formats, which means that > > > > - The CameraConfiguration::validateColorSpaces() will accept other color > > spaces that ColorSpace::Raw. > > - The IPU3 and Raspberry Pi pipeline handlers, and the Android HAL, will > > consider them as processed streams, not raw streams. > > > > This patch will change that, quite likely introducing breakages. > > > > Now, the interesting question is how to support both raw monochrome data > > and processed monochrome data. Any idea ? :-) > > Hmm. That's all a bit awkward. Am I right in thinking that this comes > fundamentally from the V4L2 drivers which tell us a format (e.g. > "10-bit greyscale") but not whether it's raw or processed? Which > presumably makes it a bit awkward to fix "properly". > > Alternatively, I suppose we could add (slightly ugly) workarounds in > the Pi pipeline handler, along the lines of "oh, it says R10, I know > that's really a raw format", because that's true for us. I don't > really think that I'm seeing anything better than this at the > moment... Here's another thought... The problem is that we have more than one possible colour encoding for the "R" formats. If we aren't going to change V4L2 to have raw and non-raw "R" formats anytime soon then maybe we should let the PixelFormatInfo record all the possible colour encodings. Now, I'm guessing that turning the PixelFormatInfo.colourEncoding field into a std::vector<enum ColourEncoding> is probably going to be a bit annoying to existing code. We could decide to do it, though another option might be to add an "alternate" colour encoding field instead. This too could just be a std::vector<enum ColourEncoding> that contains all the other possibilities. The benefit is that it doesn't upset existing code which wouldn't check the new field and nothing would change. The table in formats.h could omit the field altogether (leaving an empty vector) except where it is needed. Thoughts? David > > Thanks! > > David > > > > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > > > --- > > > src/libcamera/formats.cpp | 8 ++++---- > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp > > > index 283ecb3d..7b98fef2 100644 > > > --- a/src/libcamera/formats.cpp > > > +++ b/src/libcamera/formats.cpp > > > @@ -531,7 +531,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > > .multi = V4L2PixelFormat(), > > > }, > > > .bitsPerPixel = 8, > > > - .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > .packed = false, > > > .pixelsPerGroup = 1, > > > .planes = {{ { 1, 1 }, { 0, 0 }, { 0, 0 } }}, > > > @@ -544,7 +544,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > > .multi = V4L2PixelFormat(), > > > }, > > > .bitsPerPixel = 10, > > > - .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > .packed = false, > > > .pixelsPerGroup = 1, > > > .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > @@ -557,7 +557,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > > .multi = V4L2PixelFormat(), > > > }, > > > .bitsPerPixel = 12, > > > - .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > .packed = false, > > > .pixelsPerGroup = 1, > > > .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > @@ -570,7 +570,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > > .multi = V4L2PixelFormat(), > > > }, > > > .bitsPerPixel = 10, > > > - .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > .packed = true, > > > .pixelsPerGroup = 4, > > > .planes = {{ { 5, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > -- > > Regards, > > > > Laurent Pinchart
Hi David, On Wed, Aug 10, 2022 at 12:14:04PM +0100, David Plowman wrote: > On Mon, 8 Aug 2022 at 12:18, David Plowman wrote: > > On Thu, 4 Aug 2022 at 19:25, Laurent Pinchart wrote: > > > On Thu, Aug 04, 2022 at 11:45:49AM +0100, David Plowman via libcamera-devel wrote: > > > > These are being used for raw monochrome sensors and so the colour > > > > encoding should be "raw". > > > > > > Ouch. This is something I didn't foresee, but we have a problem here. > > > These can refer to both raw data from monochrome sensors, but also > > > greyscale data processed by an ISP. We currently don't consider as RAW > > > formats, which means that > > > > > > - The CameraConfiguration::validateColorSpaces() will accept other color > > > spaces that ColorSpace::Raw. > > > - The IPU3 and Raspberry Pi pipeline handlers, and the Android HAL, will > > > consider them as processed streams, not raw streams. > > > > > > This patch will change that, quite likely introducing breakages. > > > > > > Now, the interesting question is how to support both raw monochrome data > > > and processed monochrome data. Any idea ? :-) > > > > Hmm. That's all a bit awkward. Am I right in thinking that this comes > > fundamentally from the V4L2 drivers which tell us a format (e.g. > > "10-bit greyscale") but not whether it's raw or processed? Which > > presumably makes it a bit awkward to fix "properly". In some cases that can be problematic, such as when an ISP receives greyscale raw data and has the ability to output greyscale processed data, or bypass all processing to capture the raw data. We wouldn't be able to configure this through the format on the ISP source pad only. In that case it's probably best to use a different means to configure the ISP bypass (V4L2 control, MC link, ISP parameter buffer, ...). In all other cases, the pipeline handler should be able to figure things out. > > Alternatively, I suppose we could add (slightly ugly) workarounds in > > the Pi pipeline handler, along the lines of "oh, it says R10, I know > > that's really a raw format", because that's true for us. I don't > > really think that I'm seeing anything better than this at the > > moment... > > Here's another thought... > > The problem is that we have more than one possible colour encoding for > the "R" formats. If we aren't going to change V4L2 to have raw and > non-raw "R" formats anytime soon then maybe we should let the > PixelFormatInfo record all the possible colour encodings. The libcamera PixelFormat class shares with V4L2 the inability to discriminate between raw and processed formats, but I don't think V4L2 limits us as such. It is hidden within the pipeline handler and not exposed to applications, so we could change the PixelFormat class without requiring a similar change in V4L2 pixel formats. > Now, I'm guessing that turning the PixelFormatInfo.colourEncoding > field into a std::vector<enum ColourEncoding> is probably going to be > a bit annoying to existing code. We could decide to do it, though > another option might be to add an "alternate" colour encoding field > instead. > > This too could just be a std::vector<enum ColourEncoding> that > contains all the other possibilities. The benefit is that it doesn't > upset existing code which wouldn't check the new field and nothing > would change. The table in formats.h could omit the field altogether > (leaving an empty vector) except where it is needed. > > Thoughts? I've merged your patch that works around this issue for the Raspberry Pi pipeline handler, so from that point of view the problem is solved. I'd still like to share my thoughts though. I'm not sure the lack of discrimination ability in the pixel format is actually an issue. The pixel format is conceptually the same in both case, it's 8-bit or 10-bit or 12-bit greyscale data in memory. I like the idea that the pixel format describes the memory format, and not what processing has been applied. I would like to keep this. What we're missing, in my opinion, is a way to determine where in the pipeline to capture a frame based on other information than just the pixel format. I think this is something we could get from streams, by making them take a more important role in the camera configuration. Applications should be able to decide which streams to capture in a more explicit way. I'll resume at some poit work I started a while ago to rework the configuration API, and I will likely go in this direction. This of course leaves open the question of what the colourEncoding member of the PixelFormatInfo class should store. Your idea of using a vector isn't a bad one, but I'd like to revisit that on top of the configuration API rework, as usage of the colourEncoding field will possibly be quite different then. > > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > > > > --- > > > > src/libcamera/formats.cpp | 8 ++++---- > > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp > > > > index 283ecb3d..7b98fef2 100644 > > > > --- a/src/libcamera/formats.cpp > > > > +++ b/src/libcamera/formats.cpp > > > > @@ -531,7 +531,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > > > .multi = V4L2PixelFormat(), > > > > }, > > > > .bitsPerPixel = 8, > > > > - .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > > .packed = false, > > > > .pixelsPerGroup = 1, > > > > .planes = {{ { 1, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > @@ -544,7 +544,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > > > .multi = V4L2PixelFormat(), > > > > }, > > > > .bitsPerPixel = 10, > > > > - .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > > .packed = false, > > > > .pixelsPerGroup = 1, > > > > .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > @@ -557,7 +557,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > > > .multi = V4L2PixelFormat(), > > > > }, > > > > .bitsPerPixel = 12, > > > > - .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > > .packed = false, > > > > .pixelsPerGroup = 1, > > > > .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > @@ -570,7 +570,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > > > .multi = V4L2PixelFormat(), > > > > }, > > > > .bitsPerPixel = 10, > > > > - .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > > .packed = true, > > > > .pixelsPerGroup = 4, > > > > .planes = {{ { 5, 1 }, { 0, 0 }, { 0, 0 } }},
Hi Laurent Thanks for merging our "workaround" patch. That helps us to keep going in the meantime! On the wider subject... On Tue, 4 Oct 2022 at 21:27, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi David, > > On Wed, Aug 10, 2022 at 12:14:04PM +0100, David Plowman wrote: > > On Mon, 8 Aug 2022 at 12:18, David Plowman wrote: > > > On Thu, 4 Aug 2022 at 19:25, Laurent Pinchart wrote: > > > > On Thu, Aug 04, 2022 at 11:45:49AM +0100, David Plowman via libcamera-devel wrote: > > > > > These are being used for raw monochrome sensors and so the colour > > > > > encoding should be "raw". > > > > > > > > Ouch. This is something I didn't foresee, but we have a problem here. > > > > These can refer to both raw data from monochrome sensors, but also > > > > greyscale data processed by an ISP. We currently don't consider as RAW > > > > formats, which means that > > > > > > > > - The CameraConfiguration::validateColorSpaces() will accept other color > > > > spaces that ColorSpace::Raw. > > > > - The IPU3 and Raspberry Pi pipeline handlers, and the Android HAL, will > > > > consider them as processed streams, not raw streams. > > > > > > > > This patch will change that, quite likely introducing breakages. > > > > > > > > Now, the interesting question is how to support both raw monochrome data > > > > and processed monochrome data. Any idea ? :-) > > > > > > Hmm. That's all a bit awkward. Am I right in thinking that this comes > > > fundamentally from the V4L2 drivers which tell us a format (e.g. > > > "10-bit greyscale") but not whether it's raw or processed? Which > > > presumably makes it a bit awkward to fix "properly". > > In some cases that can be problematic, such as when an ISP receives > greyscale raw data and has the ability to output greyscale processed > data, or bypass all processing to capture the raw data. We wouldn't be > able to configure this through the format on the ISP source pad only. In > that case it's probably best to use a different means to configure the > ISP bypass (V4L2 control, MC link, ISP parameter buffer, ...). In all > other cases, the pipeline handler should be able to figure things out. > > > > Alternatively, I suppose we could add (slightly ugly) workarounds in > > > the Pi pipeline handler, along the lines of "oh, it says R10, I know > > > that's really a raw format", because that's true for us. I don't > > > really think that I'm seeing anything better than this at the > > > moment... > > > > Here's another thought... > > > > The problem is that we have more than one possible colour encoding for > > the "R" formats. If we aren't going to change V4L2 to have raw and > > non-raw "R" formats anytime soon then maybe we should let the > > PixelFormatInfo record all the possible colour encodings. > > The libcamera PixelFormat class shares with V4L2 the inability to > discriminate between raw and processed formats, but I don't think V4L2 > limits us as such. It is hidden within the pipeline handler and not > exposed to applications, so we could change the PixelFormat class > without requiring a similar change in V4L2 pixel formats. > > > Now, I'm guessing that turning the PixelFormatInfo.colourEncoding > > field into a std::vector<enum ColourEncoding> is probably going to be > > a bit annoying to existing code. We could decide to do it, though > > another option might be to add an "alternate" colour encoding field > > instead. > > > > This too could just be a std::vector<enum ColourEncoding> that > > contains all the other possibilities. The benefit is that it doesn't > > upset existing code which wouldn't check the new field and nothing > > would change. The table in formats.h could omit the field altogether > > (leaving an empty vector) except where it is needed. > > > > Thoughts? > > I've merged your patch that works around this issue for the Raspberry Pi > pipeline handler, so from that point of view the problem is solved. I'd > still like to share my thoughts though. > > I'm not sure the lack of discrimination ability in the pixel format is > actually an issue. The pixel format is conceptually the same in both > case, it's 8-bit or 10-bit or 12-bit greyscale data in memory. I like > the idea that the pixel format describes the memory format, and not what > processing has been applied. I would like to keep this. I agree with this. I guess the question is whether the PixelFormat class is describing just the memory format, or something more. In the former case I think things like "colour encoding" simply shouldn't be there. In the latter case... well, there's a long history of folks mixing memory formats, colour encodings and colour spaces together in slightly implicit ways, and I'm not sure it's been doing us much good, but it's a hard thing to fix!! > > What we're missing, in my opinion, is a way to determine where in the > pipeline to capture a frame based on other information than just the > pixel format. I think this is something we could get from streams, by > making them take a more important role in the camera configuration. > Applications should be able to decide which streams to capture in a more > explicit way. I'll resume at some poit work I started a while ago to > rework the configuration API, and I will likely go in this direction. Would be interested to hear more about this! I'm not quite sure what "which streams to capture in a more explicit way" might mean? From my point of view the configuration API seems mostly pretty good, apart from some niggles with the format / colour encoding (as discussed above) or recent colour space changes which are unhelpful to us. libcamera of course already has a lot of active users (a good thing, I hasten to add) so I'm a bit nervous about rocking the boat too much!! Thanks! David > > This of course leaves open the question of what the colourEncoding > member of the PixelFormatInfo class should store. Your idea of using a > vector isn't a bad one, but I'd like to revisit that on top of the > configuration API rework, as usage of the colourEncoding field will > possibly be quite different then. > > > > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > > > > > --- > > > > > src/libcamera/formats.cpp | 8 ++++---- > > > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > > > > > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp > > > > > index 283ecb3d..7b98fef2 100644 > > > > > --- a/src/libcamera/formats.cpp > > > > > +++ b/src/libcamera/formats.cpp > > > > > @@ -531,7 +531,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > > > > .multi = V4L2PixelFormat(), > > > > > }, > > > > > .bitsPerPixel = 8, > > > > > - .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > > > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > > > .packed = false, > > > > > .pixelsPerGroup = 1, > > > > > .planes = {{ { 1, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > > @@ -544,7 +544,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > > > > .multi = V4L2PixelFormat(), > > > > > }, > > > > > .bitsPerPixel = 10, > > > > > - .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > > > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > > > .packed = false, > > > > > .pixelsPerGroup = 1, > > > > > .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > > @@ -557,7 +557,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > > > > .multi = V4L2PixelFormat(), > > > > > }, > > > > > .bitsPerPixel = 12, > > > > > - .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > > > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > > > .packed = false, > > > > > .pixelsPerGroup = 1, > > > > > .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > > @@ -570,7 +570,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > > > > .multi = V4L2PixelFormat(), > > > > > }, > > > > > .bitsPerPixel = 10, > > > > > - .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > > > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > > > .packed = true, > > > > > .pixelsPerGroup = 4, > > > > > .planes = {{ { 5, 1 }, { 0, 0 }, { 0, 0 } }}, > > -- > Regards, > > Laurent Pinchart
diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp index 283ecb3d..7b98fef2 100644 --- a/src/libcamera/formats.cpp +++ b/src/libcamera/formats.cpp @@ -531,7 +531,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ .multi = V4L2PixelFormat(), }, .bitsPerPixel = 8, - .colourEncoding = PixelFormatInfo::ColourEncodingYUV, + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, .packed = false, .pixelsPerGroup = 1, .planes = {{ { 1, 1 }, { 0, 0 }, { 0, 0 } }}, @@ -544,7 +544,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ .multi = V4L2PixelFormat(), }, .bitsPerPixel = 10, - .colourEncoding = PixelFormatInfo::ColourEncodingYUV, + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, .packed = false, .pixelsPerGroup = 1, .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, @@ -557,7 +557,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ .multi = V4L2PixelFormat(), }, .bitsPerPixel = 12, - .colourEncoding = PixelFormatInfo::ColourEncodingYUV, + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, .packed = false, .pixelsPerGroup = 1, .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, @@ -570,7 +570,7 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ .multi = V4L2PixelFormat(), }, .bitsPerPixel = 10, - .colourEncoding = PixelFormatInfo::ColourEncodingYUV, + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, .packed = true, .pixelsPerGroup = 4, .planes = {{ { 5, 1 }, { 0, 0 }, { 0, 0 } }},
These are being used for raw monochrome sensors and so the colour encoding should be "raw". Signed-off-by: David Plowman <david.plowman@raspberrypi.com> --- src/libcamera/formats.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)