[{"id":20392,"web_url":"https://patchwork.libcamera.org/comment/20392/","msgid":"","date":"2021-10-22T13:43:25","subject":"Re: [libcamera-devel] [PATCH 6/6] pipeline: raspberrypi: Apply\n\tsensor flips at the start of configure()","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThanks for this patch!\n\nOn Fri, 22 Oct 2021 at 12:55, Naushir Patuck wrote:\n>\n> Sensor flips might change the Bayer order of the requested format. The existing\n> code would set a sensor format along with the appropriate Unicam and ISP input\n> formats, but reset the latter two on start() once the flips had been requested.\n>\n> We can now set the sensor flips just after we set the sensor mode in\n> configure(), thereby not needing the second pair of format sets in start().\n>\n> Signed-off-by: Naushir Patuck \n\nLooks like a significant improvement to me. To be honest, I'm a bit\nbaffled how complicated the previous version was. Did I write it, or\nsomething?\n\nReview-by: David Plowman \n\nThanks!\nDavid\n\n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 61 +++++++------------\n> 1 file changed, 22 insertions(+), 39 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 5aaf24436f27..8fb6197ec283 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -639,16 +639,35 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> /* First calculate the best sensor mode we can use based on the user request. */\n> SensorMode sensorMode = findBestMode(data->sensorFormats_, rawStream ? sensorSize : maxSize);\n> V4L2SubdeviceFormat sensorFormat = toV4L2SubdeviceFormat(sensorMode);\n> - V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorMode);\n>\n> ret = data->sensor_->setFormat(&sensorFormat);\n> if (ret)\n> return ret;\n>\n> /*\n> - * Unicam image output format. The ISP input format gets set at start,\n> - * just in case we have swapped bayer orders due to flips.\n> + * Configure the H/V flip controls based on the combination of\n> + * the sensor and user transform.\n> */\n> + if (data->supportsFlips_) {\n> + const RPiCameraConfiguration *rpiConfig =\n> + static_cast(config);\n> + ControlList controls;\n> +\n> + controls.set(V4L2_CID_HFLIP,\n> + static_cast(!!(rpiConfig->combinedTransform_ & Transform::HFlip)));\n> + controls.set(V4L2_CID_VFLIP,\n> + static_cast(!!(rpiConfig->combinedTransform_ & Transform::VFlip)));\n> + data->setSensorControls(controls);\n> +\n> + /*\n> + * IPA configure may have changed the sensor flips - hence the bayer\n> + * order. So update the sensor format now.\n> + */\n> + data->sensor_->device()->getFormat(0, &sensorFormat);\n> + sensorMode.first = BayerFormat::fromMbusCode(sensorFormat.mbus_code).toPixelFormat();\n> + }\n> +\n> + V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorMode);\n> ret = data->unicam_[Unicam::Image].dev()->setFormat(&unicamFormat);\n> if (ret)\n> return ret;\n> @@ -657,10 +676,6 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> << \" - Selected sensor mode: \" << sensorFormat.toString()\n> << \" - Selected unicam mode: \" << unicamFormat.toString();\n>\n> - /*\n> - * This format may be reset on start() if the bayer order has changed\n> - * because of flips in the sensor.\n> - */\n> ret = data->isp_[Isp::Input].dev()->setFormat(&unicamFormat);\n> if (ret)\n> return ret;\n> @@ -881,23 +896,6 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)\n> return ret;\n> }\n>\n> - /*\n> - * IPA configure may have changed the sensor flips - hence the bayer\n> - * order. Get the sensor format and set the ISP input now.\n> - */\n> - V4L2SubdeviceFormat sensorFormat;\n> - data->sensor_->device()->getFormat(0, &sensorFormat);\n> -\n> - V4L2DeviceFormat ispFormat;\n> - ispFormat.fourcc = BayerFormat::fromMbusCode(sensorFormat.mbus_code).toV4L2PixelFormat();\n> - ispFormat.size = sensorFormat.size;\n> -\n> - ret = data->isp_[Isp::Input].dev()->setFormat(&ispFormat);\n> - if (ret) {\n> - stop(camera);\n> - return ret;\n> - }\n> -\n> /* Enable SOF event generation. */\n> data->unicam_[Unicam::Image].dev()->setFrameStartEnabled(true);\n>\n> @@ -1311,10 +1309,6 @@ int RPiCameraData::loadIPA(ipa::RPi::SensorConfig *sensorConfig)\n>\n> int RPiCameraData::configureIPA(const CameraConfiguration *config)\n> {\n> - /* We know config must be an RPiCameraConfiguration. */\n> - const RPiCameraConfiguration *rpiConfig =\n> - static_cast(config);\n> -\n> std::map streamConfig;\n> std::map entityControls;\n> ipa::RPi::IPAConfig ipaConfig;\n> @@ -1365,17 +1359,6 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)\n> return -EPIPE;\n> }\n>\n> - /*\n> - * Configure the H/V flip controls based on the combination of\n> - * the sensor and user transform.\n> - */\n> - if (supportsFlips_) {\n> - controls.set(V4L2_CID_HFLIP,\n> - static_cast(!!(rpiConfig->combinedTransform_ & Transform::HFlip)));\n> - controls.set(V4L2_CID_VFLIP,\n> - static_cast(!!(rpiConfig->combinedTransform_ & Transform::VFlip)));\n> - }\n> -\n> if (!controls.empty())\n> setSensorControls(controls);\n>\n> --\n> 2.25.1\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 3DD5ABF415\n\tfor ;\n\tFri, 22 Oct 2021 13:43:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7806268F59;\n\tFri, 22 Oct 2021 15:43:37 +0200 (CEST)","from mail-wr1-x432.google.com (mail-wr1-x432.google.com\n\t[IPv6:2a00:1450:4864:20::432])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 538B16012A\n\tfor ;\n\tFri, 22 Oct 2021 15:43:36 +0200 (CEST)","by mail-wr1-x432.google.com with SMTP id v17so807441wrv.9\n\tfor ;\n\tFri, 22 Oct 2021 06:43:36 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"aFlbnsK4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=XtrbvxAvcw6VEQj+gRdAO19KiQIFxO8EUFi66bifypo=;\n\tb=aFlbnsK41Xz5R0rY+L1QQ0uZkgh7AzmTRwCBtyw4NCinAa5zvhaIbGFsIFC1SqYdqS\n\tw2s5uFRX+jCY63UEw94SGcA0iw/82Rl3AQsu9VhG1+mb/Zod4w4zbGtUAQMRfrXbmYXJ\n\t2BsHxj8lKEee96LgR3MLl64m2EipOxA9XwmqPmCk7/ZmvKaIkIiADvFTsMUc5aTqJh27\n\tmAEJioe/lVd+ibsMx9ccV/c2MS4+EQ41IqaROwlYYxQUYLSuKnfAC3ROEWjru7aH7aOi\n\tWU5mnY+E0qWMTknAAL4kHxUkZ/VDtkdT6F9p/5l103kLhsnkqkBueyo7V/NGLaPg6m4D\n\tGU9w==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=XtrbvxAvcw6VEQj+gRdAO19KiQIFxO8EUFi66bifypo=;\n\tb=d/81gAsh0IgSReaSIqOr/k2lZfG3z5pHMS5XyOFzhkyWYC16LqTeQdmLrkwCXsA8/P\n\tYKnYjgc06o1cuqOYbj6cT4WV6iWwcPHknaMt6y5lqh17d0xGtOdQGHo3uFzUpMMxohzK\n\tPUX2U5Vzl2/BlHtR3PmyQ91l4nMmSiVxFuCLFcTQADIKh52FFKjpVGj643n4lHv1NmgQ\n\tl0LviZZJnX7E2J3vZZD19CSxAGjyjygnmTmcOmO6b9nFS6YnIRAFq5mlLjA9D3ic4B0d\n\tYEcfhV4Cv/3z4h9dEdmhcLC0auTpz9kRK8EoPq/G7o7qTwWsHRfsES5ZAm9PGN1VMjhR\n\tt8pQ==","X-Gm-Message-State":"AOAM530AtEcKWDKsjHL6Yp6PjucFb/sfCN7EMt0BliexV5ogHb/Ey4aI\n\tjVklzAd3vVs9yvN6m8Dh/Pp/S2G++0Aie3n6YqUaZ/FQuw8=","X-Google-Smtp-Source":"ABdhPJwZ01Lt+rNrOIPtYxflsv9APczqBffKpc9nH4taV5YIVIxP4M6CH2Pn2a7H6oZlEBGrgEjKrthLP7M5pZ6YUQM=","X-Received":"by 2002:a05:6000:90:: with SMTP id\n\tm16mr11282216wrx.354.1634910215890; \n\tFri, 22 Oct 2021 06:43:35 -0700 (PDT)","MIME-Version":"1.0","References":"<20211022115537.2964533-1-naush@raspberrypi.com>\n\t<20211022115537.2964533-7-naush@raspberrypi.com>","In-Reply-To":"<20211022115537.2964533-7-naush@raspberrypi.com>","From":"David Plowman ","Date":"Fri, 22 Oct 2021 14:43:25 +0100","Message-ID":"","To":"Naushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 6/6] pipeline: raspberrypi: Apply\n\tsensor flips at the start of configure()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]