[libcamera-devel] [PATCH] cam: kms_sink: Add color space support

[Eric Curtin]

On Wed, 28 Sept 2022 at 02:49, Laurent Pinchart via libcamera-devel
<libcamera-devel at lists.libcamera.org> wrote:
>
> KMS defines YCbCr encoding and quantization properties for planes. When
> supported by the device, set them to match the color space of the
> stream to render colors accurately.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/cam/kms_sink.cpp | 80 ++++++++++++++++++++++++++++++++++++++++++++
>  src/cam/kms_sink.h   |  3 ++
>  2 files changed, 83 insertions(+)
>
> diff --git a/src/cam/kms_sink.cpp b/src/cam/kms_sink.cpp
> index 17e2fa69bff7..754b061eebf7 100644
> --- a/src/cam/kms_sink.cpp
> +++ b/src/cam/kms_sink.cpp
> @@ -149,6 +149,81 @@ int KMSSink::configure(const libcamera::CameraConfiguration &config)
>         size_ = cfg.size;
>         stride_ = cfg.stride;
>
> +       /* Configure color space. */
> +       colorEncoding_ = std::nullopt;
> +       colorRange_ = std::nullopt;
> +
> +       if (cfg.colorSpace->ycbcrEncoding == libcamera::ColorSpace::YcbcrEncoding::None)
> +               return 0;
> +
> +       /*
> +        * The encoding and range enums are defined in the kernel but not
> +        * exposed in public headers.
> +        */
> +       enum drm_color_encoding {
> +               DRM_COLOR_YCBCR_BT601,
> +               DRM_COLOR_YCBCR_BT709,
> +               DRM_COLOR_YCBCR_BT2020,
> +       };
> +
> +       enum drm_color_range {
> +               DRM_COLOR_YCBCR_LIMITED_RANGE,
> +               DRM_COLOR_YCBCR_FULL_RANGE,
> +       };
> +
> +       const DRM::Property *colorEncoding = plane_->property("COLOR_ENCODING");
> +       const DRM::Property *colorRange = plane_->property("COLOR_RANGE");
> +
> +       if (colorEncoding) {
> +               drm_color_encoding encoding;
> +
> +               switch (cfg.colorSpace->ycbcrEncoding) {
> +               case libcamera::ColorSpace::YcbcrEncoding::Rec601:
> +               default:
> +                       encoding = DRM_COLOR_YCBCR_BT601;
> +                       break;
> +               case libcamera::ColorSpace::YcbcrEncoding::Rec709:
> +                       encoding = DRM_COLOR_YCBCR_BT709;
> +                       break;
> +               case libcamera::ColorSpace::YcbcrEncoding::Rec2020:
> +                       encoding = DRM_COLOR_YCBCR_BT2020;
> +                       break;
> +               }
> +
> +               for (const auto &[id, name] : colorEncoding->enums()) {
> +                       if (id == encoding) {
> +                               colorEncoding_ = encoding;
> +                               break;
> +                       }
> +               }
> +       }
> +
> +       if (colorRange) {
> +               drm_color_range range;
> +
> +               switch (cfg.colorSpace->range) {
> +               case libcamera::ColorSpace::Range::Limited:
> +               default:
> +                       range = DRM_COLOR_YCBCR_LIMITED_RANGE;
> +                       break;
> +               case libcamera::ColorSpace::Range::Full:
> +                       range = DRM_COLOR_YCBCR_FULL_RANGE;
> +                       break;
> +               }
> +
> +               for (const auto &[id, name] : colorRange->enums()) {
> +                       if (id == range) {
> +                               colorRange_ = range;
> +                               break;
> +                       }
> +               }
> +       }
> +
> +       if (!colorEncoding_ || !colorRange_)
> +               std::cerr << "Color space " << cfg.colorSpace->toString()
> +                         << " not supported by the display device."
> +                         << " Colors may be wrong." << std::endl;
> +
>         return 0;
>  }
>
> @@ -415,6 +490,11 @@ bool KMSSink::processRequest(libcamera::Request *camRequest)
>                 drmRequest->addProperty(plane_, "CRTC_W", dst_.width);
>                 drmRequest->addProperty(plane_, "CRTC_H", dst_.height);
>
> +               if (colorEncoding_)
> +                       drmRequest->addProperty(plane_, "COLOR_ENCODING", *colorEncoding_);
> +               if (colorRange_)
> +                       drmRequest->addProperty(plane_, "COLOR_RANGE", *colorRange_);
> +
>                 flags |= DRM::AtomicRequest::FlagAllowModeset;
>         }
>
> diff --git a/src/cam/kms_sink.h b/src/cam/kms_sink.h
> index 76c4e611bf85..e2c618a19035 100644
> --- a/src/cam/kms_sink.h
> +++ b/src/cam/kms_sink.h
> @@ -10,6 +10,7 @@
>  #include <list>
>  #include <memory>
>  #include <mutex>
> +#include <optional>
>  #include <string>
>  #include <utility>
>
> @@ -67,6 +68,8 @@ private:
>         libcamera::PixelFormat format_;
>         libcamera::Size size_;
>         unsigned int stride_;
> +       std::optional<unsigned int> colorEncoding_;
> +       std::optional<unsigned int> colorRange_;
>
>         libcamera::Rectangle src_;
>         libcamera::Rectangle dst_;
> --
> Regards,
>
> Laurent Pinchart
>

LGTM

Reviewed-by: Eric Curtin <ecurtin at redhat.com>