[{"id":34931,"web_url":"https://patchwork.libcamera.org/comment/34931/","msgid":"<175284070718.560048.16576681811250947472@ping.linuxembedded.co.uk>","date":"2025-07-18T12:11:47","subject":"Re: [PATCH] libcamera: android: Add optional mirrored value for\n\tconfig","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Allen Ballway (2025-07-16 21:46:20)\n> Adds an optional `mirrored` value to the HAL config, which is used to modify\n> the `orientation` of the camera config. No rotation is used, so it can\n> only set the orientation to `Orientation::Rotate0Mirror`.\n> \n> This enables sensors which are incorrectly mirrored to be corrected.\n> \n> Signed-off-by: Allen Ballway \n> ---\n> src/android/camera_device.cpp | 6 ++++++\n> src/android/camera_device.h | 1 +\n> src/android/camera_hal_config.cpp | 14 ++++++++++++++\n> src/android/camera_hal_config.h | 1 +\n> 4 files changed, 22 insertions(+)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 80ff248c2..b20e9f3db 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -377,6 +377,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)\n> orientation_ = 0;\n> }\n> \n> + mirrored_ = cameraConfigData && cameraConfigData->mirrored;\n> return capabilities_.initialize(camera_, orientation_, facing_);\n> }\n> \n> @@ -547,6 +548,11 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> return -EINVAL;\n> }\n> \n> + // Rotation is unsupported, so if mirrored just set orientation.\n\nIs that true? I thought we usually support 0 and 180 rotations on most\nsensors at least ?\n\nWe (almost) always use /* */ comments in libcamera ... it's ... probably a\nstylistic choice because we are mostly started out as C / Linux-kernel\ndevs.\n\n\n\n> + if (mirrored_) {\n> + config->orientation = Orientation::Rotate0Mirror;\n> + }\n> +\n\nHow does this interact with rotation as well ? will it take precedence\nagainst 180 rotations?\n\n> /*\n> * Clear and remove any existing configuration from previous calls, and\n> * ensure the required entries are available without further\n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index 194ca3030..d304a1285 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -126,6 +126,7 @@ private:\n> \n> int facing_;\n> int orientation_;\n> + int mirrored_;\n> \n> CameraMetadata lastSettings_;\n> };\n> diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp\n> index 7ef451ef8..a2beba5d3 100644\n> --- a/src/android/camera_hal_config.cpp\n> +++ b/src/android/camera_hal_config.cpp\n> @@ -33,6 +33,7 @@ private:\n> int parseCameraConfigData(const std::string &cameraId, const YamlObject &);\n> int parseLocation(const YamlObject &, CameraConfigData &cameraConfigData);\n> int parseRotation(const YamlObject &, CameraConfigData &cameraConfigData);\n> + int parseMirrored(const YamlObject &, CameraConfigData &cameraConfigData);\n> \n> std::map *cameras_;\n> };\n> @@ -106,6 +107,9 @@ int CameraHalConfig::Private::parseCameraConfigData(const std::string &cameraId,\n> if (parseRotation(cameraObject, cameraConfigData))\n> return -EINVAL;\n> \n> + /* Parse optional property \"mirrored\" */\n> + parseMirrored(cameraObject, cameraConfigData);\n> +\n> return 0;\n> }\n> \n> @@ -145,6 +149,16 @@ int CameraHalConfig::Private::parseRotation(const YamlObject &cameraObject,\n> return 0;\n> }\n> \n> +int CameraHalConfig::Private::parseMirrored(const YamlObject &cameraObject,\n> + CameraConfigData &cameraConfigData)\n> +{\n> + if (!cameraObject.contains(\"mirrored\"))\n> + return -EINVAL;\n> +\n> + cameraConfigData.mirrored = cameraObject[\"mirrored\"].get(false);\n> + return 0;\n> +}\n> +\n> CameraHalConfig::CameraHalConfig()\n> : Extensible(std::make_unique()), exists_(false), valid_(false)\n> {\n> diff --git a/src/android/camera_hal_config.h b/src/android/camera_hal_config.h\n> index a4bedb6e6..a96190470 100644\n> --- a/src/android/camera_hal_config.h\n> +++ b/src/android/camera_hal_config.h\n> @@ -15,6 +15,7 @@\n> struct CameraConfigData {\n> int facing = -1;\n> int rotation = -1;\n> + bool mirrored = false;\n> };\n> \n> class CameraHalConfig final : public libcamera::Extensible\n> --\n> 2.50.0.727.gbf7dc18ff4-goog\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 EFC61C3237\n\tfor ;\n\tFri, 18 Jul 2025 12:11:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 071A468F9D;\n\tFri, 18 Jul 2025 14:11:52 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8A2C16150F\n\tfor ;\n\tFri, 18 Jul 2025 14:11:50 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B6CD159DF;\n\tFri, 18 Jul 2025 14:11:15 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"uhxJbBGm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1752840675;\n\tbh=/dvRYBMdrqlC8gDCkNDUCyFhIjfrw808gyftmaAuVVA=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=uhxJbBGm9L27d7KfMn8bCqe3oQ0ZLOsR8x+Hu/0JfoqUmXEET+qdstfa3L+7feVXn\n\tL6occ9hRYz1cmFOeg4c8XV1PCnnFp4ZysuwbTzVbQJzgCE/tbHQ4zlVnHnu4tUY0YV\n\tWZJygc9NgVCWbNfM/GwJLsQszgs+knYeNsMosmqc=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250716204625.3221592-1-ballway@chromium.org>","References":"<20250716204625.3221592-1-ballway@chromium.org>","Subject":"Re: [PATCH] libcamera: android: Add optional mirrored value for\n\tconfig","From":"Kieran Bingham ","Cc":"hanlinchen@chromium.org, Allen Ballway ","To":"Allen Ballway , libcamera-devel@lists.libcamera.org","Date":"Fri, 18 Jul 2025 13:11:47 +0100","Message-ID":"<175284070718.560048.16576681811250947472@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":34944,"web_url":"https://patchwork.libcamera.org/comment/34944/","msgid":"","date":"2025-07-18T15:27:23","subject":"Re: [PATCH] libcamera: android: Add optional mirrored value for\n\tconfig","submitter":{"id":236,"url":"https://patchwork.libcamera.org/api/people/236/","name":"Allen Ballway","email":"ballway@chromium.org"},"content":"On Fri, Jul 18, 2025 at 5:11 AM Kieran Bingham\n wrote:\n>\n> Quoting Allen Ballway (2025-07-16 21:46:20)\n> > Adds an optional `mirrored` value to the HAL config, which is used to modify\n> > the `orientation` of the camera config. No rotation is used, so it can\n> > only set the orientation to `Orientation::Rotate0Mirror`.\n> >\n> > This enables sensors which are incorrectly mirrored to be corrected.\n> >\n> > Signed-off-by: Allen Ballway \n> > ---\n> > src/android/camera_device.cpp | 6 ++++++\n> > src/android/camera_device.h | 1 +\n> > src/android/camera_hal_config.cpp | 14 ++++++++++++++\n> > src/android/camera_hal_config.h | 1 +\n> > 4 files changed, 22 insertions(+)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 80ff248c2..b20e9f3db 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -377,6 +377,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)\n> > orientation_ = 0;\n> > }\n> >\n> > + mirrored_ = cameraConfigData && cameraConfigData->mirrored;\n> > return capabilities_.initialize(camera_, orientation_, facing_);\n> > }\n> >\n> > @@ -547,6 +548,11 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > return -EINVAL;\n> > }\n> >\n> > + // Rotation is unsupported, so if mirrored just set orientation.\n>\n> Is that true? I thought we usually support 0 and 180 rotations on most\n> sensors at least ?\n\nAh I misunderstood the below comments/errors about not supporting stream\nrotations. It seems the sensors on my test device don't support rotation. I will\nsend out a v2 with this comment removed assuming the rest of this patch\nlooks good.\n>\n> We (almost) always use /* */ comments in libcamera ... it's ... probably a\n> stylistic choice because we are mostly started out as C / Linux-kernel\n> devs.\n>\n>\n>\n> > + if (mirrored_) {\n> > + config->orientation = Orientation::Rotate0Mirror;\n> > + }\n> > +\n>\n> How does this interact with rotation as well ? will it take precedence\n> against 180 rotations?\n\nI can't confirm because my sensors aren't affected by the rotation config\nbut as far as I can tell the rotation would be unaffected by setting the\norientation to mirror. There doesn't seem to be anywhere setting the\norientation in the Android layer, so any rotation would happen independently.\nAnd the mirroring will just set the HFLIP on the kernel driver, so any higher\nlevel rotation would just rotate the flipped output as expected.\n>\n> > /*\n> > * Clear and remove any existing configuration from previous calls, and\n> > * ensure the required entries are available without further\n> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > index 194ca3030..d304a1285 100644\n> > --- a/src/android/camera_device.h\n> > +++ b/src/android/camera_device.h\n> > @@ -126,6 +126,7 @@ private:\n> >\n> > int facing_;\n> > int orientation_;\n> > + int mirrored_;\n> >\n> > CameraMetadata lastSettings_;\n> > };\n> > diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp\n> > index 7ef451ef8..a2beba5d3 100644\n> > --- a/src/android/camera_hal_config.cpp\n> > +++ b/src/android/camera_hal_config.cpp\n> > @@ -33,6 +33,7 @@ private:\n> > int parseCameraConfigData(const std::string &cameraId, const YamlObject &);\n> > int parseLocation(const YamlObject &, CameraConfigData &cameraConfigData);\n> > int parseRotation(const YamlObject &, CameraConfigData &cameraConfigData);\n> > + int parseMirrored(const YamlObject &, CameraConfigData &cameraConfigData);\n> >\n> > std::map *cameras_;\n> > };\n> > @@ -106,6 +107,9 @@ int CameraHalConfig::Private::parseCameraConfigData(const std::string &cameraId,\n> > if (parseRotation(cameraObject, cameraConfigData))\n> > return -EINVAL;\n> >\n> > + /* Parse optional property \"mirrored\" */\n> > + parseMirrored(cameraObject, cameraConfigData);\n> > +\n> > return 0;\n> > }\n> >\n> > @@ -145,6 +149,16 @@ int CameraHalConfig::Private::parseRotation(const YamlObject &cameraObject,\n> > return 0;\n> > }\n> >\n> > +int CameraHalConfig::Private::parseMirrored(const YamlObject &cameraObject,\n> > + CameraConfigData &cameraConfigData)\n> > +{\n> > + if (!cameraObject.contains(\"mirrored\"))\n> > + return -EINVAL;\n> > +\n> > + cameraConfigData.mirrored = cameraObject[\"mirrored\"].get(false);\n> > + return 0;\n> > +}\n> > +\n> > CameraHalConfig::CameraHalConfig()\n> > : Extensible(std::make_unique()), exists_(false), valid_(false)\n> > {\n> > diff --git a/src/android/camera_hal_config.h b/src/android/camera_hal_config.h\n> > index a4bedb6e6..a96190470 100644\n> > --- a/src/android/camera_hal_config.h\n> > +++ b/src/android/camera_hal_config.h\n> > @@ -15,6 +15,7 @@\n> > struct CameraConfigData {\n> > int facing = -1;\n> > int rotation = -1;\n> > + bool mirrored = false;\n> > };\n> >\n> > class CameraHalConfig final : public libcamera::Extensible\n> > --\n> > 2.50.0.727.gbf7dc18ff4-goog\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 3D005BE175\n\tfor ;\n\tFri, 18 Jul 2025 15:27:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 87D2F68FAF;\n\tFri, 18 Jul 2025 17:27:36 +0200 (CEST)","from mail-ed1-x533.google.com (mail-ed1-x533.google.com\n\t[IPv6:2a00:1450:4864:20::533])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 37D686150F\n\tfor ;\n\tFri, 18 Jul 2025 17:27:35 +0200 (CEST)","by mail-ed1-x533.google.com with SMTP id\n\t4fb4d7f45d1cf-607cc1a2bd8so3469999a12.2\n\tfor ;\n\tFri, 18 Jul 2025 08:27:35 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"Gi2JeheW\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1752852454; x=1753457254;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=G0j2iJ8MylVP3KUxKPdTFOzy479cREwr+91X6MGs8Q4=;\n\tb=Gi2JeheWFCS6kytNPJrQ8on81e+woE2lHrQvRNAwDMpgc3ohe4Lg3zX2Sdf9I+8uaD\n\tb+48+lIom0M6VUvGIlNhOgfdmdCPF573sYSFDndUGhpyIH+pdwX11dUc1Iq8eZEpH2AJ\n\t+2ie1kYT13AJvi9U/gNHjnaLXaWs9WZesvhcI=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1752852454; x=1753457254;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=G0j2iJ8MylVP3KUxKPdTFOzy479cREwr+91X6MGs8Q4=;\n\tb=gvJ5NdP7D6UfMzj/+7ZzWyUwI81Vs/saMo05cDt8ezkJ67I4lGbyiRiUWpzR3Pkjoh\n\t8cSRvp3yUgDlRw1cNjgUYlvD11sTqx+GZKtGiwVMj2thwf5/bIejbE7DUEboCS03TEMu\n\t+2CPeKCqthkN9z2U72aVEWmRTUGLVH46i7sf19VH547fbtCJ9zRxg2Lrs8+YeZucvuY9\n\ttbbVBtnidkW4OLx4NrTBM2GQ0UakPJnX2wPjNEAm4hEgnV9hAjnAxJ2gy9yafwSO5zY7\n\tpR+P+2Nj1cU2iWZAyz4Bwbwgec0xicOHGLRKtV3RbfoqnSqDgOuAWNRFHv4zg5dMmdmW\n\tmh9w==","X-Gm-Message-State":"AOJu0YwwV8Yudxd+WynwI7ZWiTIx4qC0I01wNxCIzki+6YncQcWIjt8+\n\tB2Ao7AlIi+ar05EuKqfUpA9eEyhba0Efid9ByWgs369U+cYmIIg6spSYj+8HXtCWOu2QJhL4caa\n\t7XirI1EjGfnc/0lbjajWD8SIggjGAjrHplTMlFv+e","X-Gm-Gg":"ASbGncvZBdrKDfAbe7Ckw81fZlsU2BY0x8ZHEaaRSotZQe4JI1I/QMcavNXm8p+dLr8\n\tDE/F7++O1VXsEz+iPw6vFe3eySmfHtOmQoTZYhOgcmCgwbX6BdXyNtsdUSg5L9n3yP9rnI7stVX\n\tVZuUtRPJc75RWyT4uU/i6vC6tqcWRU5BjMhnnrjAuwS2UJVG2usOw/ZuLmRiq7VIJ9MSH5+2ZHq\n\tzCMfdY=","X-Google-Smtp-Source":"AGHT+IHtl0RivlK+tEzeFw0tKdVDl3kKHsmHg4FlmtlUsqkzji7O2MLww9L93ZJon//dPp6+ZvLkdUrVkiNZRM3/v0o=","X-Received":"by 2002:a17:906:f049:b0:ae3:674a:9af1 with SMTP id\n\ta640c23a62f3a-ae9ce198bd6mr1085799266b.57.1752852454443;\n\tFri, 18 Jul 2025 08:27:34 -0700 (PDT)","MIME-Version":"1.0","References":"<20250716204625.3221592-1-ballway@chromium.org>\n\t<175284070718.560048.16576681811250947472@ping.linuxembedded.co.uk>","In-Reply-To":"<175284070718.560048.16576681811250947472@ping.linuxembedded.co.uk>","From":"Allen Ballway ","Date":"Fri, 18 Jul 2025 08:27:23 -0700","X-Gm-Features":"Ac12FXy1refmpjYxTIhJXx2AoMg1U74MbqSDv5QjzWh0ajdirWvN3ou7cnmBudQ","Message-ID":"","Subject":"Re: [PATCH] libcamera: android: Add optional mirrored value for\n\tconfig","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org, hanlinchen@chromium.org","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":34960,"web_url":"https://patchwork.libcamera.org/comment/34960/","msgid":"<175309465839.50296.11090024501206804447@ping.linuxembedded.co.uk>","date":"2025-07-21T10:44:18","subject":"Re: [PATCH] libcamera: android: Add optional mirrored value for\n\tconfig","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Allen Ballway (2025-07-18 16:27:23)\n> On Fri, Jul 18, 2025 at 5:11 AM Kieran Bingham\n> wrote:\n> >\n> > Quoting Allen Ballway (2025-07-16 21:46:20)\n> > > Adds an optional `mirrored` value to the HAL config, which is used to modify\n> > > the `orientation` of the camera config. No rotation is used, so it can\n> > > only set the orientation to `Orientation::Rotate0Mirror`.\n> > >\n> > > This enables sensors which are incorrectly mirrored to be corrected.\n> > >\n> > > Signed-off-by: Allen Ballway \n> > > ---\n> > > src/android/camera_device.cpp | 6 ++++++\n> > > src/android/camera_device.h | 1 +\n> > > src/android/camera_hal_config.cpp | 14 ++++++++++++++\n> > > src/android/camera_hal_config.h | 1 +\n> > > 4 files changed, 22 insertions(+)\n> > >\n> > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > index 80ff248c2..b20e9f3db 100644\n> > > --- a/src/android/camera_device.cpp\n> > > +++ b/src/android/camera_device.cpp\n> > > @@ -377,6 +377,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)\n> > > orientation_ = 0;\n> > > }\n> > >\n> > > + mirrored_ = cameraConfigData && cameraConfigData->mirrored;\n> > > return capabilities_.initialize(camera_, orientation_, facing_);\n> > > }\n> > >\n> > > @@ -547,6 +548,11 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > > return -EINVAL;\n> > > }\n> > >\n> > > + // Rotation is unsupported, so if mirrored just set orientation.\n> >\n> > Is that true? I thought we usually support 0 and 180 rotations on most\n> > sensors at least ?\n> \n> Ah I misunderstood the below comments/errors about not supporting stream\n> rotations. It seems the sensors on my test device don't support rotation. I will\n> send out a v2 with this comment removed assuming the rest of this patch\n> looks good.\n> >\n> > We (almost) always use /* */ comments in libcamera ... it's ... probably a\n> > stylistic choice because we are mostly started out as C / Linux-kernel\n> > devs.\n> >\n> >\n> >\n> > > + if (mirrored_) {\n> > > + config->orientation = Orientation::Rotate0Mirror;\n> > > + }\n> > > +\n> >\n> > How does this interact with rotation as well ? will it take precedence\n> > against 180 rotations?\n> \n> I can't confirm because my sensors aren't affected by the rotation config\n> but as far as I can tell the rotation would be unaffected by setting the\n> orientation to mirror. There doesn't seem to be anywhere setting the\n> orientation in the Android layer, so any rotation would happen independently.\n> And the mirroring will just set the HFLIP on the kernel driver, so any higher\n> level rotation would just rotate the flipped output as expected.\n\nSome level of rotation is supposed to be handled automatically by\nsetting the physical orientation and rotation in device tree.\n\nThen libcamera will 'aim' for a 0 rotation by default.\n\nI guess the issue here is that we don't have a way in\ndevice-tree/firmware to mark that the camera is mirrored.\n\nwhat does this actually 'mean' by the way? How is the camera physically\nmirrored? Is this really a bug in the driver somewhere or something\nelse?\n\nI am weary that this patch is possibly a workaround for a problem\nsomewhere else ?\n\n--\nKieran\n\n> >\n> > > /*\n> > > * Clear and remove any existing configuration from previous calls, and\n> > > * ensure the required entries are available without further\n> > > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > > index 194ca3030..d304a1285 100644\n> > > --- a/src/android/camera_device.h\n> > > +++ b/src/android/camera_device.h\n> > > @@ -126,6 +126,7 @@ private:\n> > >\n> > > int facing_;\n> > > int orientation_;\n> > > + int mirrored_;\n> > >\n> > > CameraMetadata lastSettings_;\n> > > };\n> > > diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp\n> > > index 7ef451ef8..a2beba5d3 100644\n> > > --- a/src/android/camera_hal_config.cpp\n> > > +++ b/src/android/camera_hal_config.cpp\n> > > @@ -33,6 +33,7 @@ private:\n> > > int parseCameraConfigData(const std::string &cameraId, const YamlObject &);\n> > > int parseLocation(const YamlObject &, CameraConfigData &cameraConfigData);\n> > > int parseRotation(const YamlObject &, CameraConfigData &cameraConfigData);\n> > > + int parseMirrored(const YamlObject &, CameraConfigData &cameraConfigData);\n> > >\n> > > std::map *cameras_;\n> > > };\n> > > @@ -106,6 +107,9 @@ int CameraHalConfig::Private::parseCameraConfigData(const std::string &cameraId,\n> > > if (parseRotation(cameraObject, cameraConfigData))\n> > > return -EINVAL;\n> > >\n> > > + /* Parse optional property \"mirrored\" */\n> > > + parseMirrored(cameraObject, cameraConfigData);\n> > > +\n> > > return 0;\n> > > }\n> > >\n> > > @@ -145,6 +149,16 @@ int CameraHalConfig::Private::parseRotation(const YamlObject &cameraObject,\n> > > return 0;\n> > > }\n> > >\n> > > +int CameraHalConfig::Private::parseMirrored(const YamlObject &cameraObject,\n> > > + CameraConfigData &cameraConfigData)\n> > > +{\n> > > + if (!cameraObject.contains(\"mirrored\"))\n> > > + return -EINVAL;\n> > > +\n> > > + cameraConfigData.mirrored = cameraObject[\"mirrored\"].get(false);\n> > > + return 0;\n> > > +}\n> > > +\n> > > CameraHalConfig::CameraHalConfig()\n> > > : Extensible(std::make_unique()), exists_(false), valid_(false)\n> > > {\n> > > diff --git a/src/android/camera_hal_config.h b/src/android/camera_hal_config.h\n> > > index a4bedb6e6..a96190470 100644\n> > > --- a/src/android/camera_hal_config.h\n> > > +++ b/src/android/camera_hal_config.h\n> > > @@ -15,6 +15,7 @@\n> > > struct CameraConfigData {\n> > > int facing = -1;\n> > > int rotation = -1;\n> > > + bool mirrored = false;\n> > > };\n> > >\n> > > class CameraHalConfig final : public libcamera::Extensible\n> > > --\n> > > 2.50.0.727.gbf7dc18ff4-goog\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 1A760C3237\n\tfor ;\n\tMon, 21 Jul 2025 10:44:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0E25868FD0;\n\tMon, 21 Jul 2025 12:44:24 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B0FEC68FB1\n\tfor ;\n\tMon, 21 Jul 2025 12:44:22 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B5EDB795A;\n\tMon, 21 Jul 2025 12:43:45 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"owbaZftG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1753094625;\n\tbh=oFo3wKMpg3ofVMY4XK1IWxo6hQcyqT7ATkYul3FTvR4=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=owbaZftG2OJzpi2alAjovLeZ52LAF+c/2H2osUSa33B4DYeY3vxgNU0buG1rDT1BC\n\tT0HwhziSCtNlAesJ1aTetin0RrATBjdvck/1PO69+vmvP/HTgexHPfYPQi9yMxr3Kj\n\tZOoO3gdIhqsVbbVgE/D4oq+SBA58b1p0ARQG5fE8=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20250716204625.3221592-1-ballway@chromium.org>\n\t<175284070718.560048.16576681811250947472@ping.linuxembedded.co.uk>\n\t","Subject":"Re: [PATCH] libcamera: android: Add optional mirrored value for\n\tconfig","From":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org, hanlinchen@chromium.org","To":"Allen Ballway ","Date":"Mon, 21 Jul 2025 11:44:18 +0100","Message-ID":"<175309465839.50296.11090024501206804447@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":34964,"web_url":"https://patchwork.libcamera.org/comment/34964/","msgid":"<6tra7qb2ru2nt4cgzpfvtwhcccxjp2qmw52ii4rcizw474cppy@ngyrzgfdxjhy>","date":"2025-07-21T11:14:08","subject":"Re: [PATCH] libcamera: android: Add optional mirrored value for\n\tconfig","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Kieran\n\nOn Mon, Jul 21, 2025 at 11:44:18AM +0100, Kieran Bingham wrote:\n> Quoting Allen Ballway (2025-07-18 16:27:23)\n> > On Fri, Jul 18, 2025 at 5:11 AM Kieran Bingham\n> > wrote:\n> > >\n> > > Quoting Allen Ballway (2025-07-16 21:46:20)\n> > > > Adds an optional `mirrored` value to the HAL config, which is used to modify\n> > > > the `orientation` of the camera config. No rotation is used, so it can\n> > > > only set the orientation to `Orientation::Rotate0Mirror`.\n> > > >\n> > > > This enables sensors which are incorrectly mirrored to be corrected.\n> > > >\n> > > > Signed-off-by: Allen Ballway \n> > > > ---\n> > > > src/android/camera_device.cpp | 6 ++++++\n> > > > src/android/camera_device.h | 1 +\n> > > > src/android/camera_hal_config.cpp | 14 ++++++++++++++\n> > > > src/android/camera_hal_config.h | 1 +\n> > > > 4 files changed, 22 insertions(+)\n> > > >\n> > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > > index 80ff248c2..b20e9f3db 100644\n> > > > --- a/src/android/camera_device.cpp\n> > > > +++ b/src/android/camera_device.cpp\n> > > > @@ -377,6 +377,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)\n> > > > orientation_ = 0;\n> > > > }\n> > > >\n> > > > + mirrored_ = cameraConfigData && cameraConfigData->mirrored;\n> > > > return capabilities_.initialize(camera_, orientation_, facing_);\n> > > > }\n> > > >\n> > > > @@ -547,6 +548,11 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > > > return -EINVAL;\n> > > > }\n> > > >\n> > > > + // Rotation is unsupported, so if mirrored just set orientation.\n> > >\n> > > Is that true? I thought we usually support 0 and 180 rotations on most\n> > > sensors at least ?\n> >\n> > Ah I misunderstood the below comments/errors about not supporting stream\n> > rotations. It seems the sensors on my test device don't support rotation. I will\n> > send out a v2 with this comment removed assuming the rest of this patch\n> > looks good.\n> > >\n> > > We (almost) always use /* */ comments in libcamera ... it's ... probably a\n> > > stylistic choice because we are mostly started out as C / Linux-kernel\n> > > devs.\n> > >\n> > >\n> > >\n> > > > + if (mirrored_) {\n> > > > + config->orientation = Orientation::Rotate0Mirror;\n> > > > + }\n> > > > +\n\nAlso no {} for single line statement.\n\nIt's amusing how many times the same stylistic comments had to\nrepeated specifically to chromeos developers. We also have tools\navailable for checking this kind of issues before submitting.\n\n> > >\n> > > How does this interact with rotation as well ? will it take precedence\n> > > against 180 rotations?\n> >\n> > I can't confirm because my sensors aren't affected by the rotation config\n> > but as far as I can tell the rotation would be unaffected by setting the\n> > orientation to mirror. There doesn't seem to be anywhere setting the\n> > orientation in the Android layer, so any rotation would happen independently.\n> > And the mirroring will just set the HFLIP on the kernel driver, so any higher\n> > level rotation would just rotate the flipped output as expected.\n>\n> Some level of rotation is supposed to be handled automatically by\n> setting the physical orientation and rotation in device tree.\n>\n> Then libcamera will 'aim' for a 0 rotation by default.\n>\n> I guess the issue here is that we don't have a way in\n> device-tree/firmware to mark that the camera is mirrored.\n>\n> what does this actually 'mean' by the way? How is the camera physically\n> mirrored? Is this really a bug in the driver somewhere or something\n> else?\n>\n> I am weary that this patch is possibly a workaround for a problem\n> somewhere else ?\n\nIf my recollection is right, when it comes to the camera HAL:\n\n1) Rotation is first parsed from DT, but there we can only express\n [0, 90, 270, 360] so we don't have a \"mirror\" option\n\n2) If no \"rotation\" property is specified in DT, then the HAL\n falls-back to parse a \"rotation\" property from configuration file,\n where again we can only express an angle between [0 - 360]\n\nAll of this seems to suggest it is not possible to express in DT or\nHAL configuration file the \"mirroring\" option. However this opens the\nquestion that Kieran has asked already. How is the camera module\nphysically mirrored ? is the pixel array reading direction inverted in\nthe driver ?\n\n>\n> --\n> Kieran\n>\n> > >\n> > > > /*\n> > > > * Clear and remove any existing configuration from previous calls, and\n> > > > * ensure the required entries are available without further\n> > > > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > > > index 194ca3030..d304a1285 100644\n> > > > --- a/src/android/camera_device.h\n> > > > +++ b/src/android/camera_device.h\n> > > > @@ -126,6 +126,7 @@ private:\n> > > >\n> > > > int facing_;\n> > > > int orientation_;\n> > > > + int mirrored_;\n> > > >\n> > > > CameraMetadata lastSettings_;\n> > > > };\n> > > > diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp\n> > > > index 7ef451ef8..a2beba5d3 100644\n> > > > --- a/src/android/camera_hal_config.cpp\n> > > > +++ b/src/android/camera_hal_config.cpp\n> > > > @@ -33,6 +33,7 @@ private:\n> > > > int parseCameraConfigData(const std::string &cameraId, const YamlObject &);\n> > > > int parseLocation(const YamlObject &, CameraConfigData &cameraConfigData);\n> > > > int parseRotation(const YamlObject &, CameraConfigData &cameraConfigData);\n> > > > + int parseMirrored(const YamlObject &, CameraConfigData &cameraConfigData);\n> > > >\n> > > > std::map *cameras_;\n> > > > };\n> > > > @@ -106,6 +107,9 @@ int CameraHalConfig::Private::parseCameraConfigData(const std::string &cameraId,\n> > > > if (parseRotation(cameraObject, cameraConfigData))\n> > > > return -EINVAL;\n> > > >\n> > > > + /* Parse optional property \"mirrored\" */\n> > > > + parseMirrored(cameraObject, cameraConfigData);\n> > > > +\n> > > > return 0;\n> > > > }\n> > > >\n> > > > @@ -145,6 +149,16 @@ int CameraHalConfig::Private::parseRotation(const YamlObject &cameraObject,\n> > > > return 0;\n> > > > }\n> > > >\n> > > > +int CameraHalConfig::Private::parseMirrored(const YamlObject &cameraObject,\n> > > > + CameraConfigData &cameraConfigData)\n> > > > +{\n> > > > + if (!cameraObject.contains(\"mirrored\"))\n> > > > + return -EINVAL;\n> > > > +\n> > > > + cameraConfigData.mirrored = cameraObject[\"mirrored\"].get(false);\n> > > > + return 0;\n> > > > +}\n> > > > +\n> > > > CameraHalConfig::CameraHalConfig()\n> > > > : Extensible(std::make_unique()), exists_(false), valid_(false)\n> > > > {\n> > > > diff --git a/src/android/camera_hal_config.h b/src/android/camera_hal_config.h\n> > > > index a4bedb6e6..a96190470 100644\n> > > > --- a/src/android/camera_hal_config.h\n> > > > +++ b/src/android/camera_hal_config.h\n> > > > @@ -15,6 +15,7 @@\n> > > > struct CameraConfigData {\n> > > > int facing = -1;\n> > > > int rotation = -1;\n> > > > + bool mirrored = false;\n> > > > };\n> > > >\n> > > > class CameraHalConfig final : public libcamera::Extensible\n> > > > --\n> > > > 2.50.0.727.gbf7dc18ff4-goog\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 38A0BBDCC1\n\tfor ;\n\tMon, 21 Jul 2025 11:14:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9BE0068FE0;\n\tMon, 21 Jul 2025 13:14:14 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1270068FD0\n\tfor ;\n\tMon, 21 Jul 2025 13:14:13 +0200 (CEST)","from ideasonboard.com (mob-5-90-136-183.net.vodafone.it\n\t[5.90.136.183])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E436F8BF;\n\tMon, 21 Jul 2025 13:13:35 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"pb75f0A3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1753096416;\n\tbh=NyDPDzOaEZ2xLdPGrVc7ObgN1xb0/etvUKIChNCanBM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=pb75f0A3OP1EUHDF5lEHsHnQ2WVbYnUQbQAgXCXsZFIGalGxYs/XBrgOXOlwWYCvV\n\tkzqI3TZV9GmIiCZ1kHKflM2g/UrihkiPAFlzRqsdBNLMS3S6bxbR8PHTERHCu5q+W5\n\toD3qOOA9RrWf+0zCwYcv9Juua3TaEEkNSsVyzdrc=","Date":"Mon, 21 Jul 2025 13:14:08 +0200","From":"Jacopo Mondi ","To":"Kieran Bingham ","Cc":"Allen Ballway , \n\tlibcamera-devel@lists.libcamera.org, hanlinchen@chromium.org","Subject":"Re: [PATCH] libcamera: android: Add optional mirrored value for\n\tconfig","Message-ID":"<6tra7qb2ru2nt4cgzpfvtwhcccxjp2qmw52ii4rcizw474cppy@ngyrzgfdxjhy>","References":"<20250716204625.3221592-1-ballway@chromium.org>\n\t<175284070718.560048.16576681811250947472@ping.linuxembedded.co.uk>\n\t\n\t<175309465839.50296.11090024501206804447@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<175309465839.50296.11090024501206804447@ping.linuxembedded.co.uk>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":35028,"web_url":"https://patchwork.libcamera.org/comment/35028/","msgid":"","date":"2025-07-22T14:51:53","subject":"Re: [PATCH] libcamera: android: Add optional mirrored value for\n\tconfig","submitter":{"id":236,"url":"https://patchwork.libcamera.org/api/people/236/","name":"Allen Ballway","email":"ballway@chromium.org"},"content":"Hi all,\n\nOn Mon, Jul 21, 2025 at 4:14 AM Jacopo Mondi\n wrote:\n>\n> Hi Kieran\n>\n> On Mon, Jul 21, 2025 at 11:44:18AM +0100, Kieran Bingham wrote:\n> > Quoting Allen Ballway (2025-07-18 16:27:23)\n> > > On Fri, Jul 18, 2025 at 5:11 AM Kieran Bingham\n> > > wrote:\n> > > >\n> > > > Quoting Allen Ballway (2025-07-16 21:46:20)\n> > > > > Adds an optional `mirrored` value to the HAL config, which is used to modify\n> > > > > the `orientation` of the camera config. No rotation is used, so it can\n> > > > > only set the orientation to `Orientation::Rotate0Mirror`.\n> > > > >\n> > > > > This enables sensors which are incorrectly mirrored to be corrected.\n> > > > >\n> > > > > Signed-off-by: Allen Ballway \n> > > > > ---\n> > > > > src/android/camera_device.cpp | 6 ++++++\n> > > > > src/android/camera_device.h | 1 +\n> > > > > src/android/camera_hal_config.cpp | 14 ++++++++++++++\n> > > > > src/android/camera_hal_config.h | 1 +\n> > > > > 4 files changed, 22 insertions(+)\n> > > > >\n> > > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > > > index 80ff248c2..b20e9f3db 100644\n> > > > > --- a/src/android/camera_device.cpp\n> > > > > +++ b/src/android/camera_device.cpp\n> > > > > @@ -377,6 +377,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)\n> > > > > orientation_ = 0;\n> > > > > }\n> > > > >\n> > > > > + mirrored_ = cameraConfigData && cameraConfigData->mirrored;\n> > > > > return capabilities_.initialize(camera_, orientation_, facing_);\n> > > > > }\n> > > > >\n> > > > > @@ -547,6 +548,11 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > > > > return -EINVAL;\n> > > > > }\n> > > > >\n> > > > > + // Rotation is unsupported, so if mirrored just set orientation.\n> > > >\n> > > > Is that true? I thought we usually support 0 and 180 rotations on most\n> > > > sensors at least ?\n> > >\n> > > Ah I misunderstood the below comments/errors about not supporting stream\n> > > rotations. It seems the sensors on my test device don't support rotation. I will\n> > > send out a v2 with this comment removed assuming the rest of this patch\n> > > looks good.\n> > > >\n> > > > We (almost) always use /* */ comments in libcamera ... it's ... probably a\n> > > > stylistic choice because we are mostly started out as C / Linux-kernel\n> > > > devs.\n> > > >\n> > > >\n> > > >\n> > > > > + if (mirrored_) {\n> > > > > + config->orientation = Orientation::Rotate0Mirror;\n> > > > > + }\n> > > > > +\n>\n> Also no {} for single line statement.\n>\n> It's amusing how many times the same stylistic comments had to\n> repeated specifically to chromeos developers. We also have tools\n> available for checking this kind of issues before submitting.\n>\n\nApologies, I ran checkstyle.py on the change but found no issues, if there\nare other tools for finding style issues I'll be sure to run them in the future.\nI'll update this one as well in v2.\n> > > >\n> > > > How does this interact with rotation as well ? will it take precedence\n> > > > against 180 rotations?\n> > >\n> > > I can't confirm because my sensors aren't affected by the rotation config\n> > > but as far as I can tell the rotation would be unaffected by setting the\n> > > orientation to mirror. There doesn't seem to be anywhere setting the\n> > > orientation in the Android layer, so any rotation would happen independently.\n> > > And the mirroring will just set the HFLIP on the kernel driver, so any higher\n> > > level rotation would just rotate the flipped output as expected.\n> >\n> > Some level of rotation is supposed to be handled automatically by\n> > setting the physical orientation and rotation in device tree.\n> >\n> > Then libcamera will 'aim' for a 0 rotation by default.\n> >\n> > I guess the issue here is that we don't have a way in\n> > device-tree/firmware to mark that the camera is mirrored.\n> >\n> > what does this actually 'mean' by the way? How is the camera physically\n> > mirrored? Is this really a bug in the driver somewhere or something\n> > else?\n> >\n> > I am weary that this patch is possibly a workaround for a problem\n> > somewhere else ?\n>\n> If my recollection is right, when it comes to the camera HAL:\n>\n> 1) Rotation is first parsed from DT, but there we can only express\n> [0, 90, 270, 360] so we don't have a \"mirror\" option\n>\n> 2) If no \"rotation\" property is specified in DT, then the HAL\n> falls-back to parse a \"rotation\" property from configuration file,\n> where again we can only express an angle between [0 - 360]\n>\n> All of this seems to suggest it is not possible to express in DT or\n> HAL configuration file the \"mirroring\" option. However this opens the\n> question that Kieran has asked already. How is the camera module\n> physically mirrored ? is the pixel array reading direction inverted in\n> the driver ?\n>\n\nI'm not 100% sure where the mirroring is coming from. I confirmed it\nthrough qcam on Ubuntu as well as through the camera app and Chromium\non a ChromiumOs build so it's not just an application bug. The location and\norientation coming from ACPI both seem to be correct. The driver seems\nto be fine on all but some Surface Go devices so it seems unrelated.\nAnything below that is infeasible to fix and the sensor drivers don't\nmaintain quirks for particular misbehaving devices, so the HAL seems\nto me the best place to add a mirrored config and pipe it down to the\ndriver through the existing Orientation.\n\nThanks,\nAllen\n> >\n> > --\n> > Kieran\n> >\n> > > >\n> > > > > /*\n> > > > > * Clear and remove any existing configuration from previous calls, and\n> > > > > * ensure the required entries are available without further\n> > > > > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > > > > index 194ca3030..d304a1285 100644\n> > > > > --- a/src/android/camera_device.h\n> > > > > +++ b/src/android/camera_device.h\n> > > > > @@ -126,6 +126,7 @@ private:\n> > > > >\n> > > > > int facing_;\n> > > > > int orientation_;\n> > > > > + int mirrored_;\n> > > > >\n> > > > > CameraMetadata lastSettings_;\n> > > > > };\n> > > > > diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp\n> > > > > index 7ef451ef8..a2beba5d3 100644\n> > > > > --- a/src/android/camera_hal_config.cpp\n> > > > > +++ b/src/android/camera_hal_config.cpp\n> > > > > @@ -33,6 +33,7 @@ private:\n> > > > > int parseCameraConfigData(const std::string &cameraId, const YamlObject &);\n> > > > > int parseLocation(const YamlObject &, CameraConfigData &cameraConfigData);\n> > > > > int parseRotation(const YamlObject &, CameraConfigData &cameraConfigData);\n> > > > > + int parseMirrored(const YamlObject &, CameraConfigData &cameraConfigData);\n> > > > >\n> > > > > std::map *cameras_;\n> > > > > };\n> > > > > @@ -106,6 +107,9 @@ int CameraHalConfig::Private::parseCameraConfigData(const std::string &cameraId,\n> > > > > if (parseRotation(cameraObject, cameraConfigData))\n> > > > > return -EINVAL;\n> > > > >\n> > > > > + /* Parse optional property \"mirrored\" */\n> > > > > + parseMirrored(cameraObject, cameraConfigData);\n> > > > > +\n> > > > > return 0;\n> > > > > }\n> > > > >\n> > > > > @@ -145,6 +149,16 @@ int CameraHalConfig::Private::parseRotation(const YamlObject &cameraObject,\n> > > > > return 0;\n> > > > > }\n> > > > >\n> > > > > +int CameraHalConfig::Private::parseMirrored(const YamlObject &cameraObject,\n> > > > > + CameraConfigData &cameraConfigData)\n> > > > > +{\n> > > > > + if (!cameraObject.contains(\"mirrored\"))\n> > > > > + return -EINVAL;\n> > > > > +\n> > > > > + cameraConfigData.mirrored = cameraObject[\"mirrored\"].get(false);\n> > > > > + return 0;\n> > > > > +}\n> > > > > +\n> > > > > CameraHalConfig::CameraHalConfig()\n> > > > > : Extensible(std::make_unique()), exists_(false), valid_(false)\n> > > > > {\n> > > > > diff --git a/src/android/camera_hal_config.h b/src/android/camera_hal_config.h\n> > > > > index a4bedb6e6..a96190470 100644\n> > > > > --- a/src/android/camera_hal_config.h\n> > > > > +++ b/src/android/camera_hal_config.h\n> > > > > @@ -15,6 +15,7 @@\n> > > > > struct CameraConfigData {\n> > > > > int facing = -1;\n> > > > > int rotation = -1;\n> > > > > + bool mirrored = false;\n> > > > > };\n> > > > >\n> > > > > class CameraHalConfig final : public libcamera::Extensible\n> > > > > --\n> > > > > 2.50.0.727.gbf7dc18ff4-goog\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 B5173C3237\n\tfor ;\n\tTue, 22 Jul 2025 14:52:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D50396903C;\n\tTue, 22 Jul 2025 16:52:06 +0200 (CEST)","from mail-ed1-x52a.google.com (mail-ed1-x52a.google.com\n\t[IPv6:2a00:1450:4864:20::52a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DFD4368F93\n\tfor ;\n\tTue, 22 Jul 2025 16:52:04 +0200 (CEST)","by mail-ed1-x52a.google.com with SMTP id\n\t4fb4d7f45d1cf-60bf5a08729so10661306a12.0\n\tfor ;\n\tTue, 22 Jul 2025 07:52:04 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"dyMfo/Hc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1753195924; x=1753800724;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=aAhvIeKTB+MjH1X8xiqfFcPy5RruoHA+o1L1vfvBG8Y=;\n\tb=dyMfo/Hc2qsm0aCiNzn5/pfbEnblLxIfVyO4a4vMoGuG8Lvlj6am60syaT/J07LqlN\n\tqPOfAPpjcGPumlWXmgT0S1PpVV9HTnTM8gqOQzkKpt9mlXDo2vOse6fZi8sc/6T2Mlg8\n\tcTv+/jnYuVf97xafZfhIsgzAgWbVd/vGdgtvc=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1753195924; x=1753800724;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=aAhvIeKTB+MjH1X8xiqfFcPy5RruoHA+o1L1vfvBG8Y=;\n\tb=LioFNwV/ELwIEoI0oyU97k4Qed3UaIIRqAaoiyOXCX709wpFL1Edtg5QJHgUHsWHTi\n\tG6jOoYknZZgF9f2W7jOK3y5C1TQb8roUGsaz83HYbQPc8RmjCPiTTQmjoFvY9IQnfdaX\n\tD9B2hwby340w4CJdsMSeckHH8jA5csDzNMHEsrQ8ToqBL0TgOTmQ8FEt/H0Sqy9N5sV7\n\tTJbiIKyf4C8TIPz33rXbtuo2bL/sXO71tiJJv8p93vICgiSq+CpYbz90k927lGpWB/ga\n\tXZa/JujSykospOm1kz0aAyT7VTiV64xBemW5vSq6I9v2lFZ7R0fvAeAZLt83TMaYIpIA\n\tLATQ==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCVa+nvcclyKq3Qxp7uvc74Bv6oK1ZZQs30SK/pf4c6SrK+po1OVPysD74aFC5vP6F9/wrQC5x1Ixg4IJWQ1aOU=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YzUaFOajDCTQvGXYSQ0ZtOOaU54I/X5HoSAFct1ee3L5+4sJsAF\n\tK1Z+GArC0b00o40jQncfhdEgXEr7I8NHVmtQxdj73/4Upcp+YxNvyvGHK7B8rLI+JI+M7IOc5G7\n\tVuy9aQe00XzZrjgFW8XMzlHKy4y4rhBd9f+zDVFCjaDPHglncc4lzyg==","X-Gm-Gg":"ASbGncswwJtRBV1txx26hVv0N8gf1yH48QoG0RCKjrfQkiubhL0qd8A4h6eIyFsIkmz\n\t7VAv3HrR2RKMEsrQnKWJEMb/CL5kRpSmgDUpnDc5lw+eKacnpYaBsimkyo6ibVmzizVIRmWYZpi\n\tQP0rbp0oIwusgxEeVCR++CtSy4bT1hmcT4PiJBTAvwme/KvPdHp6DZaLYWqVxlj42nL7TcDeVlL\n\tZ4y6z8=","X-Google-Smtp-Source":"AGHT+IHRfOFawWoiWZq46Jh5M2tv+CdxYyEI6RfHwquuzq15SkltFkvXfJ316LzJ31HdMj5Bw7C1WwlIb1rkz5Slyq8=","X-Received":"by 2002:a17:907:d78b:b0:aec:4aa5:2f60 with SMTP id\n\ta640c23a62f3a-aec4aa52fd2mr2327943066b.3.1753195924059;\n\tTue, 22 Jul 2025 07:52:04 -0700 (PDT)","MIME-Version":"1.0","References":"<20250716204625.3221592-1-ballway@chromium.org>\n\t<175284070718.560048.16576681811250947472@ping.linuxembedded.co.uk>\n\t\n\t<175309465839.50296.11090024501206804447@ping.linuxembedded.co.uk>\n\t<6tra7qb2ru2nt4cgzpfvtwhcccxjp2qmw52ii4rcizw474cppy@ngyrzgfdxjhy>","In-Reply-To":"<6tra7qb2ru2nt4cgzpfvtwhcccxjp2qmw52ii4rcizw474cppy@ngyrzgfdxjhy>","From":"Allen Ballway ","Date":"Tue, 22 Jul 2025 07:51:53 -0700","X-Gm-Features":"Ac12FXy2UadVDgG2DUmfCu2tYJTQz7Ne6wO7oGrgnDDNMdfh7jEEgDGOhLspWcc","Message-ID":"","Subject":"Re: [PATCH] libcamera: android: Add optional mirrored value for\n\tconfig","To":"Jacopo Mondi ","Cc":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org, hanlinchen@chromium.org","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":35037,"web_url":"https://patchwork.libcamera.org/comment/35037/","msgid":"","date":"2025-07-23T08:15:24","subject":"Re: [PATCH] libcamera: android: Add optional mirrored value for\n\tconfig","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Allen\n\nOn Tue, Jul 22, 2025 at 07:51:53AM -0700, Allen Ballway wrote:\n> Hi all,\n>\n> On Mon, Jul 21, 2025 at 4:14 AM Jacopo Mondi\n> wrote:\n> >\n> > Hi Kieran\n> >\n> > On Mon, Jul 21, 2025 at 11:44:18AM +0100, Kieran Bingham wrote:\n> > > Quoting Allen Ballway (2025-07-18 16:27:23)\n> > > > On Fri, Jul 18, 2025 at 5:11 AM Kieran Bingham\n> > > > wrote:\n> > > > >\n> > > > > Quoting Allen Ballway (2025-07-16 21:46:20)\n> > > > > > Adds an optional `mirrored` value to the HAL config, which is used to modify\n> > > > > > the `orientation` of the camera config. No rotation is used, so it can\n> > > > > > only set the orientation to `Orientation::Rotate0Mirror`.\n> > > > > >\n> > > > > > This enables sensors which are incorrectly mirrored to be corrected.\n> > > > > >\n> > > > > > Signed-off-by: Allen Ballway \n> > > > > > ---\n> > > > > > src/android/camera_device.cpp | 6 ++++++\n> > > > > > src/android/camera_device.h | 1 +\n> > > > > > src/android/camera_hal_config.cpp | 14 ++++++++++++++\n> > > > > > src/android/camera_hal_config.h | 1 +\n> > > > > > 4 files changed, 22 insertions(+)\n> > > > > >\n> > > > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > > > > index 80ff248c2..b20e9f3db 100644\n> > > > > > --- a/src/android/camera_device.cpp\n> > > > > > +++ b/src/android/camera_device.cpp\n> > > > > > @@ -377,6 +377,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)\n> > > > > > orientation_ = 0;\n> > > > > > }\n> > > > > >\n> > > > > > + mirrored_ = cameraConfigData && cameraConfigData->mirrored;\n> > > > > > return capabilities_.initialize(camera_, orientation_, facing_);\n> > > > > > }\n> > > > > >\n> > > > > > @@ -547,6 +548,11 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > > > > > return -EINVAL;\n> > > > > > }\n> > > > > >\n> > > > > > + // Rotation is unsupported, so if mirrored just set orientation.\n> > > > >\n> > > > > Is that true? I thought we usually support 0 and 180 rotations on most\n> > > > > sensors at least ?\n> > > >\n> > > > Ah I misunderstood the below comments/errors about not supporting stream\n> > > > rotations. It seems the sensors on my test device don't support rotation. I will\n> > > > send out a v2 with this comment removed assuming the rest of this patch\n> > > > looks good.\n> > > > >\n> > > > > We (almost) always use /* */ comments in libcamera ... it's ... probably a\n> > > > > stylistic choice because we are mostly started out as C / Linux-kernel\n> > > > > devs.\n> > > > >\n> > > > >\n> > > > >\n> > > > > > + if (mirrored_) {\n> > > > > > + config->orientation = Orientation::Rotate0Mirror;\n> > > > > > + }\n> > > > > > +\n> >\n> > Also no {} for single line statement.\n> >\n> > It's amusing how many times the same stylistic comments had to\n> > repeated specifically to chromeos developers. We also have tools\n> > available for checking this kind of issues before submitting.\n> >\n>\n> Apologies, I ran checkstyle.py on the change but found no issues, if there\n> are other tools for finding style issues I'll be sure to run them in the future.\n> I'll update this one as well in v2.\n\nThis is one of your first contributions, so it has been unfair ranting\nto you about the past :)\n\nI'm just suprised a team of specialized and highly skilled engineers\ncontinue pushing their conventions (out of habit I presume) to a different\ncode base. My frustration comes from having repeated the same comments\nover and over in the past years. Apologies if you got ranted to.\n\n> > > > >\n> > > > > How does this interact with rotation as well ? will it take precedence\n> > > > > against 180 rotations?\n> > > >\n> > > > I can't confirm because my sensors aren't affected by the rotation config\n> > > > but as far as I can tell the rotation would be unaffected by setting the\n> > > > orientation to mirror. There doesn't seem to be anywhere setting the\n> > > > orientation in the Android layer, so any rotation would happen independently.\n> > > > And the mirroring will just set the HFLIP on the kernel driver, so any higher\n> > > > level rotation would just rotate the flipped output as expected.\n> > >\n> > > Some level of rotation is supposed to be handled automatically by\n> > > setting the physical orientation and rotation in device tree.\n> > >\n> > > Then libcamera will 'aim' for a 0 rotation by default.\n> > >\n> > > I guess the issue here is that we don't have a way in\n> > > device-tree/firmware to mark that the camera is mirrored.\n> > >\n> > > what does this actually 'mean' by the way? How is the camera physically\n> > > mirrored? Is this really a bug in the driver somewhere or something\n> > > else?\n> > >\n> > > I am weary that this patch is possibly a workaround for a problem\n> > > somewhere else ?\n> >\n> > If my recollection is right, when it comes to the camera HAL:\n> >\n> > 1) Rotation is first parsed from DT, but there we can only express\n> > [0, 90, 270, 360] so we don't have a \"mirror\" option\n> >\n> > 2) If no \"rotation\" property is specified in DT, then the HAL\n> > falls-back to parse a \"rotation\" property from configuration file,\n> > where again we can only express an angle between [0 - 360]\n> >\n> > All of this seems to suggest it is not possible to express in DT or\n> > HAL configuration file the \"mirroring\" option. However this opens the\n> > question that Kieran has asked already. How is the camera module\n> > physically mirrored ? is the pixel array reading direction inverted in\n> > the driver ?\n> >\n>\n> I'm not 100% sure where the mirroring is coming from. I confirmed it\n> through qcam on Ubuntu as well as through the camera app and Chromium\n> on a ChromiumOs build so it's not just an application bug. The location and\n> orientation coming from ACPI both seem to be correct. The driver seems\n\nThe 'rotation' property from DTS cannot express 'mirrored', and to me\nthis seems correct, as 'rotation' is about expressing the mounting\nrotation of the camera sensor (0, 90, 180, 270)\n\nMirroring comes from inverting the reading directions of the pixel\nunits on the pixel array, what is usually called an HFLIP in v4l2\ndrivers and we require drivers to not apply any V/HFLIP by default\n(something we might want to document in libcamera as well, not just in\nLinux).\n\n> to be fine on all but some Surface Go devices so it seems unrelated.\n\nWhat do you mean with \"seems fine\" ? It doesn't mirror ?\n\n> Anything below that is infeasible to fix and the sensor drivers don't\n> maintain quirks for particular misbehaving devices, so the HAL seems\n> to me the best place to add a mirrored config and pipe it down to the\n> driver through the existing Orientation.\n\nWhich sensor are we talking about and where does its driver live ? Can\nwe see it ? Because some downstream (but also upstream) drivers\nsometimes flip by default.\n\n\n>\n> Thanks,\n> Allen\n> > >\n> > > --\n> > > Kieran\n> > >\n> > > > >\n> > > > > > /*\n> > > > > > * Clear and remove any existing configuration from previous calls, and\n> > > > > > * ensure the required entries are available without further\n> > > > > > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > > > > > index 194ca3030..d304a1285 100644\n> > > > > > --- a/src/android/camera_device.h\n> > > > > > +++ b/src/android/camera_device.h\n> > > > > > @@ -126,6 +126,7 @@ private:\n> > > > > >\n> > > > > > int facing_;\n> > > > > > int orientation_;\n> > > > > > + int mirrored_;\n> > > > > >\n> > > > > > CameraMetadata lastSettings_;\n> > > > > > };\n> > > > > > diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp\n> > > > > > index 7ef451ef8..a2beba5d3 100644\n> > > > > > --- a/src/android/camera_hal_config.cpp\n> > > > > > +++ b/src/android/camera_hal_config.cpp\n> > > > > > @@ -33,6 +33,7 @@ private:\n> > > > > > int parseCameraConfigData(const std::string &cameraId, const YamlObject &);\n> > > > > > int parseLocation(const YamlObject &, CameraConfigData &cameraConfigData);\n> > > > > > int parseRotation(const YamlObject &, CameraConfigData &cameraConfigData);\n> > > > > > + int parseMirrored(const YamlObject &, CameraConfigData &cameraConfigData);\n> > > > > >\n> > > > > > std::map *cameras_;\n> > > > > > };\n> > > > > > @@ -106,6 +107,9 @@ int CameraHalConfig::Private::parseCameraConfigData(const std::string &cameraId,\n> > > > > > if (parseRotation(cameraObject, cameraConfigData))\n> > > > > > return -EINVAL;\n> > > > > >\n> > > > > > + /* Parse optional property \"mirrored\" */\n> > > > > > + parseMirrored(cameraObject, cameraConfigData);\n> > > > > > +\n> > > > > > return 0;\n> > > > > > }\n> > > > > >\n> > > > > > @@ -145,6 +149,16 @@ int CameraHalConfig::Private::parseRotation(const YamlObject &cameraObject,\n> > > > > > return 0;\n> > > > > > }\n> > > > > >\n> > > > > > +int CameraHalConfig::Private::parseMirrored(const YamlObject &cameraObject,\n> > > > > > + CameraConfigData &cameraConfigData)\n> > > > > > +{\n> > > > > > + if (!cameraObject.contains(\"mirrored\"))\n> > > > > > + return -EINVAL;\n> > > > > > +\n> > > > > > + cameraConfigData.mirrored = cameraObject[\"mirrored\"].get(false);\n> > > > > > + return 0;\n> > > > > > +}\n> > > > > > +\n> > > > > > CameraHalConfig::CameraHalConfig()\n> > > > > > : Extensible(std::make_unique()), exists_(false), valid_(false)\n> > > > > > {\n> > > > > > diff --git a/src/android/camera_hal_config.h b/src/android/camera_hal_config.h\n> > > > > > index a4bedb6e6..a96190470 100644\n> > > > > > --- a/src/android/camera_hal_config.h\n> > > > > > +++ b/src/android/camera_hal_config.h\n> > > > > > @@ -15,6 +15,7 @@\n> > > > > > struct CameraConfigData {\n> > > > > > int facing = -1;\n> > > > > > int rotation = -1;\n> > > > > > + bool mirrored = false;\n> > > > > > };\n> > > > > >\n> > > > > > class CameraHalConfig final : public libcamera::Extensible\n> > > > > > --\n> > > > > > 2.50.0.727.gbf7dc18ff4-goog\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 5950FC3237\n\tfor ;\n\tWed, 23 Jul 2025 08:15:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6C1C069049;\n\tWed, 23 Jul 2025 10:15:32 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 54C6C68FB6\n\tfor ;\n\tWed, 23 Jul 2025 10:15:30 +0200 (CEST)","from ideasonboard.com (93-46-82-201.ip106.fastwebnet.it\n\t[93.46.82.201])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 86C10C78;\n\tWed, 23 Jul 2025 10:14:51 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"RgpsltJy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1753258491;\n\tbh=TDLg90M2qLhr+vulqrZSdgiRUbk/c7yxXwBceI3NS50=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=RgpsltJyQS3PAyea/9kTUat8HnDFeRfAybhSUcsvjWWEs6TU0bD954S0bsNPuNLEs\n\tDYsOr9MBaTWyU84IUlwpc3lkIWRVW01Yd3z8HpaGasPSHjWWHH/lc3MKDOw26EU4Xd\n\tASLv+w+lkgZDVYGgyKrN8EUKwEnQVlnK3C0PRY88=","Date":"Wed, 23 Jul 2025 10:15:24 +0200","From":"Jacopo Mondi ","To":"Allen Ballway ","Cc":"Jacopo Mondi , \n\tKieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org, hanlinchen@chromium.org","Subject":"Re: [PATCH] libcamera: android: Add optional mirrored value for\n\tconfig","Message-ID":"","References":"<20250716204625.3221592-1-ballway@chromium.org>\n\t<175284070718.560048.16576681811250947472@ping.linuxembedded.co.uk>\n\t\n\t<175309465839.50296.11090024501206804447@ping.linuxembedded.co.uk>\n\t<6tra7qb2ru2nt4cgzpfvtwhcccxjp2qmw52ii4rcizw474cppy@ngyrzgfdxjhy>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":35042,"web_url":"https://patchwork.libcamera.org/comment/35042/","msgid":"","date":"2025-07-23T15:14:45","subject":"Re: [PATCH] libcamera: android: Add optional mirrored value for\n\tconfig","submitter":{"id":236,"url":"https://patchwork.libcamera.org/api/people/236/","name":"Allen Ballway","email":"ballway@chromium.org"},"content":"Hi Jacopo,\n\nOn Wed, Jul 23, 2025 at 1:15 AM Jacopo Mondi\n wrote:\n>\n> Hi Allen\n>\n> On Tue, Jul 22, 2025 at 07:51:53AM -0700, Allen Ballway wrote:\n> > Hi all,\n> >\n> > On Mon, Jul 21, 2025 at 4:14 AM Jacopo Mondi\n> > wrote:\n> > >\n> > > Hi Kieran\n> > >\n> > > On Mon, Jul 21, 2025 at 11:44:18AM +0100, Kieran Bingham wrote:\n> > > > Quoting Allen Ballway (2025-07-18 16:27:23)\n> > > > > On Fri, Jul 18, 2025 at 5:11 AM Kieran Bingham\n> > > > > wrote:\n> > > > > >\n> > > > > > Quoting Allen Ballway (2025-07-16 21:46:20)\n> > > > > > > Adds an optional `mirrored` value to the HAL config, which is used to modify\n> > > > > > > the `orientation` of the camera config. No rotation is used, so it can\n> > > > > > > only set the orientation to `Orientation::Rotate0Mirror`.\n> > > > > > >\n> > > > > > > This enables sensors which are incorrectly mirrored to be corrected.\n> > > > > > >\n> > > > > > > Signed-off-by: Allen Ballway \n> > > > > > > ---\n> > > > > > > src/android/camera_device.cpp | 6 ++++++\n> > > > > > > src/android/camera_device.h | 1 +\n> > > > > > > src/android/camera_hal_config.cpp | 14 ++++++++++++++\n> > > > > > > src/android/camera_hal_config.h | 1 +\n> > > > > > > 4 files changed, 22 insertions(+)\n> > > > > > >\n> > > > > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > > > > > index 80ff248c2..b20e9f3db 100644\n> > > > > > > --- a/src/android/camera_device.cpp\n> > > > > > > +++ b/src/android/camera_device.cpp\n> > > > > > > @@ -377,6 +377,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)\n> > > > > > > orientation_ = 0;\n> > > > > > > }\n> > > > > > >\n> > > > > > > + mirrored_ = cameraConfigData && cameraConfigData->mirrored;\n> > > > > > > return capabilities_.initialize(camera_, orientation_, facing_);\n> > > > > > > }\n> > > > > > >\n> > > > > > > @@ -547,6 +548,11 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > > > > > > return -EINVAL;\n> > > > > > > }\n> > > > > > >\n> > > > > > > + // Rotation is unsupported, so if mirrored just set orientation.\n> > > > > >\n> > > > > > Is that true? I thought we usually support 0 and 180 rotations on most\n> > > > > > sensors at least ?\n> > > > >\n> > > > > Ah I misunderstood the below comments/errors about not supporting stream\n> > > > > rotations. It seems the sensors on my test device don't support rotation. I will\n> > > > > send out a v2 with this comment removed assuming the rest of this patch\n> > > > > looks good.\n> > > > > >\n> > > > > > We (almost) always use /* */ comments in libcamera ... it's ... probably a\n> > > > > > stylistic choice because we are mostly started out as C / Linux-kernel\n> > > > > > devs.\n> > > > > >\n> > > > > >\n> > > > > >\n> > > > > > > + if (mirrored_) {\n> > > > > > > + config->orientation = Orientation::Rotate0Mirror;\n> > > > > > > + }\n> > > > > > > +\n> > >\n> > > Also no {} for single line statement.\n> > >\n> > > It's amusing how many times the same stylistic comments had to\n> > > repeated specifically to chromeos developers. We also have tools\n> > > available for checking this kind of issues before submitting.\n> > >\n> >\n> > Apologies, I ran checkstyle.py on the change but found no issues, if there\n> > are other tools for finding style issues I'll be sure to run them in the future.\n> > I'll update this one as well in v2.\n>\n> This is one of your first contributions, so it has been unfair ranting\n> to you about the past :)\n>\n> I'm just suprised a team of specialized and highly skilled engineers\n> continue pushing their conventions (out of habit I presume) to a different\n> code base. My frustration comes from having repeated the same comments\n> over and over in the past years. Apologies if you got ranted to.\n>\n\nSorry to have added to the frustration, and no worries :)\n> > > > > >\n> > > > > > How does this interact with rotation as well ? will it take precedence\n> > > > > > against 180 rotations?\n> > > > >\n> > > > > I can't confirm because my sensors aren't affected by the rotation config\n> > > > > but as far as I can tell the rotation would be unaffected by setting the\n> > > > > orientation to mirror. There doesn't seem to be anywhere setting the\n> > > > > orientation in the Android layer, so any rotation would happen independently.\n> > > > > And the mirroring will just set the HFLIP on the kernel driver, so any higher\n> > > > > level rotation would just rotate the flipped output as expected.\n> > > >\n> > > > Some level of rotation is supposed to be handled automatically by\n> > > > setting the physical orientation and rotation in device tree.\n> > > >\n> > > > Then libcamera will 'aim' for a 0 rotation by default.\n> > > >\n> > > > I guess the issue here is that we don't have a way in\n> > > > device-tree/firmware to mark that the camera is mirrored.\n> > > >\n> > > > what does this actually 'mean' by the way? How is the camera physically\n> > > > mirrored? Is this really a bug in the driver somewhere or something\n> > > > else?\n> > > >\n> > > > I am weary that this patch is possibly a workaround for a problem\n> > > > somewhere else ?\n> > >\n> > > If my recollection is right, when it comes to the camera HAL:\n> > >\n> > > 1) Rotation is first parsed from DT, but there we can only express\n> > > [0, 90, 270, 360] so we don't have a \"mirror\" option\n> > >\n> > > 2) If no \"rotation\" property is specified in DT, then the HAL\n> > > falls-back to parse a \"rotation\" property from configuration file,\n> > > where again we can only express an angle between [0 - 360]\n> > >\n> > > All of this seems to suggest it is not possible to express in DT or\n> > > HAL configuration file the \"mirroring\" option. However this opens the\n> > > question that Kieran has asked already. How is the camera module\n> > > physically mirrored ? is the pixel array reading direction inverted in\n> > > the driver ?\n> > >\n> >\n> > I'm not 100% sure where the mirroring is coming from. I confirmed it\n> > through qcam on Ubuntu as well as through the camera app and Chromium\n> > on a ChromiumOs build so it's not just an application bug. The location and\n> > orientation coming from ACPI both seem to be correct. The driver seems\n>\n> The 'rotation' property from DTS cannot express 'mirrored', and to me\n> this seems correct, as 'rotation' is about expressing the mounting\n> rotation of the camera sensor (0, 90, 180, 270)\n>\n> Mirroring comes from inverting the reading directions of the pixel\n> units on the pixel array, what is usually called an HFLIP in v4l2\n> drivers and we require drivers to not apply any V/HFLIP by default\n> (something we might want to document in libcamera as well, not just in\n> Linux).\n>\n> > to be fine on all but some Surface Go devices so it seems unrelated.\n>\n> What do you mean with \"seems fine\" ? It doesn't mirror ?\n>\n\n\"Seems fine\" as in I don't have a device to test and confirm that it actually\nworks as expected and isn't mirrored, but the only references I find to\nmirrored output from the sensor are for the Surface devices, and I would\nexpect that to be a prominent enough issue that it would be raised\nsomewhere.\n> > Anything below that is infeasible to fix and the sensor drivers don't\n> > maintain quirks for particular misbehaving devices, so the HAL seems\n> > to me the best place to add a mirrored config and pipe it down to the\n> > driver through the existing Orientation.\n>\n> Which sensor are we talking about and where does its driver live ? Can\n> we see it ? Because some downstream (but also upstream) drivers\n> sometimes flip by default.\n>\nThis is the ov8865 sensor, driver is drivers/media/i2c/ov8865.c in the\nupstream kernel. I confirmedthat the default state is unflipped, and\nactually have a separate patch out (https://lkml.org/lkml/2025/7/22/1443)\nbecause there's a bug in the driver which ends up resetting the flip\nstates back to default after we set it.\n\nThanks,\nAllen\n>\n> >\n> > Thanks,\n> > Allen\n> > > >\n> > > > --\n> > > > Kieran\n> > > >\n> > > > > >\n> > > > > > > /*\n> > > > > > > * Clear and remove any existing configuration from previous calls, and\n> > > > > > > * ensure the required entries are available without further\n> > > > > > > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > > > > > > index 194ca3030..d304a1285 100644\n> > > > > > > --- a/src/android/camera_device.h\n> > > > > > > +++ b/src/android/camera_device.h\n> > > > > > > @@ -126,6 +126,7 @@ private:\n> > > > > > >\n> > > > > > > int facing_;\n> > > > > > > int orientation_;\n> > > > > > > + int mirrored_;\n> > > > > > >\n> > > > > > > CameraMetadata lastSettings_;\n> > > > > > > };\n> > > > > > > diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp\n> > > > > > > index 7ef451ef8..a2beba5d3 100644\n> > > > > > > --- a/src/android/camera_hal_config.cpp\n> > > > > > > +++ b/src/android/camera_hal_config.cpp\n> > > > > > > @@ -33,6 +33,7 @@ private:\n> > > > > > > int parseCameraConfigData(const std::string &cameraId, const YamlObject &);\n> > > > > > > int parseLocation(const YamlObject &, CameraConfigData &cameraConfigData);\n> > > > > > > int parseRotation(const YamlObject &, CameraConfigData &cameraConfigData);\n> > > > > > > + int parseMirrored(const YamlObject &, CameraConfigData &cameraConfigData);\n> > > > > > >\n> > > > > > > std::map *cameras_;\n> > > > > > > };\n> > > > > > > @@ -106,6 +107,9 @@ int CameraHalConfig::Private::parseCameraConfigData(const std::string &cameraId,\n> > > > > > > if (parseRotation(cameraObject, cameraConfigData))\n> > > > > > > return -EINVAL;\n> > > > > > >\n> > > > > > > + /* Parse optional property \"mirrored\" */\n> > > > > > > + parseMirrored(cameraObject, cameraConfigData);\n> > > > > > > +\n> > > > > > > return 0;\n> > > > > > > }\n> > > > > > >\n> > > > > > > @@ -145,6 +149,16 @@ int CameraHalConfig::Private::parseRotation(const YamlObject &cameraObject,\n> > > > > > > return 0;\n> > > > > > > }\n> > > > > > >\n> > > > > > > +int CameraHalConfig::Private::parseMirrored(const YamlObject &cameraObject,\n> > > > > > > + CameraConfigData &cameraConfigData)\n> > > > > > > +{\n> > > > > > > + if (!cameraObject.contains(\"mirrored\"))\n> > > > > > > + return -EINVAL;\n> > > > > > > +\n> > > > > > > + cameraConfigData.mirrored = cameraObject[\"mirrored\"].get(false);\n> > > > > > > + return 0;\n> > > > > > > +}\n> > > > > > > +\n> > > > > > > CameraHalConfig::CameraHalConfig()\n> > > > > > > : Extensible(std::make_unique()), exists_(false), valid_(false)\n> > > > > > > {\n> > > > > > > diff --git a/src/android/camera_hal_config.h b/src/android/camera_hal_config.h\n> > > > > > > index a4bedb6e6..a96190470 100644\n> > > > > > > --- a/src/android/camera_hal_config.h\n> > > > > > > +++ b/src/android/camera_hal_config.h\n> > > > > > > @@ -15,6 +15,7 @@\n> > > > > > > struct CameraConfigData {\n> > > > > > > int facing = -1;\n> > > > > > > int rotation = -1;\n> > > > > > > + bool mirrored = false;\n> > > > > > > };\n> > > > > > >\n> > > > > > > class CameraHalConfig final : public libcamera::Extensible\n> > > > > > > --\n> > > > > > > 2.50.0.727.gbf7dc18ff4-goog\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 13E57BDCC1\n\tfor ;\n\tWed, 23 Jul 2025 15:15:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1A96F69054;\n\tWed, 23 Jul 2025 17:15:00 +0200 (CEST)","from mail-ej1-x632.google.com (mail-ej1-x632.google.com\n\t[IPv6:2a00:1450:4864:20::632])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6A635614ED\n\tfor ;\n\tWed, 23 Jul 2025 17:14:57 +0200 (CEST)","by mail-ej1-x632.google.com with SMTP id\n\ta640c23a62f3a-ae9be1697easo204554066b.1\n\tfor ;\n\tWed, 23 Jul 2025 08:14:57 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"SliCSH/3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1753283697; x=1753888497;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=A4KfWmGo4uPRTAoexUSxG227eY99tjCn8HHcy8I8kJk=;\n\tb=SliCSH/3J2zzMN8xOupnKq8iztGFMQrXpOEsxywOG6hpF+9kdSJpyHU+D2McehtTc8\n\trIHlLzju+0ruKCz0PgSNIiCrNlJlIZ6bYLGtFfmtqPmEi6IUZmJxSief7BmTOBpLziH0\n\tOc0uh1C4G9TGT/sa8S3ZDtu2FL10v/GHWkQaY=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1753283697; x=1753888497;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=A4KfWmGo4uPRTAoexUSxG227eY99tjCn8HHcy8I8kJk=;\n\tb=chM5+mmP0lMp5LTdCp5YMkz+1fy2RZGPGwpiFjqIcpzVVI9zirbmwep9Is9XGgkgZt\n\t4YHgDI88thOfuQGMraxSSQkurPKiBzbl5HcM8DmjxaTYeJ3JnWEssDDR9AdOrZpTGpvZ\n\tkI9eLb/inPNGz+iNi75DrUkn63KM5LUscP6+tdGRQGikPRzqnB4o4CsFvNBCC5Hrxfn/\n\tELYO4awfLpg+glf9jcnKBRxttggrM092Itf6OZ5eDBit2dOUvWP/o6YKEE+1YU7d3Ij7\n\tj7UdIJDtiRZaB+ZLr5MLeckP34PFkXn5JbxmYzku2Ek9GeLn/h+Sq0RYaCTIHdGHZ88F\n\tWhjg==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCVq+C2MqVJNV9FYMTOAm+B8TqFshlLY2xJNw3T0liQur8g/Kyfwu7stPOlVdbf1gL3kZPOl4q8rUGGGtTEytiw=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YzLD/M3HZ3WtSrzlbczw+ZpVQZgQUNlO1w4beZKlaJP5Iu2Zci7\n\tLHbSXBQkbvM+GTbbF9/LvE4dXdLCOmY0wQyo+RGLAlXKltS50P4zfm01c9aR9f6kjvw7MSMeYCj\n\trZAs98oTe4y96zXW1icybWzmtnHn3I+tthmGJaEe9","X-Gm-Gg":"ASbGncuQxkn+MmiF1AE9Iqok1XWQRrUMdthTRfhp9C/pHMszYu5TpygBg9dUK702fGy\n\tE4eidb18bDPbYn63brPngE6l4DdZrw1DW91vIiWuU2qYFJbX/5xfTxlscmIkojx6HgQmLdfsisA\n\t8dF480ZqQeKyJdI4Wae5fFRt53J+Q+sY3S5+jt5G0adoUbWuEh37iVIyYJ62MgwyL4i8IvYcd+Q\n\tKD2j6w=","X-Google-Smtp-Source":"AGHT+IH61XS4/KqqcST5zWxw2ecV3ES2i1n19ZfxefKTesWtjqusmYfxc3bpFA5/wQuMDm4Eb619uwkG2dev7m+lMf0=","X-Received":"by 2002:a17:907:a088:b0:ae3:eab4:21ed with SMTP id\n\ta640c23a62f3a-af152bfa375mr764840566b.11.1753283696690;\n\tWed, 23 Jul 2025 08:14:56 -0700 (PDT)","MIME-Version":"1.0","References":"<20250716204625.3221592-1-ballway@chromium.org>\n\t<175284070718.560048.16576681811250947472@ping.linuxembedded.co.uk>\n\t\n\t<175309465839.50296.11090024501206804447@ping.linuxembedded.co.uk>\n\t<6tra7qb2ru2nt4cgzpfvtwhcccxjp2qmw52ii4rcizw474cppy@ngyrzgfdxjhy>\n\t\n\t","In-Reply-To":"","From":"Allen Ballway ","Date":"Wed, 23 Jul 2025 08:14:45 -0700","X-Gm-Features":"Ac12FXyLquUhSz-jRwlql0PgsCzjiG98PkeAduObn9_U94nkeXXfGEGQESO0-dc","Message-ID":"","Subject":"Re: [PATCH] libcamera: android: Add optional mirrored value for\n\tconfig","To":"Jacopo Mondi ","Cc":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org, hanlinchen@chromium.org","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":35043,"web_url":"https://patchwork.libcamera.org/comment/35043/","msgid":"<20250723154314.GG6719@pendragon.ideasonboard.com>","date":"2025-07-23T15:43:14","subject":"Re: [PATCH] libcamera: android: Add optional mirrored value for\n\tconfig","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Jul 23, 2025 at 08:14:45AM -0700, Allen Ballway wrote:\n> On Wed, Jul 23, 2025 at 1:15 AM Jacopo Mondi wrote:\n> > On Tue, Jul 22, 2025 at 07:51:53AM -0700, Allen Ballway wrote:\n> > > On Mon, Jul 21, 2025 at 4:14 AM Jacopo Mondi wrote:\n> > > > On Mon, Jul 21, 2025 at 11:44:18AM +0100, Kieran Bingham wrote:\n> > > > > Quoting Allen Ballway (2025-07-18 16:27:23)\n> > > > > > On Fri, Jul 18, 2025 at 5:11 AM Kieran Bingham wrote:\n> > > > > > > Quoting Allen Ballway (2025-07-16 21:46:20)\n> > > > > > > > Adds an optional `mirrored` value to the HAL config, which is used to modify\n> > > > > > > > the `orientation` of the camera config. No rotation is used, so it can\n> > > > > > > > only set the orientation to `Orientation::Rotate0Mirror`.\n> > > > > > > >\n> > > > > > > > This enables sensors which are incorrectly mirrored to be corrected.\n> > > > > > > >\n> > > > > > > > Signed-off-by: Allen Ballway \n> > > > > > > > ---\n> > > > > > > > src/android/camera_device.cpp | 6 ++++++\n> > > > > > > > src/android/camera_device.h | 1 +\n> > > > > > > > src/android/camera_hal_config.cpp | 14 ++++++++++++++\n> > > > > > > > src/android/camera_hal_config.h | 1 +\n> > > > > > > > 4 files changed, 22 insertions(+)\n> > > > > > > >\n> > > > > > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > > > > > > index 80ff248c2..b20e9f3db 100644\n> > > > > > > > --- a/src/android/camera_device.cpp\n> > > > > > > > +++ b/src/android/camera_device.cpp\n> > > > > > > > @@ -377,6 +377,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)\n> > > > > > > > orientation_ = 0;\n> > > > > > > > }\n> > > > > > > >\n> > > > > > > > + mirrored_ = cameraConfigData && cameraConfigData->mirrored;\n> > > > > > > > return capabilities_.initialize(camera_, orientation_, facing_);\n> > > > > > > > }\n> > > > > > > >\n> > > > > > > > @@ -547,6 +548,11 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > > > > > > > return -EINVAL;\n> > > > > > > > }\n> > > > > > > >\n> > > > > > > > + // Rotation is unsupported, so if mirrored just set orientation.\n> > > > > > >\n> > > > > > > Is that true? I thought we usually support 0 and 180 rotations on most\n> > > > > > > sensors at least ?\n> > > > > >\n> > > > > > Ah I misunderstood the below comments/errors about not supporting stream\n> > > > > > rotations. It seems the sensors on my test device don't support rotation. I will\n> > > > > > send out a v2 with this comment removed assuming the rest of this patch\n> > > > > > looks good.\n> > > > > > >\n> > > > > > > We (almost) always use /* */ comments in libcamera ... it's ... probably a\n> > > > > > > stylistic choice because we are mostly started out as C / Linux-kernel\n> > > > > > > devs.\n> > > > > > >\n> > > > > > >\n> > > > > > >\n> > > > > > > > + if (mirrored_) {\n> > > > > > > > + config->orientation = Orientation::Rotate0Mirror;\n> > > > > > > > + }\n> > > > > > > > +\n> > > >\n> > > > Also no {} for single line statement.\n> > > >\n> > > > It's amusing how many times the same stylistic comments had to\n> > > > repeated specifically to chromeos developers. We also have tools\n> > > > available for checking this kind of issues before submitting.\n> > > >\n> > >\n> > > Apologies, I ran checkstyle.py on the change but found no issues, if there\n> > > are other tools for finding style issues I'll be sure to run them in the future.\n> > > I'll update this one as well in v2.\n> >\n> > This is one of your first contributions, so it has been unfair ranting\n> > to you about the past :)\n> >\n> > I'm just suprised a team of specialized and highly skilled engineers\n> > continue pushing their conventions (out of habit I presume) to a different\n> > code base. My frustration comes from having repeated the same comments\n> > over and over in the past years. Apologies if you got ranted to.\n> \n> Sorry to have added to the frustration, and no worries :)\n> \n> > > > > > >\n> > > > > > > How does this interact with rotation as well ? will it take precedence\n> > > > > > > against 180 rotations?\n> > > > > >\n> > > > > > I can't confirm because my sensors aren't affected by the rotation config\n> > > > > > but as far as I can tell the rotation would be unaffected by setting the\n> > > > > > orientation to mirror. There doesn't seem to be anywhere setting the\n> > > > > > orientation in the Android layer, so any rotation would happen independently.\n> > > > > > And the mirroring will just set the HFLIP on the kernel driver, so any higher\n> > > > > > level rotation would just rotate the flipped output as expected.\n> > > > >\n> > > > > Some level of rotation is supposed to be handled automatically by\n> > > > > setting the physical orientation and rotation in device tree.\n> > > > >\n> > > > > Then libcamera will 'aim' for a 0 rotation by default.\n> > > > >\n> > > > > I guess the issue here is that we don't have a way in\n> > > > > device-tree/firmware to mark that the camera is mirrored.\n> > > > >\n> > > > > what does this actually 'mean' by the way? How is the camera physically\n> > > > > mirrored? Is this really a bug in the driver somewhere or something\n> > > > > else?\n> > > > >\n> > > > > I am weary that this patch is possibly a workaround for a problem\n> > > > > somewhere else ?\n> > > >\n> > > > If my recollection is right, when it comes to the camera HAL:\n> > > >\n> > > > 1) Rotation is first parsed from DT, but there we can only express\n> > > > [0, 90, 270, 360] so we don't have a \"mirror\" option\n> > > >\n> > > > 2) If no \"rotation\" property is specified in DT, then the HAL\n> > > > falls-back to parse a \"rotation\" property from configuration file,\n> > > > where again we can only express an angle between [0 - 360]\n> > > >\n> > > > All of this seems to suggest it is not possible to express in DT or\n> > > > HAL configuration file the \"mirroring\" option. However this opens the\n> > > > question that Kieran has asked already. How is the camera module\n> > > > physically mirrored ? is the pixel array reading direction inverted in\n> > > > the driver ?\n> > >\n> > > I'm not 100% sure where the mirroring is coming from. I confirmed it\n> > > through qcam on Ubuntu as well as through the camera app and Chromium\n> > > on a ChromiumOs build so it's not just an application bug. The location and\n> > > orientation coming from ACPI both seem to be correct. The driver seems\n> >\n> > The 'rotation' property from DTS cannot express 'mirrored', and to me\n> > this seems correct, as 'rotation' is about expressing the mounting\n> > rotation of the camera sensor (0, 90, 180, 270)\n> >\n> > Mirroring comes from inverting the reading directions of the pixel\n> > units on the pixel array, what is usually called an HFLIP in v4l2\n> > drivers and we require drivers to not apply any V/HFLIP by default\n> > (something we might want to document in libcamera as well, not just in\n> > Linux).\n> >\n> > > to be fine on all but some Surface Go devices so it seems unrelated.\n> >\n> > What do you mean with \"seems fine\" ? It doesn't mirror ?\n> \n> \"Seems fine\" as in I don't have a device to test and confirm that it actually\n> works as expected and isn't mirrored, but the only references I find to\n> mirrored output from the sensor are for the Surface devices, and I would\n> expect that to be a prominent enough issue that it would be raised\n> somewhere.\n\nDo you mean that all devices you know about that integrate an OV8865\ndisplay mirrored images, or is there a mix of devices with the same\nsensor that display mirrored and non-mirrored images ?\n\n> > > Anything below that is infeasible to fix and the sensor drivers don't\n> > > maintain quirks for particular misbehaving devices, so the HAL seems\n> > > to me the best place to add a mirrored config and pipe it down to the\n> > > driver through the existing Orientation.\n> >\n> > Which sensor are we talking about and where does its driver live ? Can\n> > we see it ? Because some downstream (but also upstream) drivers\n> > sometimes flip by default.\n> \n> This is the ov8865 sensor, driver is drivers/media/i2c/ov8865.c in the\n> upstream kernel. I confirmedthat the default state is unflipped, and\n> actually have a separate patch out (https://lkml.org/lkml/2025/7/22/1443)\n> because there's a bug in the driver which ends up resetting the flip\n> states back to default after we set it.\n> \n> > > > > > > > /*\n> > > > > > > > * Clear and remove any existing configuration from previous calls, and\n> > > > > > > > * ensure the required entries are available without further\n> > > > > > > > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > > > > > > > index 194ca3030..d304a1285 100644\n> > > > > > > > --- a/src/android/camera_device.h\n> > > > > > > > +++ b/src/android/camera_device.h\n> > > > > > > > @@ -126,6 +126,7 @@ private:\n> > > > > > > >\n> > > > > > > > int facing_;\n> > > > > > > > int orientation_;\n> > > > > > > > + int mirrored_;\n> > > > > > > >\n> > > > > > > > CameraMetadata lastSettings_;\n> > > > > > > > };\n> > > > > > > > diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp\n> > > > > > > > index 7ef451ef8..a2beba5d3 100644\n> > > > > > > > --- a/src/android/camera_hal_config.cpp\n> > > > > > > > +++ b/src/android/camera_hal_config.cpp\n> > > > > > > > @@ -33,6 +33,7 @@ private:\n> > > > > > > > int parseCameraConfigData(const std::string &cameraId, const YamlObject &);\n> > > > > > > > int parseLocation(const YamlObject &, CameraConfigData &cameraConfigData);\n> > > > > > > > int parseRotation(const YamlObject &, CameraConfigData &cameraConfigData);\n> > > > > > > > + int parseMirrored(const YamlObject &, CameraConfigData &cameraConfigData);\n> > > > > > > >\n> > > > > > > > std::map *cameras_;\n> > > > > > > > };\n> > > > > > > > @@ -106,6 +107,9 @@ int CameraHalConfig::Private::parseCameraConfigData(const std::string &cameraId,\n> > > > > > > > if (parseRotation(cameraObject, cameraConfigData))\n> > > > > > > > return -EINVAL;\n> > > > > > > >\n> > > > > > > > + /* Parse optional property \"mirrored\" */\n> > > > > > > > + parseMirrored(cameraObject, cameraConfigData);\n> > > > > > > > +\n> > > > > > > > return 0;\n> > > > > > > > }\n> > > > > > > >\n> > > > > > > > @@ -145,6 +149,16 @@ int CameraHalConfig::Private::parseRotation(const YamlObject &cameraObject,\n> > > > > > > > return 0;\n> > > > > > > > }\n> > > > > > > >\n> > > > > > > > +int CameraHalConfig::Private::parseMirrored(const YamlObject &cameraObject,\n> > > > > > > > + CameraConfigData &cameraConfigData)\n> > > > > > > > +{\n> > > > > > > > + if (!cameraObject.contains(\"mirrored\"))\n> > > > > > > > + return -EINVAL;\n> > > > > > > > +\n> > > > > > > > + cameraConfigData.mirrored = cameraObject[\"mirrored\"].get(false);\n> > > > > > > > + return 0;\n> > > > > > > > +}\n> > > > > > > > +\n> > > > > > > > CameraHalConfig::CameraHalConfig()\n> > > > > > > > : Extensible(std::make_unique()), exists_(false), valid_(false)\n> > > > > > > > {\n> > > > > > > > diff --git a/src/android/camera_hal_config.h b/src/android/camera_hal_config.h\n> > > > > > > > index a4bedb6e6..a96190470 100644\n> > > > > > > > --- a/src/android/camera_hal_config.h\n> > > > > > > > +++ b/src/android/camera_hal_config.h\n> > > > > > > > @@ -15,6 +15,7 @@\n> > > > > > > > struct CameraConfigData {\n> > > > > > > > int facing = -1;\n> > > > > > > > int rotation = -1;\n> > > > > > > > + bool mirrored = false;\n> > > > > > > > };\n> > > > > > > >\n> > > > > > > > class CameraHalConfig final : public libcamera::Extensible\n> > > > > > > > --\n> > > > > > > > 2.50.0.727.gbf7dc18ff4-goog\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 E9B03C3237\n\tfor ;\n\tWed, 23 Jul 2025 15:43:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AA74B69055;\n\tWed, 23 Jul 2025 17:43:18 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 35C3E614ED\n\tfor ;\n\tWed, 23 Jul 2025 17:43:17 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id A0D46E92;\n\tWed, 23 Jul 2025 17:42:38 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"qDxNXmHZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1753285358;\n\tbh=zyUbyFGpg8SMdKK7TRmR+n3WEIDPxi7sYPC1rA2y8/Y=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qDxNXmHZbEBA49PFLU1SOssySYwHVGstHjroNg2i4/RWZpRFlflB71dA4ijc3uIIM\n\tm78VsonHb22bc3A6BKNMklijj7FGkfvcSU4XsVoM8qrDUM+Bq7zOz1kNAAFFFwlxqR\n\tHrXqY62kXtXWEudVUhI2/eH7ZD8lLQKE91OU33vM=","Date":"Wed, 23 Jul 2025 18:43:14 +0300","From":"Laurent Pinchart ","To":"Allen Ballway ","Cc":"Jacopo Mondi ,\n\tKieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org, hanlinchen@chromium.org","Subject":"Re: [PATCH] libcamera: android: Add optional mirrored value for\n\tconfig","Message-ID":"<20250723154314.GG6719@pendragon.ideasonboard.com>","References":"<20250716204625.3221592-1-ballway@chromium.org>\n\t<175284070718.560048.16576681811250947472@ping.linuxembedded.co.uk>\n\t\n\t<175309465839.50296.11090024501206804447@ping.linuxembedded.co.uk>\n\t<6tra7qb2ru2nt4cgzpfvtwhcccxjp2qmw52ii4rcizw474cppy@ngyrzgfdxjhy>\n\t\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":35046,"web_url":"https://patchwork.libcamera.org/comment/35046/","msgid":"","date":"2025-07-23T15:54:20","subject":"Re: [PATCH] libcamera: android: Add optional mirrored value for\n\tconfig","submitter":{"id":236,"url":"https://patchwork.libcamera.org/api/people/236/","name":"Allen Ballway","email":"ballway@chromium.org"},"content":"On Wed, Jul 23, 2025 at 8:43 AM Laurent Pinchart\n wrote:\n>\n> On Wed, Jul 23, 2025 at 08:14:45AM -0700, Allen Ballway wrote:\n> > On Wed, Jul 23, 2025 at 1:15 AM Jacopo Mondi wrote:\n> > > On Tue, Jul 22, 2025 at 07:51:53AM -0700, Allen Ballway wrote:\n> > > > On Mon, Jul 21, 2025 at 4:14 AM Jacopo Mondi wrote:\n> > > > > On Mon, Jul 21, 2025 at 11:44:18AM +0100, Kieran Bingham wrote:\n> > > > > > Quoting Allen Ballway (2025-07-18 16:27:23)\n> > > > > > > On Fri, Jul 18, 2025 at 5:11 AM Kieran Bingham wrote:\n> > > > > > > > Quoting Allen Ballway (2025-07-16 21:46:20)\n> > > > > > > > > Adds an optional `mirrored` value to the HAL config, which is used to modify\n> > > > > > > > > the `orientation` of the camera config. No rotation is used, so it can\n> > > > > > > > > only set the orientation to `Orientation::Rotate0Mirror`.\n> > > > > > > > >\n> > > > > > > > > This enables sensors which are incorrectly mirrored to be corrected.\n> > > > > > > > >\n> > > > > > > > > Signed-off-by: Allen Ballway \n> > > > > > > > > ---\n> > > > > > > > > src/android/camera_device.cpp | 6 ++++++\n> > > > > > > > > src/android/camera_device.h | 1 +\n> > > > > > > > > src/android/camera_hal_config.cpp | 14 ++++++++++++++\n> > > > > > > > > src/android/camera_hal_config.h | 1 +\n> > > > > > > > > 4 files changed, 22 insertions(+)\n> > > > > > > > >\n> > > > > > > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > > > > > > > index 80ff248c2..b20e9f3db 100644\n> > > > > > > > > --- a/src/android/camera_device.cpp\n> > > > > > > > > +++ b/src/android/camera_device.cpp\n> > > > > > > > > @@ -377,6 +377,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)\n> > > > > > > > > orientation_ = 0;\n> > > > > > > > > }\n> > > > > > > > >\n> > > > > > > > > + mirrored_ = cameraConfigData && cameraConfigData->mirrored;\n> > > > > > > > > return capabilities_.initialize(camera_, orientation_, facing_);\n> > > > > > > > > }\n> > > > > > > > >\n> > > > > > > > > @@ -547,6 +548,11 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > > > > > > > > return -EINVAL;\n> > > > > > > > > }\n> > > > > > > > >\n> > > > > > > > > + // Rotation is unsupported, so if mirrored just set orientation.\n> > > > > > > >\n> > > > > > > > Is that true? I thought we usually support 0 and 180 rotations on most\n> > > > > > > > sensors at least ?\n> > > > > > >\n> > > > > > > Ah I misunderstood the below comments/errors about not supporting stream\n> > > > > > > rotations. It seems the sensors on my test device don't support rotation. I will\n> > > > > > > send out a v2 with this comment removed assuming the rest of this patch\n> > > > > > > looks good.\n> > > > > > > >\n> > > > > > > > We (almost) always use /* */ comments in libcamera ... it's ... probably a\n> > > > > > > > stylistic choice because we are mostly started out as C / Linux-kernel\n> > > > > > > > devs.\n> > > > > > > >\n> > > > > > > >\n> > > > > > > >\n> > > > > > > > > + if (mirrored_) {\n> > > > > > > > > + config->orientation = Orientation::Rotate0Mirror;\n> > > > > > > > > + }\n> > > > > > > > > +\n> > > > >\n> > > > > Also no {} for single line statement.\n> > > > >\n> > > > > It's amusing how many times the same stylistic comments had to\n> > > > > repeated specifically to chromeos developers. We also have tools\n> > > > > available for checking this kind of issues before submitting.\n> > > > >\n> > > >\n> > > > Apologies, I ran checkstyle.py on the change but found no issues, if there\n> > > > are other tools for finding style issues I'll be sure to run them in the future.\n> > > > I'll update this one as well in v2.\n> > >\n> > > This is one of your first contributions, so it has been unfair ranting\n> > > to you about the past :)\n> > >\n> > > I'm just suprised a team of specialized and highly skilled engineers\n> > > continue pushing their conventions (out of habit I presume) to a different\n> > > code base. My frustration comes from having repeated the same comments\n> > > over and over in the past years. Apologies if you got ranted to.\n> >\n> > Sorry to have added to the frustration, and no worries :)\n> >\n> > > > > > > >\n> > > > > > > > How does this interact with rotation as well ? will it take precedence\n> > > > > > > > against 180 rotations?\n> > > > > > >\n> > > > > > > I can't confirm because my sensors aren't affected by the rotation config\n> > > > > > > but as far as I can tell the rotation would be unaffected by setting the\n> > > > > > > orientation to mirror. There doesn't seem to be anywhere setting the\n> > > > > > > orientation in the Android layer, so any rotation would happen independently.\n> > > > > > > And the mirroring will just set the HFLIP on the kernel driver, so any higher\n> > > > > > > level rotation would just rotate the flipped output as expected.\n> > > > > >\n> > > > > > Some level of rotation is supposed to be handled automatically by\n> > > > > > setting the physical orientation and rotation in device tree.\n> > > > > >\n> > > > > > Then libcamera will 'aim' for a 0 rotation by default.\n> > > > > >\n> > > > > > I guess the issue here is that we don't have a way in\n> > > > > > device-tree/firmware to mark that the camera is mirrored.\n> > > > > >\n> > > > > > what does this actually 'mean' by the way? How is the camera physically\n> > > > > > mirrored? Is this really a bug in the driver somewhere or something\n> > > > > > else?\n> > > > > >\n> > > > > > I am weary that this patch is possibly a workaround for a problem\n> > > > > > somewhere else ?\n> > > > >\n> > > > > If my recollection is right, when it comes to the camera HAL:\n> > > > >\n> > > > > 1) Rotation is first parsed from DT, but there we can only express\n> > > > > [0, 90, 270, 360] so we don't have a \"mirror\" option\n> > > > >\n> > > > > 2) If no \"rotation\" property is specified in DT, then the HAL\n> > > > > falls-back to parse a \"rotation\" property from configuration file,\n> > > > > where again we can only express an angle between [0 - 360]\n> > > > >\n> > > > > All of this seems to suggest it is not possible to express in DT or\n> > > > > HAL configuration file the \"mirroring\" option. However this opens the\n> > > > > question that Kieran has asked already. How is the camera module\n> > > > > physically mirrored ? is the pixel array reading direction inverted in\n> > > > > the driver ?\n> > > >\n> > > > I'm not 100% sure where the mirroring is coming from. I confirmed it\n> > > > through qcam on Ubuntu as well as through the camera app and Chromium\n> > > > on a ChromiumOs build so it's not just an application bug. The location and\n> > > > orientation coming from ACPI both seem to be correct. The driver seems\n> > >\n> > > The 'rotation' property from DTS cannot express 'mirrored', and to me\n> > > this seems correct, as 'rotation' is about expressing the mounting\n> > > rotation of the camera sensor (0, 90, 180, 270)\n> > >\n> > > Mirroring comes from inverting the reading directions of the pixel\n> > > units on the pixel array, what is usually called an HFLIP in v4l2\n> > > drivers and we require drivers to not apply any V/HFLIP by default\n> > > (something we might want to document in libcamera as well, not just in\n> > > Linux).\n> > >\n> > > > to be fine on all but some Surface Go devices so it seems unrelated.\n> > >\n> > > What do you mean with \"seems fine\" ? It doesn't mirror ?\n> >\n> > \"Seems fine\" as in I don't have a device to test and confirm that it actually\n> > works as expected and isn't mirrored, but the only references I find to\n> > mirrored output from the sensor are for the Surface devices, and I would\n> > expect that to be a prominent enough issue that it would be raised\n> > somewhere.\n>\n> Do you mean that all devices you know about that integrate an OV8865\n> display mirrored images, or is there a mix of devices with the same\n> sensor that display mirrored and non-mirrored images ?\n>\nI have only been able to test on Surface Go devices with this sensor, and\nall of them have been mirrored. I'm unsure what other devices use this\nsensor, but from a lack of complaints or issue reports around mirroring\nmake me assume that they receive non-mirrored images.\n> > > > Anything below that is infeasible to fix and the sensor drivers don't\n> > > > maintain quirks for particular misbehaving devices, so the HAL seems\n> > > > to me the best place to add a mirrored config and pipe it down to the\n> > > > driver through the existing Orientation.\n> > >\n> > > Which sensor are we talking about and where does its driver live ? Can\n> > > we see it ? Because some downstream (but also upstream) drivers\n> > > sometimes flip by default.\n> >\n> > This is the ov8865 sensor, driver is drivers/media/i2c/ov8865.c in the\n> > upstream kernel. I confirmedthat the default state is unflipped, and\n> > actually have a separate patch out (https://lkml.org/lkml/2025/7/22/1443)\n> > because there's a bug in the driver which ends up resetting the flip\n> > states back to default after we set it.\n> >\n> > > > > > > > > /*\n> > > > > > > > > * Clear and remove any existing configuration from previous calls, and\n> > > > > > > > > * ensure the required entries are available without further\n> > > > > > > > > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > > > > > > > > index 194ca3030..d304a1285 100644\n> > > > > > > > > --- a/src/android/camera_device.h\n> > > > > > > > > +++ b/src/android/camera_device.h\n> > > > > > > > > @@ -126,6 +126,7 @@ private:\n> > > > > > > > >\n> > > > > > > > > int facing_;\n> > > > > > > > > int orientation_;\n> > > > > > > > > + int mirrored_;\n> > > > > > > > >\n> > > > > > > > > CameraMetadata lastSettings_;\n> > > > > > > > > };\n> > > > > > > > > diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp\n> > > > > > > > > index 7ef451ef8..a2beba5d3 100644\n> > > > > > > > > --- a/src/android/camera_hal_config.cpp\n> > > > > > > > > +++ b/src/android/camera_hal_config.cpp\n> > > > > > > > > @@ -33,6 +33,7 @@ private:\n> > > > > > > > > int parseCameraConfigData(const std::string &cameraId, const YamlObject &);\n> > > > > > > > > int parseLocation(const YamlObject &, CameraConfigData &cameraConfigData);\n> > > > > > > > > int parseRotation(const YamlObject &, CameraConfigData &cameraConfigData);\n> > > > > > > > > + int parseMirrored(const YamlObject &, CameraConfigData &cameraConfigData);\n> > > > > > > > >\n> > > > > > > > > std::map *cameras_;\n> > > > > > > > > };\n> > > > > > > > > @@ -106,6 +107,9 @@ int CameraHalConfig::Private::parseCameraConfigData(const std::string &cameraId,\n> > > > > > > > > if (parseRotation(cameraObject, cameraConfigData))\n> > > > > > > > > return -EINVAL;\n> > > > > > > > >\n> > > > > > > > > + /* Parse optional property \"mirrored\" */\n> > > > > > > > > + parseMirrored(cameraObject, cameraConfigData);\n> > > > > > > > > +\n> > > > > > > > > return 0;\n> > > > > > > > > }\n> > > > > > > > >\n> > > > > > > > > @@ -145,6 +149,16 @@ int CameraHalConfig::Private::parseRotation(const YamlObject &cameraObject,\n> > > > > > > > > return 0;\n> > > > > > > > > }\n> > > > > > > > >\n> > > > > > > > > +int CameraHalConfig::Private::parseMirrored(const YamlObject &cameraObject,\n> > > > > > > > > + CameraConfigData &cameraConfigData)\n> > > > > > > > > +{\n> > > > > > > > > + if (!cameraObject.contains(\"mirrored\"))\n> > > > > > > > > + return -EINVAL;\n> > > > > > > > > +\n> > > > > > > > > + cameraConfigData.mirrored = cameraObject[\"mirrored\"].get(false);\n> > > > > > > > > + return 0;\n> > > > > > > > > +}\n> > > > > > > > > +\n> > > > > > > > > CameraHalConfig::CameraHalConfig()\n> > > > > > > > > : Extensible(std::make_unique()), exists_(false), valid_(false)\n> > > > > > > > > {\n> > > > > > > > > diff --git a/src/android/camera_hal_config.h b/src/android/camera_hal_config.h\n> > > > > > > > > index a4bedb6e6..a96190470 100644\n> > > > > > > > > --- a/src/android/camera_hal_config.h\n> > > > > > > > > +++ b/src/android/camera_hal_config.h\n> > > > > > > > > @@ -15,6 +15,7 @@\n> > > > > > > > > struct CameraConfigData {\n> > > > > > > > > int facing = -1;\n> > > > > > > > > int rotation = -1;\n> > > > > > > > > + bool mirrored = false;\n> > > > > > > > > };\n> > > > > > > > >\n> > > > > > > > > class CameraHalConfig final : public libcamera::Extensible\n> > > > > > > > > --\n> > > > > > > > > 2.50.0.727.gbf7dc18ff4-goog\n> > > > > > > > >\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 5E3D7C3237\n\tfor ;\n\tWed, 23 Jul 2025 15:54:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 53EAE69058;\n\tWed, 23 Jul 2025 17:54:33 +0200 (CEST)","from mail-ej1-x62e.google.com (mail-ej1-x62e.google.com\n\t[IPv6:2a00:1450:4864:20::62e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 156DC614ED\n\tfor ;\n\tWed, 23 Jul 2025 17:54:32 +0200 (CEST)","by mail-ej1-x62e.google.com with SMTP id\n\ta640c23a62f3a-aef575ad59eso572667466b.2\n\tfor ;\n\tWed, 23 Jul 2025 08:54:32 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"lukep6uo\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1753286071; x=1753890871;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=gD00FhJUqLT/O1SfTOgXTlm5TlaVAUIY45yRF2G4GGc=;\n\tb=lukep6uogCRBLWh/ei31nixMkMfj0b7gdLjg/GT7rzCVZvojFifH+49LEzWa9Zu1dC\n\t1RUjg7oN5DkOWKElvo4rgWnWOhxxCaeoUf0QexK0oESPGf0FCnpGKTlo/OIRrRuthDSg\n\tWcN0j9IS0dtUs4P5r7BkU9wZMAtDX7xhaico4=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1753286071; x=1753890871;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=gD00FhJUqLT/O1SfTOgXTlm5TlaVAUIY45yRF2G4GGc=;\n\tb=bhJzKRJv9CTpYgNBHfkYpepBfN8c0x1z8YldM6GknOmN1v7QuGfGBs/XK1x4ED9neg\n\tGGR4z6bjtVLS9zL3hU/jWtcwideFEr+91R8wEHE1J0WQ+H+vRoAgteDNSpp5IqWPYoRq\n\tTGq+k67uBFD6wTQOI4NgCOxXV33cY6DN3Xzkt/rqBT4Yy7vmvuJko+lGRXt11dnN1V/0\n\tDct2C8T9aFgNvuWxWpLRIIPB+EiFhH+F+jKSP0yJZQ0hxCR2omiNEYRJBiK3xTTDu70g\n\tX041sTL8Lg75JMNINIwVZKTMipEw+IDUZzOEu3esD02lSn0k9JGBN3Xz1l5/CqBrh4r+\n\tZnMw==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCV+3TScqifmKoqKMM2E30KWyNccxnKlsaP0sBa9AX0pFJgIwPCJWTXeCXTqh5ILE6mQp4dd737d1jtjxIL1N0U=@lists.libcamera.org","X-Gm-Message-State":"AOJu0Yyfid7y6YP7jLOF0ZE7Jr0ccTWDo7ynVBf0hxxzZ3Tp89LzeJr8\n\t+e7zp4yZ9rvrT+0mkxYIgixlb4xQJT0ql25bYTGUzBGJH6JtR152Ku7LMwXrKj7WKeOrojH58sn\n\t1uL7zP/UYNKgxUDd30dqrbwaCeAwOnI4UYgDqRwHV","X-Gm-Gg":"ASbGncvCYsmzT6BjXm+AZUzIphJE9eUMK6O+9hwZPwr6k5av7CESMTYFjCbUSISE7Pe\n\tE1UGpgqayzAOajrRC6W5Ap/8GalJe5Dl+A9Yp9dTBg/BomMg0UhskW0CD0BwZApOuxdtooas0Xz\n\tkDLxXd9M8twj1Q2f/NaW6+dYF4E2eRsQpgllv6UR4Ckge9PwJEt83YyedS5N0i1Q8Ic/mZ8FJJe\n\tYnhABpRggeWTVYz/g==","X-Google-Smtp-Source":"AGHT+IFM7AP0jq4gPAxJXahO9/o5QSwQhUeE1FRoy+At+MhurW+whLX7GUVASdg4YWujWhpiBPzAB94HF4JXaYNCN6c=","X-Received":"by 2002:a17:907:7e84:b0:af2:844e:a72f with SMTP id\n\ta640c23a62f3a-af2f8d4ece7mr318637666b.47.1753286071388;\n\tWed, 23 Jul 2025 08:54:31 -0700 (PDT)","MIME-Version":"1.0","References":"<20250716204625.3221592-1-ballway@chromium.org>\n\t<175284070718.560048.16576681811250947472@ping.linuxembedded.co.uk>\n\t\n\t<175309465839.50296.11090024501206804447@ping.linuxembedded.co.uk>\n\t<6tra7qb2ru2nt4cgzpfvtwhcccxjp2qmw52ii4rcizw474cppy@ngyrzgfdxjhy>\n\t\n\t\n\t\n\t<20250723154314.GG6719@pendragon.ideasonboard.com>","In-Reply-To":"<20250723154314.GG6719@pendragon.ideasonboard.com>","From":"Allen Ballway ","Date":"Wed, 23 Jul 2025 08:54:20 -0700","X-Gm-Features":"Ac12FXzbpXcs7OLhX7Su3fIllSonvbdUAtmsY6ZEdqVVzIONf3le7xzrNKE0GVw","Message-ID":"","Subject":"Re: [PATCH] libcamera: android: Add optional mirrored value for\n\tconfig","To":"Laurent Pinchart ","Cc":"Jacopo Mondi , \n\tKieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org, hanlinchen@chromium.org","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":35049,"web_url":"https://patchwork.libcamera.org/comment/35049/","msgid":"<20250723161441.GC14576@pendragon.ideasonboard.com>","date":"2025-07-23T16:14:41","subject":"Re: [PATCH] libcamera: android: Add optional mirrored value for\n\tconfig","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Jul 23, 2025 at 08:54:20AM -0700, Allen Ballway wrote:\n> On Wed, Jul 23, 2025 at 8:43 AM Laurent Pinchart wrote:\n> > On Wed, Jul 23, 2025 at 08:14:45AM -0700, Allen Ballway wrote:\n> > > On Wed, Jul 23, 2025 at 1:15 AM Jacopo Mondi wrote:\n> > > > On Tue, Jul 22, 2025 at 07:51:53AM -0700, Allen Ballway wrote:\n> > > > > On Mon, Jul 21, 2025 at 4:14 AM Jacopo Mondi wrote:\n> > > > > > On Mon, Jul 21, 2025 at 11:44:18AM +0100, Kieran Bingham wrote:\n> > > > > > > Quoting Allen Ballway (2025-07-18 16:27:23)\n> > > > > > > > On Fri, Jul 18, 2025 at 5:11 AM Kieran Bingham wrote:\n> > > > > > > > > Quoting Allen Ballway (2025-07-16 21:46:20)\n> > > > > > > > > > Adds an optional `mirrored` value to the HAL config, which is used to modify\n> > > > > > > > > > the `orientation` of the camera config. No rotation is used, so it can\n> > > > > > > > > > only set the orientation to `Orientation::Rotate0Mirror`.\n> > > > > > > > > >\n> > > > > > > > > > This enables sensors which are incorrectly mirrored to be corrected.\n> > > > > > > > > >\n> > > > > > > > > > Signed-off-by: Allen Ballway \n> > > > > > > > > > ---\n> > > > > > > > > > src/android/camera_device.cpp | 6 ++++++\n> > > > > > > > > > src/android/camera_device.h | 1 +\n> > > > > > > > > > src/android/camera_hal_config.cpp | 14 ++++++++++++++\n> > > > > > > > > > src/android/camera_hal_config.h | 1 +\n> > > > > > > > > > 4 files changed, 22 insertions(+)\n> > > > > > > > > >\n> > > > > > > > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > > > > > > > > index 80ff248c2..b20e9f3db 100644\n> > > > > > > > > > --- a/src/android/camera_device.cpp\n> > > > > > > > > > +++ b/src/android/camera_device.cpp\n> > > > > > > > > > @@ -377,6 +377,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)\n> > > > > > > > > > orientation_ = 0;\n> > > > > > > > > > }\n> > > > > > > > > >\n> > > > > > > > > > + mirrored_ = cameraConfigData && cameraConfigData->mirrored;\n> > > > > > > > > > return capabilities_.initialize(camera_, orientation_, facing_);\n> > > > > > > > > > }\n> > > > > > > > > >\n> > > > > > > > > > @@ -547,6 +548,11 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > > > > > > > > > return -EINVAL;\n> > > > > > > > > > }\n> > > > > > > > > >\n> > > > > > > > > > + // Rotation is unsupported, so if mirrored just set orientation.\n> > > > > > > > >\n> > > > > > > > > Is that true? I thought we usually support 0 and 180 rotations on most\n> > > > > > > > > sensors at least ?\n> > > > > > > >\n> > > > > > > > Ah I misunderstood the below comments/errors about not supporting stream\n> > > > > > > > rotations. It seems the sensors on my test device don't support rotation. I will\n> > > > > > > > send out a v2 with this comment removed assuming the rest of this patch\n> > > > > > > > looks good.\n> > > > > > > > >\n> > > > > > > > > We (almost) always use /* */ comments in libcamera ... it's ... probably a\n> > > > > > > > > stylistic choice because we are mostly started out as C / Linux-kernel\n> > > > > > > > > devs.\n> > > > > > > > >\n> > > > > > > > >\n> > > > > > > > >\n> > > > > > > > > > + if (mirrored_) {\n> > > > > > > > > > + config->orientation = Orientation::Rotate0Mirror;\n> > > > > > > > > > + }\n> > > > > > > > > > +\n> > > > > >\n> > > > > > Also no {} for single line statement.\n> > > > > >\n> > > > > > It's amusing how many times the same stylistic comments had to\n> > > > > > repeated specifically to chromeos developers. We also have tools\n> > > > > > available for checking this kind of issues before submitting.\n> > > > > >\n> > > > >\n> > > > > Apologies, I ran checkstyle.py on the change but found no issues, if there\n> > > > > are other tools for finding style issues I'll be sure to run them in the future.\n> > > > > I'll update this one as well in v2.\n> > > >\n> > > > This is one of your first contributions, so it has been unfair ranting\n> > > > to you about the past :)\n> > > >\n> > > > I'm just suprised a team of specialized and highly skilled engineers\n> > > > continue pushing their conventions (out of habit I presume) to a different\n> > > > code base. My frustration comes from having repeated the same comments\n> > > > over and over in the past years. Apologies if you got ranted to.\n> > >\n> > > Sorry to have added to the frustration, and no worries :)\n> > >\n> > > > > > > > >\n> > > > > > > > > How does this interact with rotation as well ? will it take precedence\n> > > > > > > > > against 180 rotations?\n> > > > > > > >\n> > > > > > > > I can't confirm because my sensors aren't affected by the rotation config\n> > > > > > > > but as far as I can tell the rotation would be unaffected by setting the\n> > > > > > > > orientation to mirror. There doesn't seem to be anywhere setting the\n> > > > > > > > orientation in the Android layer, so any rotation would happen independently.\n> > > > > > > > And the mirroring will just set the HFLIP on the kernel driver, so any higher\n> > > > > > > > level rotation would just rotate the flipped output as expected.\n> > > > > > >\n> > > > > > > Some level of rotation is supposed to be handled automatically by\n> > > > > > > setting the physical orientation and rotation in device tree.\n> > > > > > >\n> > > > > > > Then libcamera will 'aim' for a 0 rotation by default.\n> > > > > > >\n> > > > > > > I guess the issue here is that we don't have a way in\n> > > > > > > device-tree/firmware to mark that the camera is mirrored.\n> > > > > > >\n> > > > > > > what does this actually 'mean' by the way? How is the camera physically\n> > > > > > > mirrored? Is this really a bug in the driver somewhere or something\n> > > > > > > else?\n> > > > > > >\n> > > > > > > I am weary that this patch is possibly a workaround for a problem\n> > > > > > > somewhere else ?\n> > > > > >\n> > > > > > If my recollection is right, when it comes to the camera HAL:\n> > > > > >\n> > > > > > 1) Rotation is first parsed from DT, but there we can only express\n> > > > > > [0, 90, 270, 360] so we don't have a \"mirror\" option\n> > > > > >\n> > > > > > 2) If no \"rotation\" property is specified in DT, then the HAL\n> > > > > > falls-back to parse a \"rotation\" property from configuration file,\n> > > > > > where again we can only express an angle between [0 - 360]\n> > > > > >\n> > > > > > All of this seems to suggest it is not possible to express in DT or\n> > > > > > HAL configuration file the \"mirroring\" option. However this opens the\n> > > > > > question that Kieran has asked already. How is the camera module\n> > > > > > physically mirrored ? is the pixel array reading direction inverted in\n> > > > > > the driver ?\n> > > > >\n> > > > > I'm not 100% sure where the mirroring is coming from. I confirmed it\n> > > > > through qcam on Ubuntu as well as through the camera app and Chromium\n> > > > > on a ChromiumOs build so it's not just an application bug. The location and\n> > > > > orientation coming from ACPI both seem to be correct. The driver seems\n> > > >\n> > > > The 'rotation' property from DTS cannot express 'mirrored', and to me\n> > > > this seems correct, as 'rotation' is about expressing the mounting\n> > > > rotation of the camera sensor (0, 90, 180, 270)\n> > > >\n> > > > Mirroring comes from inverting the reading directions of the pixel\n> > > > units on the pixel array, what is usually called an HFLIP in v4l2\n> > > > drivers and we require drivers to not apply any V/HFLIP by default\n> > > > (something we might want to document in libcamera as well, not just in\n> > > > Linux).\n> > > >\n> > > > > to be fine on all but some Surface Go devices so it seems unrelated.\n> > > >\n> > > > What do you mean with \"seems fine\" ? It doesn't mirror ?\n> > >\n> > > \"Seems fine\" as in I don't have a device to test and confirm that it actually\n> > > works as expected and isn't mirrored, but the only references I find to\n> > > mirrored output from the sensor are for the Surface devices, and I would\n> > > expect that to be a prominent enough issue that it would be raised\n> > > somewhere.\n> >\n> > Do you mean that all devices you know about that integrate an OV8865\n> > display mirrored images, or is there a mix of devices with the same\n> > sensor that display mirrored and non-mirrored images ?\n> \n> I have only been able to test on Surface Go devices with this sensor, and\n> all of them have been mirrored. I'm unsure what other devices use this\n> sensor, but from a lack of complaints or issue reports around mirroring\n> make me assume that they receive non-mirrored images.\n\nI wouldn't be so sure about that honestly.\n\nIn any case, there's no way the Surface machines have mounted a mirror\nin front of the sensor. This issue may be a bug in the driver that\naffects all platforms, or possibly a difference in behaviour between\ndifferent revisions of the sensor. In any case, the fix shouldn't be in\nlibcamera, especially not in the HAL implementation.\n\n> > > > > Anything below that is infeasible to fix and the sensor drivers don't\n> > > > > maintain quirks for particular misbehaving devices, so the HAL seems\n> > > > > to me the best place to add a mirrored config and pipe it down to the\n> > > > > driver through the existing Orientation.\n> > > >\n> > > > Which sensor are we talking about and where does its driver live ? Can\n> > > > we see it ? Because some downstream (but also upstream) drivers\n> > > > sometimes flip by default.\n> > >\n> > > This is the ov8865 sensor, driver is drivers/media/i2c/ov8865.c in the\n> > > upstream kernel. I confirmedthat the default state is unflipped, and\n> > > actually have a separate patch out (https://lkml.org/lkml/2025/7/22/1443)\n> > > because there's a bug in the driver which ends up resetting the flip\n> > > states back to default after we set it.\n> > >\n> > > > > > > > > > /*\n> > > > > > > > > > * Clear and remove any existing configuration from previous calls, and\n> > > > > > > > > > * ensure the required entries are available without further\n> > > > > > > > > > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > > > > > > > > > index 194ca3030..d304a1285 100644\n> > > > > > > > > > --- a/src/android/camera_device.h\n> > > > > > > > > > +++ b/src/android/camera_device.h\n> > > > > > > > > > @@ -126,6 +126,7 @@ private:\n> > > > > > > > > >\n> > > > > > > > > > int facing_;\n> > > > > > > > > > int orientation_;\n> > > > > > > > > > + int mirrored_;\n> > > > > > > > > >\n> > > > > > > > > > CameraMetadata lastSettings_;\n> > > > > > > > > > };\n> > > > > > > > > > diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp\n> > > > > > > > > > index 7ef451ef8..a2beba5d3 100644\n> > > > > > > > > > --- a/src/android/camera_hal_config.cpp\n> > > > > > > > > > +++ b/src/android/camera_hal_config.cpp\n> > > > > > > > > > @@ -33,6 +33,7 @@ private:\n> > > > > > > > > > int parseCameraConfigData(const std::string &cameraId, const YamlObject &);\n> > > > > > > > > > int parseLocation(const YamlObject &, CameraConfigData &cameraConfigData);\n> > > > > > > > > > int parseRotation(const YamlObject &, CameraConfigData &cameraConfigData);\n> > > > > > > > > > + int parseMirrored(const YamlObject &, CameraConfigData &cameraConfigData);\n> > > > > > > > > >\n> > > > > > > > > > std::map *cameras_;\n> > > > > > > > > > };\n> > > > > > > > > > @@ -106,6 +107,9 @@ int CameraHalConfig::Private::parseCameraConfigData(const std::string &cameraId,\n> > > > > > > > > > if (parseRotation(cameraObject, cameraConfigData))\n> > > > > > > > > > return -EINVAL;\n> > > > > > > > > >\n> > > > > > > > > > + /* Parse optional property \"mirrored\" */\n> > > > > > > > > > + parseMirrored(cameraObject, cameraConfigData);\n> > > > > > > > > > +\n> > > > > > > > > > return 0;\n> > > > > > > > > > }\n> > > > > > > > > >\n> > > > > > > > > > @@ -145,6 +149,16 @@ int CameraHalConfig::Private::parseRotation(const YamlObject &cameraObject,\n> > > > > > > > > > return 0;\n> > > > > > > > > > }\n> > > > > > > > > >\n> > > > > > > > > > +int CameraHalConfig::Private::parseMirrored(const YamlObject &cameraObject,\n> > > > > > > > > > + CameraConfigData &cameraConfigData)\n> > > > > > > > > > +{\n> > > > > > > > > > + if (!cameraObject.contains(\"mirrored\"))\n> > > > > > > > > > + return -EINVAL;\n> > > > > > > > > > +\n> > > > > > > > > > + cameraConfigData.mirrored = cameraObject[\"mirrored\"].get(false);\n> > > > > > > > > > + return 0;\n> > > > > > > > > > +}\n> > > > > > > > > > +\n> > > > > > > > > > CameraHalConfig::CameraHalConfig()\n> > > > > > > > > > : Extensible(std::make_unique()), exists_(false), valid_(false)\n> > > > > > > > > > {\n> > > > > > > > > > diff --git a/src/android/camera_hal_config.h b/src/android/camera_hal_config.h\n> > > > > > > > > > index a4bedb6e6..a96190470 100644\n> > > > > > > > > > --- a/src/android/camera_hal_config.h\n> > > > > > > > > > +++ b/src/android/camera_hal_config.h\n> > > > > > > > > > @@ -15,6 +15,7 @@\n> > > > > > > > > > struct CameraConfigData {\n> > > > > > > > > > int facing = -1;\n> > > > > > > > > > int rotation = -1;\n> > > > > > > > > > + bool mirrored = false;\n> > > > > > > > > > };\n> > > > > > > > > >\n> > > > > > > > > > class CameraHalConfig final : public libcamera::Extensible","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 191DCBDCC1\n\tfor ;\n\tWed, 23 Jul 2025 16:14:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4D0A169057;\n\tWed, 23 Jul 2025 18:14:45 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 94462614ED\n\tfor ;\n\tWed, 23 Jul 2025 18:14:44 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id E42E4982;\n\tWed, 23 Jul 2025 18:14:05 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"hhF/8v05\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1753287246;\n\tbh=Iy2z+vFn5wUgt3A3OgBUCUnItZCHtc2Hq0a9aec1P48=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hhF/8v05WVqwmKFBRRDoxGUAKA2lYnJgwZ7Ls7VGpYRBZW3fCXq/lHl6GijXXStjn\n\tvY7o6TNTwdb7iX5yk5W3MNFceu86XMeRgpLc42i2Bp4hndhX2uqKTh7LfHKLFdl9Lq\n\tA63SDrgt6XO8mxxXsy1TCertT1Dl+DUQlzMU9yF4=","Date":"Wed, 23 Jul 2025 19:14:41 +0300","From":"Laurent Pinchart ","To":"Allen Ballway ","Cc":"Jacopo Mondi ,\n\tKieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org, hanlinchen@chromium.org","Subject":"Re: [PATCH] libcamera: android: Add optional mirrored value for\n\tconfig","Message-ID":"<20250723161441.GC14576@pendragon.ideasonboard.com>","References":"<20250716204625.3221592-1-ballway@chromium.org>\n\t<175284070718.560048.16576681811250947472@ping.linuxembedded.co.uk>\n\t\n\t<175309465839.50296.11090024501206804447@ping.linuxembedded.co.uk>\n\t<6tra7qb2ru2nt4cgzpfvtwhcccxjp2qmw52ii4rcizw474cppy@ngyrzgfdxjhy>\n\t\n\t\n\t\n\t<20250723154314.GG6719@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]