[{"id":4998,"web_url":"https://patchwork.libcamera.org/comment/4998/","msgid":"<20200604021857.GC7617@pendragon.ideasonboard.com>","date":"2020-06-04T02:18:57","subject":"Re: [libcamera-devel] [PATCH 6/8] android: camera_manager: Replace\n\thardcoded stream configuration","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Tue, May 26, 2020 at 04:22:35PM +0200, Jacopo Mondi wrote:\n> Replace the hardcoded stream configuration map with the information\n> collected at CameraDevice initialization time.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  src/android/camera_device.cpp | 38 ++++++++++++++++++-----------------\n>  1 file changed, 20 insertions(+), 18 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 6cc377820210..f8a52342abe5 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -623,23 +623,22 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n>  \tstaticMetadata_->addEntry(ANDROID_SCALER_AVAILABLE_MAX_DIGITAL_ZOOM,\n>  \t\t\t\t  &maxDigitalZoom, 1);\n>  \n> -\tstd::vector<uint32_t> availableStreamFormats = {\n> -\t\tANDROID_SCALER_AVAILABLE_FORMATS_BLOB,\n> -\t\tANDROID_SCALER_AVAILABLE_FORMATS_YCbCr_420_888,\n> -\t\tANDROID_SCALER_AVAILABLE_FORMATS_IMPLEMENTATION_DEFINED,\n> -\t};\n> +\tstd::vector<uint32_t> availableStreamFormats;\n> +\tfor (const auto &entry : streamConfigurations_)\n> +\t\tavailableStreamFormats.push_back(entry.androidScalerCode);\n\n\tstd::transform(streamConfigurations_.begin(), streamConfigurations_.end(),\n\t\t       std::back_inserter(availableStreamFormats),\n\t\t       [](const auto &entry) { return entry.androidScalerCode; });\n\nUp to you.\n\n>  \tstaticMetadata_->addEntry(ANDROID_SCALER_AVAILABLE_FORMATS,\n>  \t\t\t\t  availableStreamFormats.data(),\n>  \t\t\t\t  availableStreamFormats.size());\n>  \n> -\tstd::vector<uint32_t> availableStreamConfigurations = {\n> -\t\tANDROID_SCALER_AVAILABLE_FORMATS_BLOB, 2560, 1920,\n> -\t\tANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT,\n> -\t\tANDROID_SCALER_AVAILABLE_FORMATS_YCbCr_420_888, 2560, 1920,\n> -\t\tANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT,\n> -\t\tANDROID_SCALER_AVAILABLE_FORMATS_IMPLEMENTATION_DEFINED, 2560, 1920,\n> -\t\tANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT,\n> -\t};\n> +\tstd::vector<uint32_t> availableStreamConfigurations;\n\nIf you want to make this a bit more efficient, you could call\n\n\tavailableStreamConfigurations.reserve(streamConfigurations_.size() * 4);\n\n> +\tfor (const auto &entry : streamConfigurations_) {\n> +\t\tavailableStreamConfigurations.push_back(entry.androidScalerCode);\n> +\t\tavailableStreamConfigurations.push_back(entry.resolution.width);\n> +\t\tavailableStreamConfigurations.push_back(entry.resolution.height);\n> +\t\tavailableStreamConfigurations.push_back(entry.input ?\n> +\t\t\tANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_INPUT :\n> +\t\t\tANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT);\n\nThis is where I would hardcode OUTPUT (see review of previous patches).\n\n> +\t}\n>  \tstaticMetadata_->addEntry(ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS,\n>  \t\t\t\t  availableStreamConfigurations.data(),\n>  \t\t\t\t  availableStreamConfigurations.size());\n> @@ -651,11 +650,14 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n>  \t\t\t\t  availableStallDurations.data(),\n>  \t\t\t\t  availableStallDurations.size());\n>  \n> -\tstd::vector<int64_t> minFrameDurations = {\n> -\t\tANDROID_SCALER_AVAILABLE_FORMATS_BLOB, 2560, 1920, 33333333,\n> -\t\tANDROID_SCALER_AVAILABLE_FORMATS_IMPLEMENTATION_DEFINED, 2560, 1920, 33333333,\n> -\t\tANDROID_SCALER_AVAILABLE_FORMATS_YCbCr_420_888, 2560, 1920, 33333333,\n> -\t};\n> +\t/* \\todo Collect the minimum frame duration from the camera. */\n> +\tstd::vector<int64_t> minFrameDurations;\n> +\tfor (const auto &entry : streamConfigurations_) {\n> +\t\tminFrameDurations.push_back(entry.androidScalerCode);\n> +\t\tminFrameDurations.push_back(entry.resolution.width);\n> +\t\tminFrameDurations.push_back(entry.resolution.height);\n> +\t\tminFrameDurations.push_back(33333333);\n> +\t}\n>  \tstaticMetadata_->addEntry(ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS,\n>  \t\t\t\t  minFrameDurations.data(),\n>  \t\t\t\t  minFrameDurations.size());\n\nVery nice :-)\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["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 72B6561012\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  4 Jun 2020 04:19:14 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E464629B;\n\tThu,  4 Jun 2020 04:19:13 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"vr02RL2k\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1591237154;\n\tbh=6pm7G353OpCgwysC24HyBGUu3cV7qcWI0Y9itIZpf5g=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=vr02RL2kGBjLF7Q7zg1tUH0YtSF8RMkbkziyRAqc2Vsjfpb5uq0kNYOvd0J1yjud2\n\tMeFhILQNHqsahr5mhzijoa6G4ZeQwcq7oQoktUbE5AMjxBTw0LiXADVPiLbYqG6NcV\n\tdnwdkQ4walK4UD5FHBq/Q9Gd0HJDXyvD3KyHlKmM=","Date":"Thu, 4 Jun 2020 05:18:57 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200604021857.GC7617@pendragon.ideasonboard.com>","References":"<20200526142237.407557-1-jacopo@jmondi.org>\n\t<20200526142237.407557-7-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200526142237.407557-7-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH 6/8] android: camera_manager: Replace\n\thardcoded stream configuration","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 04 Jun 2020 02:19:14 -0000"}}]