[{"id":20419,"web_url":"https://patchwork.libcamera.org/comment/20419/","msgid":"","date":"2021-10-25T08:23:09","subject":"Re: [libcamera-devel] [PATCH v3 5/7] libcamera: Support passing\n\tColorSpaces to V4L2 subdevices","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\nThank you for your work.\n\nOn Wed, 20 Oct 2021 at 12:08, David Plowman \nwrote:\n\n> The ColorSpace from the StreamConfiguration is now handled\n> appropriately in the V4L2Subdevice.\n>\n> Signed-off-by: David Plowman \n> ---\n> include/libcamera/internal/v4l2_subdevice.h | 2 ++\n> src/libcamera/camera_sensor.cpp | 1 +\n> src/libcamera/v4l2_subdevice.cpp | 37 ++++++++++++++++++++-\n> 3 files changed, 39 insertions(+), 1 deletion(-)\n>\n> diff --git a/include/libcamera/internal/v4l2_subdevice.h\n> b/include/libcamera/internal/v4l2_subdevice.h\n> index 97b89fb9..f3ab8454 100644\n> --- a/include/libcamera/internal/v4l2_subdevice.h\n> +++ b/include/libcamera/internal/v4l2_subdevice.h\n> @@ -14,6 +14,7 @@\n> #include \n> #include \n>\n> +#include \n> #include \n>\n> #include \"libcamera/internal/formats.h\"\n> @@ -27,6 +28,7 @@ class MediaDevice;\n> struct V4L2SubdeviceFormat {\n> uint32_t mbus_code;\n> Size size;\n> + ColorSpace colorSpace;\n>\n> const std::string toString() const;\n> uint8_t bitsPerPixel() const;\n> diff --git a/src/libcamera/camera_sensor.cpp\n> b/src/libcamera/camera_sensor.cpp\n> index 9fdb8c09..6fcd1c1d 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -613,6 +613,7 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const\n> std::vector &mbu\n> V4L2SubdeviceFormat format{\n> .mbus_code = bestCode,\n> .size = *bestSize,\n> + .colorSpace = ColorSpace::Raw,\n> };\n>\n> return format;\n> diff --git a/src/libcamera/v4l2_subdevice.cpp\n> b/src/libcamera/v4l2_subdevice.cpp\n> index 023e2328..8538e66c 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -168,6 +168,18 @@ const std::map\n> formatInfoMap = {\n> * \\brief The image size in pixels\n> */\n>\n> +/**\n> + * \\var V4L2SubdeviceFormat::colorSpace\n> + * \\brief The color space of the pixels\n> + *\n> + * When setting or trying a format, passing in \"Undefined\" fields in the\n> + * ColorSpace is not recommended because the driver will then make an\n> + * arbitrary choice of its own. Choices made by the driver will be\n> + * passed back in the normal way, though note that \"Undefined\" values can\n> + * be returned if the device has chosen something that the ColorSpace\n> + * cannot represent.\n> + */\n> +\n> /**\n> * \\brief Assemble and return a string describing the format\n> * \\return A string describing the V4L2SubdeviceFormat\n> @@ -400,6 +412,12 @@ int V4L2Subdevice::getFormat(unsigned int pad,\n> V4L2SubdeviceFormat *format,\n> format->size.height = subdevFmt.format.height;\n> format->mbus_code = subdevFmt.format.code;\n>\n> + format->colorSpace = v4l2ToColorSpace(subdevFmt.format);\n> + if (!format->colorSpace.isFullyDefined())\n> + LOG(V4L2, Warning)\n> + << \"Retrieved undefined color space: \"\n> + << format->colorSpace.toString();\n> +\n> return 0;\n> }\n>\n> @@ -418,6 +436,11 @@ int V4L2Subdevice::getFormat(unsigned int pad,\n> V4L2SubdeviceFormat *format,\n> int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat\n> *format,\n> Whence whence)\n> {\n> + if (!format->colorSpace.isFullyDefined())\n> + LOG(V4L2, Error)\n> + << \"Trying to set undefined color space: \"\n> + << format->colorSpace.toString();\n> +\n>\n\nLike the previous patch, should we return with an error code here? Other\nthan that:\n\nReviewed-by: Naushir Patuck \n\n\n> struct v4l2_subdev_format subdevFmt = {};\n> subdevFmt.which = whence == ActiveFormat ?\n> V4L2_SUBDEV_FORMAT_ACTIVE\n> : V4L2_SUBDEV_FORMAT_TRY;\n> @@ -427,7 +450,13 @@ int V4L2Subdevice::setFormat(unsigned int pad,\n> V4L2SubdeviceFormat *format,\n> subdevFmt.format.code = format->mbus_code;\n> subdevFmt.format.field = V4L2_FIELD_NONE;\n>\n> - int ret = ioctl(VIDIOC_SUBDEV_S_FMT, &subdevFmt);\n> + int ret = colorSpaceToV4l2(format->colorSpace, subdevFmt.format);\n> + if (ret < 0)\n> + LOG(V4L2, Warning)\n> + << \"Setting color space unrecognised by V4L2: \"\n> + << format->colorSpace.toString();\n> +\n> + ret = ioctl(VIDIOC_SUBDEV_S_FMT, &subdevFmt);\n> if (ret) {\n> LOG(V4L2, Error)\n> << \"Unable to set format on pad \" << pad\n> @@ -439,6 +468,12 @@ int V4L2Subdevice::setFormat(unsigned int pad,\n> V4L2SubdeviceFormat *format,\n> format->size.height = subdevFmt.format.height;\n> format->mbus_code = subdevFmt.format.code;\n>\n> + format->colorSpace = v4l2ToColorSpace(subdevFmt.format);\n> + if (!format->colorSpace.isFullyDefined())\n> + LOG(V4L2, Warning)\n> + << \"Undefined color space has been set: \"\n> + << format->colorSpace.toString();\n> +\n> return 0;\n> }\n>\n> --\n> 2.20.1\n>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 335A1BDB1C\n\tfor ;\n\tMon, 25 Oct 2021 08:23:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8A96C6486F;\n\tMon, 25 Oct 2021 10:23:27 +0200 (CEST)","from mail-lf1-x131.google.com (mail-lf1-x131.google.com\n\t[IPv6:2a00:1450:4864:20::131])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 73FBD60124\n\tfor ;\n\tMon, 25 Oct 2021 10:23:26 +0200 (CEST)","by mail-lf1-x131.google.com with SMTP id u21so11827080lff.8\n\tfor ;\n\tMon, 25 Oct 2021 01:23:26 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"WdnT41f/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=rWg/MyxJjZXl4ENrWYpDAdO953O+HjWwImTo7+wdEmo=;\n\tb=WdnT41f/7tETkjl4qnUBx9p1XtPuJp6LDkmZqqvU9IyNnn0gJ/hKAVDigWtNEJlWCE\n\tEXGvIP/4RkpOIJHFCt6bQIGy1OQjy9fF7Yc0+RDqSQUmB1n+3k+ryQdDU3gNeT4hDxXH\n\tYuBv3N5Nf3SBYhdtRSqORmj9Z3G/Xm/wEXXvYaFg+4h37bSn/NpKh+alzm9OLya69O8l\n\tlfzfbJBiXIx7tiWfW4m7d9uF+andpQ1p/J0Qf3RnJNTnFyVt+BXp7exlP+LHz4w6yqWU\n\tEFoqQL8YCKPyiJFjh90vOc8vW30n9Pwn4eCoUk6YeNl3esG9m36qB5d0NFXW92Ga6cJi\n\tlsHA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=rWg/MyxJjZXl4ENrWYpDAdO953O+HjWwImTo7+wdEmo=;\n\tb=asmNsh0jKkkU+nHxTsd4acaoS3MYasVpBvdLFWSsEEdphHLW1Ps1t7kelZyA3+IzkW\n\tSXd5+TmV9WQxH5fJbOWq+hsFzPa8KX5Ev9SJN5FLJusGZsYDBNxVxDvLdjVG8D6B/YAc\n\tqFqaAqbRmM5u8CQejkKm4p1lXGtlsrehSzwnvuBT+GR9qbW1XQpH86wQ2H+fRCNRgVH7\n\t2804HGBj80poDeqC8DuzE6knpcS81N4iUsfdnvFwRmC7KooIx6kLY+DjcXRcfCQNC61m\n\tEied/cGpLNZpZwWL5vzSP/RBThAJwg7uhQfHakNT7QxnUx3XNvTTzK9EvXq8M5kBFoVU\n\tzbMA==","X-Gm-Message-State":"AOAM530EsDRX1lRdNvVYWmq+GUQwdMWLk+LeHuayIxn6YGhRSH99wr5t\n\th3DXKFU+5AXPwd8wEFzd8bvFJEsGHlAZzs2fQlJ3gVUYG0+Osg==","X-Google-Smtp-Source":"ABdhPJxXpa4eo4HrV5AQwHYnaXAb29m9NrF8oU5QZFRP+VDh8T+MFTu74NHoXrGfRWQpq8IN1YeAXqAcTFMdDbf0cxY=","X-Received":"by 2002:ac2:5304:: with SMTP id c4mr7195665lfh.687.1635150205780;\n\tMon, 25 Oct 2021 01:23:25 -0700 (PDT)","MIME-Version":"1.0","References":"<20211020110825.12902-1-david.plowman@raspberrypi.com>\n\t<20211020110825.12902-6-david.plowman@raspberrypi.com>","In-Reply-To":"<20211020110825.12902-6-david.plowman@raspberrypi.com>","From":"Naushir Patuck ","Date":"Mon, 25 Oct 2021 09:23:09 +0100","Message-ID":"","To":"David Plowman ","Content-Type":"multipart/alternative; boundary=\"0000000000002b96ec05cf2913a6\"","Subject":"Re: [libcamera-devel] [PATCH v3 5/7] libcamera: Support passing\n\tColorSpaces to V4L2 subdevices","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]