[{"id":5081,"web_url":"https://patchwork.libcamera.org/comment/5081/","msgid":"<20200606000103.GU26752@pendragon.ideasonboard.com>","date":"2020-06-06T00:01:03","subject":"Re: [libcamera-devel] [PATCH v3 1/8] android: camera_device:\n\tInitialize 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 Fri, Jun 05, 2020 at 04:09:55PM +0200, Jacopo Mondi wrote:\n> Initialize the stream configuration map by applying the Android Camera3\n> requested resolutions and formats to the libcamera Camera device.\n> \n> For each required format test a list of required and optional\n> resolutions, construct a map to translate from Android format to the\n> libcamera formats and store the available stream configuration to\n> be provided to the Android framework through static metadata.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  src/android/camera_device.cpp | 224 ++++++++++++++++++++++++++++++++++\n>  src/android/camera_device.h   |  12 ++\n>  2 files changed, 236 insertions(+)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index b30263451b76..cb9e4a6bc15b 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -8,6 +8,8 @@\n>  #include \"camera_device.h\"\n>  #include \"camera_ops.h\"\n>  \n> +#include <vector>\n> +\n>  #include <libcamera/controls.h>\n>  #include <libcamera/property_ids.h>\n>  \n> @@ -15,9 +17,75 @@\n>  #include \"libcamera/internal/utils.h\"\n>  \n>  #include \"camera_metadata.h\"\n> +#include \"system/graphics.h\"\n>  \n>  using namespace libcamera;\n>  \n> +namespace {\n> +\n> +/*\n> + * \\var camera3Resolutions\n> + * \\brief The list of image resolutions defined as mandatory to be supported by\n> + * the Android Camera3 specification\n> + */\n> +const std::vector<Size> camera3Resolutions = {\n> +\t{ 320, 240 },\n> +\t{ 640, 480 },\n> +\t{ 1280, 720 },\n> +\t{ 1920, 1080 }\n> +};\n> +\n> +/*\n> + * \\struct Camera3Format\n> + * \\brief Data associated with an Android format identifier\n> + * \\var libcameraFormats List of libcamera pixel formats compatible with the\n> + * Android format\n> + * \\var scalerFormat The format identifier to be reported to the android\n> + * framework through the static format configuration map\n> + * \\var formatName The human-readable representation of the Android format code\n> + */\n> +struct Camera3Format {\n> +\tstd::vector<PixelFormat> libcameraFormats;\n> +\tcamera_metadata_enum_android_scaler_available_formats_t scalerFormat;\n> +\tconst char *formatName;\n\nI would name this field \"name\" as the struct already qualifies it with\n\"format\".\n\n> +};\n> +\n> +/*\n> + * \\var camera3FormatsMap\n> + * \\brief Associate Android format code with ancillary data\n> + */\n> +const std::map<int, const Camera3Format> camera3FormatsMap = {\n> +\t{\n> +\t\tHAL_PIXEL_FORMAT_BLOB, {\n> +\t\t\t{ PixelFormat(DRM_FORMAT_MJPEG) },\n> +\t\t\tANDROID_SCALER_AVAILABLE_FORMATS_BLOB,\n> +\t\t\t\"HAL_PIXEL_FORMAT_BLOB\"\n\nCould we shorten the names by removing the HAL_PIXEL_FORMAT_ prefix ?\nIt's only used in a log message where it may be a bit long.\n\n> +\t\t}\n> +\t},\n> +\n> +\t{\n\nHow about\n\n\t}, {\n\nas we often do elsewhere ?\n\n> +\t\tHAL_PIXEL_FORMAT_YCbCr_420_888, {\n> +\t\t\t{ PixelFormat(DRM_FORMAT_NV12), PixelFormat(DRM_FORMAT_NV21) },\n> +\t\t\tANDROID_SCALER_AVAILABLE_FORMATS_YCbCr_420_888,\n> +\t\t\t\"HAL_PIXEL_FORMAT_YCbCr_420_888\"\n> +\t\t}\n> +\t},\n> +\n> +\t/*\n> +\t * \\todo Translate IMPLEMENTATION_DEFINED inspecting the\n> +\t * gralloc usage flag. For now, copy the YCbCr_420 configuration.\n> +\t */\n> +\t{\n\n\t}, {\n\t\t/*\n\t\t * \\todo Translate IMPLEMENTATION_DEFINED inspecting the\n\t\t * gralloc usage flag. For now, copy the YCbCr_420\n\t\t * configuration.\n\t\t */\n\n> +\t\tHAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED, {\n> +\t\t\t{ PixelFormat(DRM_FORMAT_NV12), PixelFormat(DRM_FORMAT_NV21) },\n> +\t\t\tANDROID_SCALER_AVAILABLE_FORMATS_IMPLEMENTATION_DEFINED,\n> +\t\t\t\"HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED\"\n> +\t\t}\n> +\t},\n> +};\n> +\n> +} /* namespace */\n> +\n>  LOG_DECLARE_CATEGORY(HAL);\n>  \n>  /*\n> @@ -100,6 +168,162 @@ int CameraDevice::initialize()\n>  \tif (properties.contains(properties::Rotation))\n>  \t\torientation_ = properties.get(properties::Rotation);\n>  \n> +\tint ret = camera_->acquire();\n> +\tif (ret) {\n> +\t\tLOG(HAL, Error) << \"Failed to temporarily acquire the camera\";\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tret = initializeStreamConfigurations();\n> +\tcamera_->release();\n> +\treturn ret;\n> +}\n> +\n> +/*\n> + * Initialize the format conversion map to translate from Android format\n> + * identifier to libcamera pixel formats and fill in the list of supported\n> + * stream configurations to be reported to the Android camera framework through\n> + * the static stream configuration metadata.\n> + */\n> +int CameraDevice::initializeStreamConfigurations()\n> +{\n> +\t/*\n> +\t * Get the maximum output resolutions\n> +\t * \\todo Get this from the camera properties once defined\n> +\t */\n> +\tstd::unique_ptr<CameraConfiguration> cameraConfig =\n> +\t\tcamera_->generateConfiguration({ StillCapture });\n> +\tif (!cameraConfig) {\n> +\t\tLOG(HAL, Error) << \"Failed to get maximum resolution\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\tStreamConfiguration &cfg = cameraConfig->at(0);\n> +\n> +\t/*\n> +\t * \\todo JPEG - Adjust the maximum available resolution by taking the\n> +\t * JPEG encoder requirements into account (alignment and aspect ratio).\n> +\t */\n> +\tconst Size maxRes = cfg.size;\n> +\tLOG(HAL, Debug) << \"Maximum supported resolution: \" << maxRes.toString();\n> +\n> +\t/*\n> +\t * Build the list of supported image resolutions.\n> +\t *\n> +\t * The resolutions listed in camera3Resolution are mandatory to be\n> +\t * supported, up to the camera maximum resolution.\n> +\t *\n> +\t * Augment the list by adding resolutions calculated from the camera\n> +\t * maximum one.\n> +\t */\n> +\tstd::vector<Size> cameraResolutions;\n> +\tstd::copy_if(camera3Resolutions.begin(), camera3Resolutions.end(),\n> +\t\t     std::back_inserter(cameraResolutions),\n> +\t\t     [&](const Size &res) { return res < maxRes; });\n> +\n> +\t/* Camera3 specification suggests to add 1/2 and 1/4 max resolution. */\n\ns/Camera3/The Camera3/\ns/suggests to add/suggests adding/\ns/max resolution/of the maximum resolution/\n\n> +\tfor (unsigned int divider = 2;; divider <<= 1) {\n> +\t\tSize derivedSize{\n> +\t\t\tmaxRes.width / divider,\n> +\t\t\tmaxRes.height / divider,\n> +\t\t};\n> +\n> +\t\tif (derivedSize.width < 320 ||\n> +\t\t    derivedSize.height < 240)\n> +\t\t\tbreak;\n> +\n> +\t\tcameraResolutions.push_back(derivedSize);\n> +\t}\n> +\tcameraResolutions.push_back(maxRes);\n> +\n> +\t/* Remove duplicated entries from the list of supported resolutions. */\n> +\tstd::sort(cameraResolutions.begin(), cameraResolutions.end());\n> +\tauto last = std::unique(cameraResolutions.begin(), cameraResolutions.end());\n> +\tcameraResolutions.erase(last, cameraResolutions.end());\n> +\n> +\t/*\n> +\t * Build the list of supported camera formats.\n> +\t *\n> +\t * To each Android format a list of compatible libcamera formats is\n> +\t * associated. The first libcamera format that tests successful is added\n> +\t * to the format translation map used when configuring the streams.\n> +\t * It is then tested against the list of supported camera resolutions to\n> +\t * build the stream configuration map reported through the camera static\n> +\t * metadata.\n> +\t */\n> +\tfor (const auto &format : camera3FormatsMap) {\n> +\t\tint androidFormat = format.first;\n> +\t\tconst Camera3Format &camera3Format = format.second;\n> +\t\tconst std::vector<PixelFormat> &libcameraFormats =\n> +\t\t\tcamera3Format.libcameraFormats;\n> +\n> +\t\t/*\n> +\t\t * Test the libcamera formats that can produce images\n> +\t\t * compatible with the Android defined format.\n\ns/Android defined/Android-defined/ (or simply \"with the format defined\nby Android).\n\n> +\t\t */\n> +\t\tPixelFormat mappedFormat{};\n\nThe {} are not needed, PixelFormat has a default constructor.\n\nReally nice work.\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> +\t\tfor (const PixelFormat &pixelFormat : libcameraFormats) {\n> +\t\t\t/* \\todo Fixed mapping for JPEG. */\n> +\t\t\tif (androidFormat == HAL_PIXEL_FORMAT_BLOB) {\n> +\t\t\t\tmappedFormat = PixelFormat(DRM_FORMAT_MJPEG);\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\n> +\t\t\t/*\n> +\t\t\t * The stream configuration size can be adjusted,\n> +\t\t\t * not the pixel format.\n> +\t\t\t *\n> +\t\t\t * \\todo This could be simplified once all pipeline\n> +\t\t\t * handlers will report the StreamFormats list of\n> +\t\t\t * supported formats.\n> +\t\t\t */\n> +\t\t\tcfg.pixelFormat = pixelFormat;\n> +\n> +\t\t\tCameraConfiguration::Status status = cameraConfig->validate();\n> +\t\t\tif (status != CameraConfiguration::Invalid &&\n> +\t\t\t    cfg.pixelFormat == pixelFormat) {\n> +\t\t\t\tmappedFormat = pixelFormat;\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\t\t}\n> +\t\tif (!mappedFormat.isValid()) {\n> +\t\t\tLOG(HAL, Error) << \"Failed to map Android format \"\n> +\t\t\t\t\t<< camera3Format.formatName << \" (\"\n> +\t\t\t\t\t<< utils::hex(androidFormat) << \")\";\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\n> +\t\t/*\n> +\t\t * Record the mapping and then proceed to generate the\n> +\t\t * stream configurations map, by testing the image resolutions.\n> +\t\t */\n> +\t\tformatsMap_[androidFormat] = mappedFormat;\n> +\n> +\t\tfor (const Size &res : cameraResolutions) {\n> +\t\t\tcfg.pixelFormat = mappedFormat;\n> +\t\t\tcfg.size = res;\n> +\n> +\t\t\tCameraConfiguration::Status status = cameraConfig->validate();\n> +\t\t\t/*\n> +\t\t\t * Unconditionally report we can produce JPEG.\n> +\t\t\t *\n> +\t\t\t * \\todo The JPEG stream will be implemented as an\n> +\t\t\t * HAL-only stream, but some cameras can produce it\n> +\t\t\t * directly. As of now, claim support for JPEG without\n> +\t\t\t * inspecting where the JPEG stream is produced.\n> +\t\t\t */\n> +\t\t\tif (androidFormat != HAL_PIXEL_FORMAT_BLOB &&\n> +\t\t\t    status != CameraConfiguration::Valid)\n> +\t\t\t\tcontinue;\n> +\n> +\t\t\tstreamConfigurations_.push_back({ res, camera3Format.scalerFormat });\n> +\t\t}\n> +\t}\n> +\n> +\tLOG(HAL, Debug) << \"Collected stream configuration map: \";\n> +\tfor (const auto &entry : streamConfigurations_)\n> +\t\tLOG(HAL, Debug) << \"{ \" << entry.resolution.toString() << \" - \"\n> +\t\t\t\t<< utils::hex(entry.androidScalerCode) << \" }\";\n> +\n>  \treturn 0;\n>  }\n>  \n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index a87f7623c790..4e8911da5770 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -7,12 +7,15 @@\n>  #ifndef __ANDROID_CAMERA_DEVICE_H__\n>  #define __ANDROID_CAMERA_DEVICE_H__\n>  \n> +#include <map>\n>  #include <memory>\n> +#include <vector>\n>  \n>  #include <hardware/camera3.h>\n>  \n>  #include <libcamera/buffer.h>\n>  #include <libcamera/camera.h>\n> +#include <libcamera/geometry.h>\n>  #include <libcamera/request.h>\n>  #include <libcamera/stream.h>\n>  \n> @@ -59,6 +62,12 @@ private:\n>  \t\tcamera3_stream_buffer_t *buffers;\n>  \t};\n>  \n> +\tstruct Camera3StreamConfiguration {\n> +\t\tlibcamera::Size resolution;\n> +\t\tint androidScalerCode;\n> +\t};\n> +\n> +\tint initializeStreamConfigurations();\n>  \tvoid notifyShutter(uint32_t frameNumber, uint64_t timestamp);\n>  \tvoid notifyError(uint32_t frameNumber, camera3_stream_t *stream);\n>  \tstd::unique_ptr<CameraMetadata> getResultMetadata(int frame_number,\n> @@ -75,6 +84,9 @@ private:\n>  \tstd::map<unsigned int, CameraMetadata *> requestTemplates_;\n>  \tconst camera3_callback_ops_t *callbacks_;\n>  \n> +\tstd::vector<Camera3StreamConfiguration> streamConfigurations_;\n> +\tstd::map<int, libcamera::PixelFormat> formatsMap_;\n> +\n>  \tint facing_;\n>  \tint orientation_;\n>  };","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4F4FA600F7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat,  6 Jun 2020 02:01:23 +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 8DBAD27C;\n\tSat,  6 Jun 2020 02:01:22 +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=\"cGjqdrZ5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1591401682;\n\tbh=1kRuVf0QA5ZTmqdlDtq9DEMqLJhxCA7nypgb84IrRig=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cGjqdrZ5Ht3qrOc/Gvl4CmY6h4x0L7H4q5D2FaLIQ5nP3V1ZDwDuwNGCHlPMRY2mX\n\t3FRVRJJlRhBFRPgiGd0w6m2fhFQAUv8JUbV7kDgbYn8ZEerNhwuHv6jto1OmXZukxH\n\tKaW6ZB/t1W1rnu97jBxHF1aqMQSRYe+u5FUpI4nI=","Date":"Sat, 6 Jun 2020 03:01:03 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200606000103.GU26752@pendragon.ideasonboard.com>","References":"<20200605141002.49119-1-jacopo@jmondi.org>\n\t<20200605141002.49119-2-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200605141002.49119-2-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v3 1/8] android: camera_device:\n\tInitialize 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":"Sat, 06 Jun 2020 00:01:23 -0000"}}]