[libcamera-devel,v5,0/7] Colour spaces
mbox series

Message ID 20211115161205.24335-1-david.plowman@raspberrypi.com
Headers show
Series
  • Colour spaces
Related show

Message

David Plowman Nov. 15, 2021, 4:11 p.m. UTC 
Hi

This is a re-post of this patch set with an increased distribution
list.

For the moment I would say the "code details" are not so important as
those can be sorted out later, it's the basic ideas here that we need
to get right first.

To try and summarise those:

1. There's a ColorSpace class consisting of 4 enums, for the
primaries, the Y'CbCr encoding, the transfer function and the
quantisation (range).

2. Every colour space has just one representation in terms of these
four enums.

3. I've insisted that pipeline handlers must say what colour space
they want. I'm nervous about letting people "not say", and then you
get a slightly wrong colour space and never notice!

4. I'm worried about drivers (in future, perhaps) wanting to use
colour spaces that aren't listed in the ColorSpace class. Do I worry
too much?

5. Point 4 led me to having two ColorSpace fields in the configuration
object, one for the "colour space you want", and one for "what the
driver will give you".

6. Again, because of point 4, I've allowed the "what the driver will
give you" colour space to be "undefined". But you _may not_ pass
"undefined" as the "colour space you want".

7. Whenever colour spaces don't match up I think there need to be
warnings that are hard to ignore, because it's so easy to get colour
spaces wrong (and never notice). At the same time, outright failure
seems unhelpful because there really could be times when you can't
have the colour space you really want.

8. I've suggested pipeline handlers should pick reasonable defaults in
their generateConfiguration() methods. Perhaps JPEG for stills, and
Rec.709 for video.

9. I do wonder about a future where people might want to specify
custom colour spaces (obviously there'd have to be driver support). I
imagine that happening through a "Custom" value for some of the enums,
and then extra fields which are then "active". For example, you might
specify your 3 colour primaries.

I think that's it. If I can clarify anything further between now and
Wednesday, please don't hesitate to ask.

Best regards
David


David Plowman (7):
  libcamera: Add ColorSpace class
  libcamera: Add ColorSpace fields to StreamConfiguration
  libcamera: Convert between ColorSpace class and V4L2 formats
  libcamera: Support passing ColorSpaces to V4L2 video devices
  libcamera: Support passing ColorSpaces to V4L2 subdevices
  libcamera: Add validateColorSpaces to CameraConfiguration class
  libcamera: pipeline: raspberrypi: Support color spaces

 include/libcamera/camera.h                    |   2 +
 include/libcamera/color_space.h               |  88 ++++++
 include/libcamera/internal/v4l2_device.h      |   7 +
 include/libcamera/internal/v4l2_subdevice.h   |   2 +
 include/libcamera/internal/v4l2_videodevice.h |   2 +
 include/libcamera/meson.build                 |   1 +
 include/libcamera/stream.h                    |   4 +
 src/libcamera/camera.cpp                      |  51 ++++
 src/libcamera/camera_sensor.cpp               |   1 +
 src/libcamera/color_space.cpp                 | 257 ++++++++++++++++++
 src/libcamera/meson.build                     |   1 +
 .../pipeline/raspberrypi/raspberrypi.cpp      |  42 +++
 src/libcamera/stream.cpp                      |  25 ++
 src/libcamera/v4l2_device.cpp                 | 171 ++++++++++++
 src/libcamera/v4l2_subdevice.cpp              |  42 ++-
 src/libcamera/v4l2_videodevice.cpp            |  67 ++++-
 16 files changed, 758 insertions(+), 5 deletions(-)
 create mode 100644 include/libcamera/color_space.h
 create mode 100644 src/libcamera/color_space.cpp