{"id":9471,"url":"https://patchwork.libcamera.org/api/patches/9471/?format=json","web_url":"https://patchwork.libcamera.org/patch/9471/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20200902152236.69770-11-jacopo@jmondi.org>","date":"2020-09-02T15:22:34","name":"[libcamera-devel,v2,10/12] android: camera_device: Rework CameraStream handling","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"a8264627db212d318c5f6842f7133b39bd7e1c68","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/?format=json","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/9471/mbox/","series":[{"id":1259,"url":"https://patchwork.libcamera.org/api/series/1259/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=1259","date":"2020-09-02T15:22:24","name":"android: camera_device: Fix JPEG/RAW sizes","version":2,"mbox":"https://patchwork.libcamera.org/series/1259/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/9471/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/9471/checks/","tags":{},"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 777E6BE174\n\tfor ;\n\tWed, 2 Sep 2020 15:19:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4CD4F629DF;\n\tWed, 2 Sep 2020 17:19:11 +0200 (CEST)","from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6F5C560374\n\tfor ;\n\tWed, 2 Sep 2020 17:19:10 +0200 (CEST)","from uno.lan (93-34-118-233.ip49.fastwebnet.it [93.34.118.233])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 4F635240017;\n\tWed, 2 Sep 2020 15:19:09 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","From":"Jacopo Mondi ","To":"libcamera-devel@lists.libcamera.org","Date":"Wed, 2 Sep 2020 17:22:34 +0200","Message-Id":"<20200902152236.69770-11-jacopo@jmondi.org>","X-Mailer":"git-send-email 2.28.0","In-Reply-To":"<20200902152236.69770-1-jacopo@jmondi.org>","References":"<20200902152236.69770-1-jacopo@jmondi.org>","MIME-Version":"1.0","Subject":"[libcamera-devel] [PATCH v2 10/12] android: camera_device: Rework\n\tCameraStream handling","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":"tfiga@google.com, hiroh@google.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"The CameraDevice::streams_ vector of CameraStream instances is\ncurrently mostly accessed by index. The current implementation\ncreates all the CameraStream during the first loop that inspects the\ncamera3_stream instances, and the update the index of the\nStreamConfiguration associated with the CameraStream during a second\nloop that inspects MJPEG streams. A third loop creates the JPEG encoder\nassociated with CameraStreams that produce MJPEG format.\n\nAs the index-based association is hard to follow and rather fragile,\nrework the creation and handling of CameraStream:\n\n1) Make the StreamConfiguration index a constructor parameter and a\n private struct member. This disallow the creation of CameraStream\n without a StreamConfiguration index assigned.\n\n2) Create CameraStream only after the associated StreamConfiguration\n has been identified. The first loop creates CameraStream for non-JPEG\n streams, the second for the JPEG ones after having identified the\n associated StreamConfiguration. Since we have just created the\n CameraStream, create the JPEG encoder at the same time instead of\n deferring it.\n\nThis change removes all accesses by index to the CameraDevice::streams_\nvector.\n\nNo functional changes intended, but this change aims to make the code\neasier to follow and more robust.\n\nReviewed-by: Kieran Bingham \nSigned-off-by: Jacopo Mondi \n---\n src/android/camera_device.cpp | 75 +++++++++++++++++++----------------\n src/android/camera_device.h | 18 +++++----\n 2 files changed, 51 insertions(+), 42 deletions(-)","diff":"diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\nindex 3630e87e8814..9bcd1d993c17 100644\n--- a/src/android/camera_device.cpp\n+++ b/src/android/camera_device.cpp\n@@ -168,8 +168,8 @@ MappedCamera3Buffer::MappedCamera3Buffer(const buffer_handle_t camera3buffer,\n \t}\n }\n \n-CameraStream::CameraStream(PixelFormat f, Size s)\n-\t: index(-1), format(f), size(s), jpeg(nullptr)\n+CameraStream::CameraStream(PixelFormat f, Size s, unsigned int i)\n+\t: format(f), size(s), jpeg(nullptr), index_(i)\n {\n }\n \n@@ -1212,30 +1212,28 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n \t\tif (!format.isValid())\n \t\t\treturn -EINVAL;\n \n-\t\tstreams_.emplace_back(format, size);\n-\t\tstream->priv = static_cast(&streams_[i]);\n-\n \t\t/* Defer handling of MJPEG streams until all others are known. */\n \t\tif (stream->format == HAL_PIXEL_FORMAT_BLOB)\n \t\t\tcontinue;\n \n \t\tStreamConfiguration streamConfiguration;\n-\n \t\tstreamConfiguration.size = size;\n \t\tstreamConfiguration.pixelFormat = format;\n \n-\t\tstreams_[i].index = config_->addConfiguration(streamConfiguration);\n+\t\tunsigned int index = config_->addConfiguration(streamConfiguration);\n+\t\tCameraStream &cameraStream = streams_.emplace_back(format, size, index);\n+\t\tstream->priv = static_cast(&cameraStream);\n \t}\n \n \t/* Now handle MJPEG streams, adding a new stream if required. */\n \tfor (unsigned int i = 0; i < stream_list->num_streams; ++i) {\n \t\tcamera3_stream_t *stream = stream_list->streams[i];\n-\t\tbool match = false;\n-\n \t\tif (stream->format != HAL_PIXEL_FORMAT_BLOB)\n \t\t\tcontinue;\n \n-\t\t/* Search for a compatible stream */\n+\t\t/* Search for a compatible stream in the non-JPEG ones. */\n+\t\tbool match = false;\n+\t\tunsigned int index;\n \t\tfor (unsigned int j = 0; j < config_->size(); j++) {\n \t\t\tStreamConfiguration &cfg = config_->at(j);\n \n@@ -1243,13 +1241,15 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n \t\t\t * \\todo The PixelFormat must also be compatible with\n \t\t\t * the encoder.\n \t\t\t */\n-\t\t\tif (cfg.size == streams_[i].size) {\n-\t\t\t\tLOG(HAL, Info) << \"Stream \" << i\n-\t\t\t\t\t << \" using libcamera stream \" << j;\n+\t\t\tif (cfg.size.width != stream->width ||\n+\t\t\t cfg.size.height != stream->height)\n+\t\t\t\tcontinue;\n \n-\t\t\t\tmatch = true;\n-\t\t\t\tstreams_[i].index = j;\n-\t\t\t}\n+\t\t\tLOG(HAL, Info) << \"Stream \" << i\n+\t\t\t\t << \" using libcamera stream \" << j;\n+\n+\t\t\tindex = j;\n+\t\t\tmatch = true;\n \t\t}\n \n \t\t/*\n@@ -1272,7 +1272,26 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n \t\t\tLOG(HAL, Info) << \"Adding \" << streamConfiguration.toString()\n \t\t\t\t << \" for MJPEG support\";\n \n-\t\t\tstreams_[i].index = config_->addConfiguration(streamConfiguration);\n+\t\t\tindex = config_->addConfiguration(streamConfiguration);\n+\t\t}\n+\n+\t\tStreamConfiguration &cfg = config_->at(index);\n+\t\tPixelFormat format = formats::MJPEG;\n+\t\tSize size = cfg.size;\n+\n+\t\tCameraStream &cameraStream = streams_.emplace_back(format, size, index);\n+\t\tstream->priv = static_cast(&cameraStream);\n+\n+\t\t/*\n+\t\t * Construct a software encoder for MJPEG streams from the\n+\t\t * chosen libcamera source stream.\n+\t\t */\n+\t\tcameraStream.jpeg = new EncoderLibJpeg();\n+\t\tint ret = cameraStream.jpeg->configure(cfg);\n+\t\tif (ret) {\n+\t\t\tLOG(HAL, Error)\n+\t\t\t\t<< \"Failed to configure encoder\";\n+\t\t\treturn ret;\n \t\t}\n \t}\n \n@@ -1295,25 +1314,11 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n \n \tfor (unsigned int i = 0; i < stream_list->num_streams; ++i) {\n \t\tcamera3_stream_t *stream = stream_list->streams[i];\n-\t\tCameraStream *cameraStream = &streams_[i];\n-\t\tStreamConfiguration &cfg = config_->at(cameraStream->index);\n+\t\tCameraStream *cameraStream = static_cast(stream->priv);\n+\t\tStreamConfiguration &cfg = config_->at(cameraStream->index());\n \n \t\t/* Use the bufferCount confirmed by the validation process. */\n \t\tstream->max_buffers = cfg.bufferCount;\n-\n-\t\t/*\n-\t\t * Construct a software encoder for MJPEG streams from the\n-\t\t * chosen libcamera source stream.\n-\t\t */\n-\t\tif (cameraStream->format == formats::MJPEG) {\n-\t\t\tcameraStream->jpeg = new EncoderLibJpeg();\n-\t\t\tint ret = cameraStream->jpeg->configure(cfg);\n-\t\t\tif (ret) {\n-\t\t\t\tLOG(HAL, Error)\n-\t\t\t\t\t<< \"Failed to configure encoder\";\n-\t\t\t\treturn ret;\n-\t\t\t}\n-\t\t}\n \t}\n \n \t/*\n@@ -1427,7 +1432,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n \t\t}\n \t\tdescriptor->frameBuffers.emplace_back(buffer);\n \n-\t\tStreamConfiguration *streamConfiguration = &config_->at(cameraStream->index);\n+\t\tStreamConfiguration *streamConfiguration = &config_->at(cameraStream->index());\n \t\tStream *stream = streamConfiguration->stream();\n \n \t\trequest->addBuffer(stream, buffer);\n@@ -1482,7 +1487,7 @@ void CameraDevice::requestComplete(Request *request)\n \t\t\tcontinue;\n \t\t}\n \n-\t\tStreamConfiguration *streamConfiguration = &config_->at(cameraStream->index);\n+\t\tStreamConfiguration *streamConfiguration = &config_->at(cameraStream->index());\n \t\tStream *stream = streamConfiguration->stream();\n \t\tFrameBuffer *buffer = request->findBuffer(stream);\n \t\tif (!buffer) {\ndiff --git a/src/android/camera_device.h b/src/android/camera_device.h\nindex dc0ee664d443..f8f237203ce9 100644\n--- a/src/android/camera_device.h\n+++ b/src/android/camera_device.h\n@@ -28,20 +28,24 @@\n class CameraMetadata;\n \n struct CameraStream {\n-\tCameraStream(libcamera::PixelFormat, libcamera::Size);\n+public:\n+\tCameraStream(libcamera::PixelFormat, libcamera::Size, unsigned int i);\n \t~CameraStream();\n \n-\t/*\n-\t * The index of the libcamera StreamConfiguration as added during\n-\t * configureStreams(). A single libcamera Stream may be used to deliver\n-\t * one or more streams to the Android framework.\n-\t */\n-\tunsigned int index;\n+\tunsigned int index() const { return index_; }\n \n \tlibcamera::PixelFormat format;\n \tlibcamera::Size size;\n \n \tEncoder *jpeg;\n+\n+private:\n+\t/*\n+\t * The index of the libcamera StreamConfiguration as added during\n+\t * configureStreams(). A single libcamera Stream may be used to deliver\n+\t * one or more streams to the Android framework.\n+\t */\n+\tunsigned int index_;\n };\n \n class CameraDevice : protected libcamera::Loggable\n","prefixes":["libcamera-devel","v2","10/12"]}