[{"id":20416,"web_url":"https://patchwork.libcamera.org/comment/20416/","msgid":"<CAEmqJPouA1R7aTSrC=Bt_NBNNWqfOhxwT-KcWv76vW91qS2K8A@mail.gmail.com>","date":"2021-10-25T07:45:04","subject":"Re: [libcamera-devel] [PATCH v3 2/7] libcamera: Add ColorSpace\n\tfields to StreamConfiguration","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 patch.\n\nOn Wed, 20 Oct 2021 at 12:08, David Plowman <david.plowman@raspberrypi.com>\nwrote:\n\n> This is so that applications can choose appropriate color spaces which\n> will then be passed down to the V4L2 devices.\n>\n> There are two fields: the \"requestedColorSpace\" which is the one an\n> application wants, and the \"actualColorSpace\" which is what the\n> underlying hardware can deliver, and which is filled in by the\n> validate() method.\n>\n> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> ---\n>  include/libcamera/stream.h |  4 ++++\n>  src/libcamera/stream.cpp   | 25 +++++++++++++++++++++++++\n>  2 files changed, 29 insertions(+)\n>\n> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> index 0c55e716..fe491ff5 100644\n> --- a/include/libcamera/stream.h\n> +++ b/include/libcamera/stream.h\n> @@ -12,6 +12,7 @@\n>  #include <string>\n>  #include <vector>\n>\n> +#include <libcamera/color_space.h>\n>  #include <libcamera/framebuffer.h>\n>  #include <libcamera/geometry.h>\n>  #include <libcamera/pixel_format.h>\n> @@ -47,6 +48,9 @@ struct StreamConfiguration {\n>\n>         unsigned int bufferCount;\n>\n> +       ColorSpace requestedColorSpace;\n> +       ColorSpace actualColorSpace;\n> +\n>\n\nI had a brief look-ahead in this series, but it was not immediately\nobvious to me why we would need requested and actual? Could\nwe make do with just one and update the user requested value\nwith what the HW will actually give us?\n\n\n>         Stream *stream() const { return stream_; }\n>         void setStream(Stream *stream) { stream_ = stream; }\n>         const StreamFormats &formats() const { return formats_; }\n> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> index b421e17e..1ddbbb8c 100644\n> --- a/src/libcamera/stream.cpp\n> +++ b/src/libcamera/stream.cpp\n> @@ -329,6 +329,31 @@ StreamConfiguration::StreamConfiguration(const\n> StreamFormats &formats)\n>   * \\brief Requested number of buffers to allocate for the stream\n>   */\n>\n> +/**\n> + * \\var StreamConfiguration::requestedColorSpace\n> + * \\brief The ColorSpace this stream should have\n> + *\n> + * The generateConfiguration method will generate reasonable default\n> + * values (ColorSpace::Jpeg for stills, ColorSpace::Rec709 for video and\n> + * ColorSpace::Raw for raw streams) but applications are free to overwrite\n> + * this value.\n> + */\n> +\n> +/**\n> + * \\var StreamConfiguration::actualColorSpace\n> + * \\brief The ColorSpace the will be used for this stream\n> + *\n> + * This field is filled in by CameraConfiguration::validate().\n> + * Normally this should match the requestedColorSpace, but it may differ\n> + * if the hardware does not support it.\n> + *\n> + * In general cameras may have different constraints here, for example,\n> + * they may require all output streams to share the same color space.\n> + * Sometimes the fields within this color space may report \"Undefined\"\n> + * values if the hardware drivers are going to use a color space that\n> + * is not recognised by the ColorSpace class.\n> + */\n> +\n>  /**\n>   * \\fn StreamConfiguration::stream()\n>   * \\brief Retrieve the stream associated with the configuration\n> --\n> 2.20.1\n>\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 27A9FBDB1C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 25 Oct 2021 07:45:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7DE0064870;\n\tMon, 25 Oct 2021 09:45:22 +0200 (CEST)","from mail-lf1-x12e.google.com (mail-lf1-x12e.google.com\n\t[IPv6:2a00:1450:4864:20::12e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0FDBD60124\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 25 Oct 2021 09:45:21 +0200 (CEST)","by mail-lf1-x12e.google.com with SMTP id j9so7514570lfu.7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 25 Oct 2021 00:45:20 -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=\"fYpruDDd\"; 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=/c72nsUaOm0n0dQoHfa4CCYO1PBt/UPwRJOJvKKpu2c=;\n\tb=fYpruDDdtr7b0Mag90Vh41004NXTvMMZUczlxrl5n3kUX70EQqHq6wZT/hmIjENymX\n\tW0jPL+hVuVfGkOK5jJUja5wZNMLpll741bkVWonqMQoaf+tRe4WekSaTMLB/zPVTl9NG\n\tLzDVRJDaC3KyGgy9/m/Pa/iONOjU/S/faiZLBJji8XQoStyR5F+YIttwdBfy+0Zk69gX\n\tFZvNmW8Dn5wS4mJ9oZ4sNeATumpI9Z8hObLvVEX0GVOC6kWXMizvSlEndo2hkm0cHG79\n\tHz7R+FunubhcsC7HH5gy/ymTTdTkJtauWV3L9yNTsoBwYUoG9IIb7WqkB811gxsHlH/i\n\tO2zw==","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=/c72nsUaOm0n0dQoHfa4CCYO1PBt/UPwRJOJvKKpu2c=;\n\tb=q3QeINisi2xAaPS11vysiYo7r3Djy1OrGud9QqQfP5rcutcGJXb0CYqeWRl3z22Vm/\n\t6oG7p7MQX6oLeTK/3PZKjW5nFR47gWvWwyHNe9j3AFQchu5WehGih8MihLCQIEiHqdBo\n\ttvDb4XSq6+yk2Y4s8ScSln4eFJ7ZTRrh4/UCxIM26sruQ2gJ6VWrAK+oHasObQ6A1Qhq\n\tlJ4/OBEUEzNOCqSFvHTakzfTkJTkyLQJwOIG3ZblEIYAV7WP++L0eXBzfdWoW8FULH6c\n\tqRVdo1KLldGo3yTAkeGOyJys66roFRIOmJxMYZ+6xzU73+7j6gjtttaSONO3SOoApRny\n\trcVA==","X-Gm-Message-State":"AOAM530mWHcv/kcOSU/1a9Wlf5gSsu/u7fQNESYk/9xuvXQYLQuaqUQs\n\tysKALbHUcaNWn7GUKQpylPA5m8QZgTjtz40EVhQeFA==","X-Google-Smtp-Source":"ABdhPJzxmeCY6EupfCXULETfvTZwt5HtVr4i/afyPM25yDlj91esdejAHYSnBKqCUh01naYMGAtCGpMSQB5aAujAL0o=","X-Received":"by 2002:a05:6512:39d1:: with SMTP id\n\tk17mr616285lfu.79.1635147920288; \n\tMon, 25 Oct 2021 00:45:20 -0700 (PDT)","MIME-Version":"1.0","References":"<20211020110825.12902-1-david.plowman@raspberrypi.com>\n\t<20211020110825.12902-3-david.plowman@raspberrypi.com>","In-Reply-To":"<20211020110825.12902-3-david.plowman@raspberrypi.com>","From":"Naushir Patuck <naush@raspberrypi.com>","Date":"Mon, 25 Oct 2021 08:45:04 +0100","Message-ID":"<CAEmqJPouA1R7aTSrC=Bt_NBNNWqfOhxwT-KcWv76vW91qS2K8A@mail.gmail.com>","To":"David Plowman <david.plowman@raspberrypi.com>","Content-Type":"multipart/alternative; boundary=\"000000000000f1bcd705cf288a62\"","Subject":"Re: [libcamera-devel] [PATCH v3 2/7] libcamera: Add ColorSpace\n\tfields to StreamConfiguration","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":20424,"web_url":"https://patchwork.libcamera.org/comment/20424/","msgid":"<CAHW6GYJDiVyL8noz+K02ktDKOZ5FEa6ts-B6jKKk1Tz_S8HcMA@mail.gmail.com>","date":"2021-10-25T09:37:32","subject":"Re: [libcamera-devel] [PATCH v3 2/7] libcamera: Add ColorSpace\n\tfields to StreamConfiguration","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThanks for the feedback!\n\nOn Mon, 25 Oct 2021 at 08:45, Naushir Patuck <naush@raspberrypi.com> wrote:\n>\n> Hi David,\n>\n> Thank you for your patch.\n>\n> On Wed, 20 Oct 2021 at 12:08, David Plowman <david.plowman@raspberrypi.com> wrote:\n>>\n>> This is so that applications can choose appropriate color spaces which\n>> will then be passed down to the V4L2 devices.\n>>\n>> There are two fields: the \"requestedColorSpace\" which is the one an\n>> application wants, and the \"actualColorSpace\" which is what the\n>> underlying hardware can deliver, and which is filled in by the\n>> validate() method.\n>>\n>> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n>> ---\n>>  include/libcamera/stream.h |  4 ++++\n>>  src/libcamera/stream.cpp   | 25 +++++++++++++++++++++++++\n>>  2 files changed, 29 insertions(+)\n>>\n>> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n>> index 0c55e716..fe491ff5 100644\n>> --- a/include/libcamera/stream.h\n>> +++ b/include/libcamera/stream.h\n>> @@ -12,6 +12,7 @@\n>>  #include <string>\n>>  #include <vector>\n>>\n>> +#include <libcamera/color_space.h>\n>>  #include <libcamera/framebuffer.h>\n>>  #include <libcamera/geometry.h>\n>>  #include <libcamera/pixel_format.h>\n>> @@ -47,6 +48,9 @@ struct StreamConfiguration {\n>>\n>>         unsigned int bufferCount;\n>>\n>> +       ColorSpace requestedColorSpace;\n>> +       ColorSpace actualColorSpace;\n>> +\n>\n>\n> I had a brief look-ahead in this series, but it was not immediately\n> obvious to me why we would need requested and actual? Could\n> we make do with just one and update the user requested value\n> with what the HW will actually give us?\n\nYes, this is an important point so we need to understand if this is necessary!\n\nThe thing that was bothering me here was if the application asks for\nsome colour space but the driver gives us something else that the\nColorSpace class doesn't recognise, so we get \"undefined\" back, at\nleast in some of the fields.\n\nNow, if the application sees that validate() returns \"adjusted\", but\nthinks \"whatever, I'll just go with it\", then having \"undefined\" in\nthe colour space that we now request is actually an error. (Or at\nleast, I've defined it to be such.)\n\nAlternatively, if you look at the returned colour space, see that it's\n\"fully defined\", you can now set \"requested = actual\" and try\nvalidate() again, and it won't get \"adjusted\" (on account of the\ncolour space, anyway).\n\nDoes this make any sense, or am I being way too devious?\n\nThanks!\n\nDavis\n\n>\n>>\n>>         Stream *stream() const { return stream_; }\n>>         void setStream(Stream *stream) { stream_ = stream; }\n>>         const StreamFormats &formats() const { return formats_; }\n>> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n>> index b421e17e..1ddbbb8c 100644\n>> --- a/src/libcamera/stream.cpp\n>> +++ b/src/libcamera/stream.cpp\n>> @@ -329,6 +329,31 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats)\n>>   * \\brief Requested number of buffers to allocate for the stream\n>>   */\n>>\n>> +/**\n>> + * \\var StreamConfiguration::requestedColorSpace\n>> + * \\brief The ColorSpace this stream should have\n>> + *\n>> + * The generateConfiguration method will generate reasonable default\n>> + * values (ColorSpace::Jpeg for stills, ColorSpace::Rec709 for video and\n>> + * ColorSpace::Raw for raw streams) but applications are free to overwrite\n>> + * this value.\n>> + */\n>> +\n>> +/**\n>> + * \\var StreamConfiguration::actualColorSpace\n>> + * \\brief The ColorSpace the will be used for this stream\n>> + *\n>> + * This field is filled in by CameraConfiguration::validate().\n>> + * Normally this should match the requestedColorSpace, but it may differ\n>> + * if the hardware does not support it.\n>> + *\n>> + * In general cameras may have different constraints here, for example,\n>> + * they may require all output streams to share the same color space.\n>> + * Sometimes the fields within this color space may report \"Undefined\"\n>> + * values if the hardware drivers are going to use a color space that\n>> + * is not recognised by the ColorSpace class.\n>> + */\n>> +\n>>  /**\n>>   * \\fn StreamConfiguration::stream()\n>>   * \\brief Retrieve the stream associated with the configuration\n>> --\n>> 2.20.1\n>>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 590D3BF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 25 Oct 2021 09:37:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 727A064872;\n\tMon, 25 Oct 2021 11:37:44 +0200 (CEST)","from mail-wr1-x436.google.com (mail-wr1-x436.google.com\n\t[IPv6:2a00:1450:4864:20::436])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2515F60124\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 25 Oct 2021 11:37:43 +0200 (CEST)","by mail-wr1-x436.google.com with SMTP id w15so8032183wra.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 25 Oct 2021 02:37:43 -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=\"dPxngwxh\"; 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=YNvOG3fkBBTaHf/zD5HdRgffUCVx9lMHX85KFz4A19s=;\n\tb=dPxngwxh5rnMAIFKumZ0J6Bct5qT0ZrhPTBGVSOrKdvOgsHXWoPfePzhf5NO8ruePI\n\tBwjQcnZo8AjQd0EwvfjsL5VfPRPWnU5IdI9TTAh5zFEGXu1vS8TxGpVMBwtmV10jCEc5\n\tAWGTKqga4tnfgjBymBoJ/isCNPKULwSBaQ/LMTTS0T8Ivhxfa1GCxExFbh8S8wkUpu5p\n\t40aYmWYh3ZQEnmy4CM5VOq/kOn/1dUuTRiS3EX0VQZTa23HpSFi2krOdre4cjMxV2G2a\n\tdZbpA43x7t6iBdLlQJ4ApFgyW5340PwDZltuyAwolg0AnqOVWMRab0WjhGeQtMZLea+b\n\tRCjg==","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=YNvOG3fkBBTaHf/zD5HdRgffUCVx9lMHX85KFz4A19s=;\n\tb=lZA5Wd9A18nxv4c02mP7IyC/hzqAFSSVMvzQFbM4eJyxPgSRuVCGNgtfm2k5ypc00i\n\teR/okejDoIJUHNcIHSEM8gDofr0YVW2Qk+uPl3AqsNYi4QstAHjXiOkGa7wOcbCtw/d1\n\tDxiEgSkGgYjw4XG/mRfZ7iyO9P+PKIr25+XmIcxOhHKbd4ixRfn9qAglx7/kMX5cywDw\n\tJFLA+ef4qg9skvMvnFB4P2j3LC+P2P+Y7qp3MnQbjnNQaSJXhUVuVSylhszXqCt9r2L1\n\tDLfT4cjXa95coHGs/MZRPu9z+uVaHibK25jpPI1MAU6Uz3/T2m2ivz0UeZeQDEW2zcCa\n\tgnzg==","X-Gm-Message-State":"AOAM5320r0Jr+RCeIxQIy+Trrii1SyMEu4MZ7BVRe8EYu7wVNfispnIg\n\to3APDHD2FgiDL9+q5yLnFu59eZt6/+Px4c+AoDGXdgmnzfs=","X-Google-Smtp-Source":"ABdhPJwOPXWBU9SxGH4coAZ94+D3/fQNUMbEB3A/AIEzNxX05Y6xH868nEhK+Pyh3OYSGogY1SOGAr51oXP9W/5vuD8=","X-Received":"by 2002:a05:6000:4c:: with SMTP id\n\tk12mr16513284wrx.354.1635154662703; \n\tMon, 25 Oct 2021 02:37:42 -0700 (PDT)","MIME-Version":"1.0","References":"<20211020110825.12902-1-david.plowman@raspberrypi.com>\n\t<20211020110825.12902-3-david.plowman@raspberrypi.com>\n\t<CAEmqJPouA1R7aTSrC=Bt_NBNNWqfOhxwT-KcWv76vW91qS2K8A@mail.gmail.com>","In-Reply-To":"<CAEmqJPouA1R7aTSrC=Bt_NBNNWqfOhxwT-KcWv76vW91qS2K8A@mail.gmail.com>","From":"David Plowman <david.plowman@raspberrypi.com>","Date":"Mon, 25 Oct 2021 10:37:32 +0100","Message-ID":"<CAHW6GYJDiVyL8noz+K02ktDKOZ5FEa6ts-B6jKKk1Tz_S8HcMA@mail.gmail.com>","To":"Naushir Patuck <naush@raspberrypi.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v3 2/7] libcamera: Add ColorSpace\n\tfields to StreamConfiguration","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":20428,"web_url":"https://patchwork.libcamera.org/comment/20428/","msgid":"<CAEmqJPovx2BwHbLG40__-puJ-AhjoWvG68p2BFrxG+=87qkojQ@mail.gmail.com>","date":"2021-10-25T09:57:42","subject":"Re: [libcamera-devel] [PATCH v3 2/7] libcamera: Add ColorSpace\n\tfields to StreamConfiguration","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\nOn Mon, 25 Oct 2021 at 10:37, David Plowman <david.plowman@raspberrypi.com>\nwrote:\n\n> Hi Naush\n>\n> Thanks for the feedback!\n>\n> On Mon, 25 Oct 2021 at 08:45, Naushir Patuck <naush@raspberrypi.com>\n> wrote:\n> >\n> > Hi David,\n> >\n> > Thank you for your patch.\n> >\n> > On Wed, 20 Oct 2021 at 12:08, David Plowman <\n> david.plowman@raspberrypi.com> wrote:\n> >>\n> >> This is so that applications can choose appropriate color spaces which\n> >> will then be passed down to the V4L2 devices.\n> >>\n> >> There are two fields: the \"requestedColorSpace\" which is the one an\n> >> application wants, and the \"actualColorSpace\" which is what the\n> >> underlying hardware can deliver, and which is filled in by the\n> >> validate() method.\n> >>\n> >> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> >> ---\n> >>  include/libcamera/stream.h |  4 ++++\n> >>  src/libcamera/stream.cpp   | 25 +++++++++++++++++++++++++\n> >>  2 files changed, 29 insertions(+)\n> >>\n> >> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> >> index 0c55e716..fe491ff5 100644\n> >> --- a/include/libcamera/stream.h\n> >> +++ b/include/libcamera/stream.h\n> >> @@ -12,6 +12,7 @@\n> >>  #include <string>\n> >>  #include <vector>\n> >>\n> >> +#include <libcamera/color_space.h>\n> >>  #include <libcamera/framebuffer.h>\n> >>  #include <libcamera/geometry.h>\n> >>  #include <libcamera/pixel_format.h>\n> >> @@ -47,6 +48,9 @@ struct StreamConfiguration {\n> >>\n> >>         unsigned int bufferCount;\n> >>\n> >> +       ColorSpace requestedColorSpace;\n> >> +       ColorSpace actualColorSpace;\n> >> +\n> >\n> >\n> > I had a brief look-ahead in this series, but it was not immediately\n> > obvious to me why we would need requested and actual? Could\n> > we make do with just one and update the user requested value\n> > with what the HW will actually give us?\n>\n> Yes, this is an important point so we need to understand if this is\n> necessary!\n>\n> The thing that was bothering me here was if the application asks for\n> some colour space but the driver gives us something else that the\n> ColorSpace class doesn't recognise, so we get \"undefined\" back, at\n> least in some of the fields.\n>\n> Now, if the application sees that validate() returns \"adjusted\", but\n> thinks \"whatever, I'll just go with it\", then having \"undefined\" in\n> the colour space that we now request is actually an error. (Or at\n> least, I've defined it to be such.)\n>\n> Alternatively, if you look at the returned colour space, see that it's\n> \"fully defined\", you can now set \"requested = actual\" and try\n> validate() again, and it won't get \"adjusted\" (on account of the\n> colour space, anyway).\n>\n> Does this make any sense, or am I being way too devious?\n>\n\nYes, that reasoning makes sense.  I suppose we can achieve the same result\nwith\na single member variable, but add a tad more logic to the applications. I\nhave no strong\ninclination either way.\n\nNaush\n\n\n>\n> Thanks!\n>\n> Davis\n>\n> >\n> >>\n> >>         Stream *stream() const { return stream_; }\n> >>         void setStream(Stream *stream) { stream_ = stream; }\n> >>         const StreamFormats &formats() const { return formats_; }\n> >> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> >> index b421e17e..1ddbbb8c 100644\n> >> --- a/src/libcamera/stream.cpp\n> >> +++ b/src/libcamera/stream.cpp\n> >> @@ -329,6 +329,31 @@ StreamConfiguration::StreamConfiguration(const\n> StreamFormats &formats)\n> >>   * \\brief Requested number of buffers to allocate for the stream\n> >>   */\n> >>\n> >> +/**\n> >> + * \\var StreamConfiguration::requestedColorSpace\n> >> + * \\brief The ColorSpace this stream should have\n> >> + *\n> >> + * The generateConfiguration method will generate reasonable default\n> >> + * values (ColorSpace::Jpeg for stills, ColorSpace::Rec709 for video\n> and\n> >> + * ColorSpace::Raw for raw streams) but applications are free to\n> overwrite\n> >> + * this value.\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\var StreamConfiguration::actualColorSpace\n> >> + * \\brief The ColorSpace the will be used for this stream\n> >> + *\n> >> + * This field is filled in by CameraConfiguration::validate().\n> >> + * Normally this should match the requestedColorSpace, but it may\n> differ\n> >> + * if the hardware does not support it.\n> >> + *\n> >> + * In general cameras may have different constraints here, for example,\n> >> + * they may require all output streams to share the same color space.\n> >> + * Sometimes the fields within this color space may report \"Undefined\"\n> >> + * values if the hardware drivers are going to use a color space that\n> >> + * is not recognised by the ColorSpace class.\n> >> + */\n> >> +\n> >>  /**\n> >>   * \\fn StreamConfiguration::stream()\n> >>   * \\brief Retrieve the stream associated with the configuration\n> >> --\n> >> 2.20.1\n> >>\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 16D22BDB1C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 25 Oct 2021 09:58:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CC91C64870;\n\tMon, 25 Oct 2021 11:57:59 +0200 (CEST)","from mail-qk1-x736.google.com (mail-qk1-x736.google.com\n\t[IPv6:2607:f8b0:4864:20::736])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0FFB260124\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 25 Oct 2021 11:57:59 +0200 (CEST)","by mail-qk1-x736.google.com with SMTP id bj31so11280808qkb.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 25 Oct 2021 02:57:59 -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=\"pNeWXfFc\"; 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=36GSKwkW5/1GyWDnk/D0SpHn2k24un3ZYQh+HBKm+HM=;\n\tb=pNeWXfFcQFZS7KFgdShntkbqmLjCaoigCsqEFL9SLGClB8//cHIK3I5JlXAoDLrWB3\n\tOng9RSnCfqCo/2STNySOtGdi8UVagl/jQjrZP73885ogm4KZLqUhhUm8pbc5T9e3ef/e\n\tye1CTEiYVXqqJAVs3yT6eBHPJodFBh4K2h/hqP1y6W36ICaYwri4z8s85jWivNTnSAjs\n\tptleadNA6u3ccVjOpbxm63Yzb4dMDjBPtFWegflGKNHyv8dXrsev9jwgPQik44gAm9Pg\n\tUdGsN/4kDbQBACcap6n//yVE9YTKXwwY4V1AEAt5nsw4OVe8cl1UT3woFnKp0rmXCfrg\n\tkEkg==","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=36GSKwkW5/1GyWDnk/D0SpHn2k24un3ZYQh+HBKm+HM=;\n\tb=mrCMHQX8NMK1d+kDF0xwu0E+Cf4CnhNZa2Hg3t8Xw7wyIkdwL5D9ZuqHOteHAk76b1\n\tw8PbxBEE7Ckt+tfsr95S6OB4PvYNBqIyQH0ilJ6HJIVr8JObO9yPJ0d6v5q+NyJhayZT\n\tEtTFD9OyLClpZU16PwyADkq7qLla6Y0J0p0s4Xh6JM1jQAixeBuOZoTi22UZk1MaGQw8\n\ta+vuoVrHKZUBW0Pw98AC81DZ0N0OIMifiFcINU8xIyEGzXUBroRQRUDoF0nAGWMMP81A\n\tlINJftDsbC3kCKrlQAy0neFJbvLV03uG/JD5FRWw5FetR1F0qwey25h8vZmpqEIBOoMB\n\tHZTA==","X-Gm-Message-State":"AOAM530hcqaE4y7IfzvAF6PT5OYrBi3Vid0VE/kw1gg3v0ToEe0gjYl1\n\tZP2eTok5tUUJ7PXB0qAKq8O8rWt0MSxQjP3+vStTBA==","X-Google-Smtp-Source":"ABdhPJze0JnlocFBAkfQz+wBc4jcr3lfc+OyQERL9XUiQW17rmrJ29iH3SGsaseUNsinBo2vRvHM5Qd1AoDmUVVbfqw=","X-Received":"by 2002:a37:58c4:: with SMTP id\n\tm187mr12483708qkb.481.1635155877682; \n\tMon, 25 Oct 2021 02:57:57 -0700 (PDT)","MIME-Version":"1.0","References":"<20211020110825.12902-1-david.plowman@raspberrypi.com>\n\t<20211020110825.12902-3-david.plowman@raspberrypi.com>\n\t<CAEmqJPouA1R7aTSrC=Bt_NBNNWqfOhxwT-KcWv76vW91qS2K8A@mail.gmail.com>\n\t<CAHW6GYJDiVyL8noz+K02ktDKOZ5FEa6ts-B6jKKk1Tz_S8HcMA@mail.gmail.com>","In-Reply-To":"<CAHW6GYJDiVyL8noz+K02ktDKOZ5FEa6ts-B6jKKk1Tz_S8HcMA@mail.gmail.com>","From":"Naushir Patuck <naush@raspberrypi.com>","Date":"Mon, 25 Oct 2021 10:57:42 +0100","Message-ID":"<CAEmqJPovx2BwHbLG40__-puJ-AhjoWvG68p2BFrxG+=87qkojQ@mail.gmail.com>","To":"David Plowman <david.plowman@raspberrypi.com>","Content-Type":"multipart/alternative; boundary=\"0000000000003df45405cf2a65ef\"","Subject":"Re: [libcamera-devel] [PATCH v3 2/7] libcamera: Add ColorSpace\n\tfields to StreamConfiguration","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]