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

17447 diff mbox series

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@ideasonboard.com>
---
 src/cam/kms_sink.cpp | 80 ++++++++++++++++++++++++++++++++++++++++++++
 src/cam/kms_sink.h   |  3 ++
 2 files changed, 83 insertions(+)

On Wed, 28 Sept 2022 at 02:49, Laurent Pinchart via libcamera-devel
<libcamera-devel@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@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@redhat.com>