[{"id":11837,"web_url":"https://patchwork.libcamera.org/comment/11837/","msgid":"<20200804150442.socusctwobbbnqxs@uno.localdomain>","date":"2020-08-04T15:04:42","subject":"Re: [libcamera-devel] [PATCH v2 12/12] android: camera_device:\n\tSupport MJPEG stream construction","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran,\n\nOn Mon, Aug 03, 2020 at 05:18:16PM +0100, Kieran Bingham wrote:\n> MJPEG streams must be created referencing a libcamera stream.\n> This stream may already be provided by the request configuration,\n> in which case the existing stream is utilised.\n>\n> If no compatible stream is available to encode, a new stream is requested\n> from the libcamera configuration.\n>\n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n>\n> This is probably the main patch holding me up at the moment, as there\n> are just so many bits I'm not happy with.\n>\n>  - Clean up the \"Fill in the JPEG blob header.\"\n>  - Added streams for different MPEG requirements don't have their own\n>    buffers (only just realised that today, so I think we should just\n>    pull that out for a first integration)\n>  - JPEG compression should run in a thread.\n>  - I really dislike the current mapping between camera3_stream to\n>    libcamera stream with the index\n>  - The JPEG compressor is not currently destructed.\n>    I was going to handle that by moving the CameraStream to a class\n>    which would then destroy the compressor in it's destructor - but that\n>    hasn't gone down that path, maybe it still should.\n>  - Metadata additions are horrible to keep in sync (especially through\n>    rebases)\n>  - More stuff ;-)\n>\n> Anyway, here's the current WIP - and it works ... so hey that's a bonus\n>   ;-)\n>\n>\n>\n>\n>  src/android/camera_device.cpp | 202 +++++++++++++++++++++++++++++++++-\n>  src/android/camera_device.h   |   8 ++\n>  2 files changed, 204 insertions(+), 6 deletions(-)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index e23ab055d012..42b08cfc5fed 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -8,6 +8,7 @@\n>  #include \"camera_device.h\"\n>  #include \"camera_ops.h\"\n>\n> +#include <sys/mman.h>\n>  #include <tuple>\n>  #include <vector>\n>\n> @@ -21,6 +22,8 @@\n>  #include \"camera_metadata.h\"\n>  #include \"system/graphics.h\"\n>\n> +#include \"jpeg/compressor_jpeg.h\"\n> +\n>  using namespace libcamera;\n>\n>  namespace {\n> @@ -80,6 +83,40 @@ const std::map<int, const Camera3Format> camera3FormatsMap = {\n>\n>  LOG_DECLARE_CATEGORY(HAL);\n>\n> +class MappedCamera3Buffer : public MappedBuffer\n> +{\n> +public:\n> +\tMappedCamera3Buffer(const buffer_handle_t camera3buffer, int flags)\n> +\t{\n\nIsn't this a bit long to be inlined ? Or does inlining just happens if\nthe method is implemented in the class definition in an header file ?\nI don't think so, as per the compiler perspective they're equivalent.\n\nAnd I would anyway place the implementation outside of the class\ndefinition to save and indentation level, inlining apart.\n\n> +\t\tmaps_.reserve(camera3buffer->numFds);\n> +\t\terror_ = 0;\n> +\n> +\t\tfor (int i = 0; i < camera3buffer->numFds; i++) {\n> +\t\t\tif (camera3buffer->data[i] == -1)\n> +\t\t\t\tcontinue;\n> +\n> +\t\t\toff_t length = lseek(camera3buffer->data[i], 0, SEEK_END);\n> +\t\t\tif (length < 0) {\n> +\t\t\t\terror_ = errno;\n> +\t\t\t\tLOG(HAL, Error) << \"Failed to query plane length\";\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\n> +\t\t\tvoid *address = mmap(nullptr, length, flags, MAP_SHARED,\n> +\t\t\t\t\t     camera3buffer->data[i], 0);\n> +\t\t\tif (address == MAP_FAILED) {\n> +\t\t\t\terror_ = errno;\n> +\t\t\t\tLOG(HAL, Error) << \"Failed to mmap plane\";\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\n> +\t\t\tmaps_.emplace_back(static_cast<uint8_t *>(address), static_cast<size_t>(length));\n> +\t\t}\n> +\n> +\t\tvalid_ = error_ == 0;\n> +\t}\n> +};\n> +\n>  /*\n>   * \\struct Camera3RequestDescriptor\n>   *\n> @@ -368,9 +405,9 @@ std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize()\n>  {\n>  \t/*\n>  \t * \\todo Keep this in sync with the actual number of entries.\n> -\t * Currently: 50 entries, 647 bytes of static metadata\n> +\t * Currently: 51 entries, 651 bytes of static metadata\n>  \t */\n> -\tuint32_t numEntries = 50;\n> +\tuint32_t numEntries = 51;\n>  \tuint32_t byteSize = 651;\n\nThe comment was clearly wrong, you should increase the actual size to\n667. You need 16 bytes more for:\n- The uint32_t transported by ANDROID_JPEG_MAX_SIZE\n- The three int32_t tags you add to availableResultKeys (see below)\n>\n>  \t/*\n> @@ -527,6 +564,9 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n>  \t\t\t\t  availableThumbnailSizes.data(),\n>  \t\t\t\t  availableThumbnailSizes.size());\n>\n> +\tint32_t jpegMaxSize = 13 << 20; /* 13631488 from USB HAL */\n\nThat's worth a\n        \\todo Calculate the maximum JPEG buffer size by inspecting the\n        largest image size and the jpeg compressor requirements\n\nor something similar, as I'm not sure the \"compressor requirements\"\nmake sense or not.\n\n> +\tstaticMetadata_->addEntry(ANDROID_JPEG_MAX_SIZE, &jpegMaxSize, 1);\n> +\n>  \t/* Sensor static metadata. */\n>  \tint32_t pixelArraySize[] = {\n>  \t\t2592, 1944,\n> @@ -729,6 +769,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n>  \t\tANDROID_CONTROL_AWB_LOCK_AVAILABLE,\n>  \t\tANDROID_CONTROL_AVAILABLE_MODES,\n>  \t\tANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES,\n> +\t\tANDROID_JPEG_MAX_SIZE,\n>  \t\tANDROID_SENSOR_INFO_PIXEL_ARRAY_SIZE,\n>  \t\tANDROID_SENSOR_INFO_ACTIVE_ARRAY_SIZE,\n>  \t\tANDROID_SENSOR_INFO_SENSITIVITY_RANGE,\n> @@ -795,6 +836,9 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n>  \t\tANDROID_SENSOR_EXPOSURE_TIME,\n>  \t\tANDROID_STATISTICS_LENS_SHADING_MAP_MODE,\n>  \t\tANDROID_STATISTICS_SCENE_FLICKER,\n> +\t\tANDROID_JPEG_SIZE,\n> +\t\tANDROID_JPEG_QUALITY,\n> +\t\tANDROID_JPEG_ORIENTATION,\n\nYou should add the size of 3 more integers to the static metadata pack\nsize if you add these here. (now commented above too)\n\n>  \t};\n>  \tstaticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_RESULT_KEYS,\n>  \t\t\t\t  availableResultKeys.data(),\n> @@ -956,6 +1000,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>  \t */\n>  \tunsigned int streamIndex = 0;\n>\n> +\t/* First handle all non-MJPEG streams */\n>  \tfor (unsigned int i = 0; i < stream_list->num_streams; ++i) {\n>  \t\tcamera3_stream_t *stream = stream_list->streams[i];\n>\n> @@ -971,6 +1016,18 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>  \t\tif (!format.isValid())\n>  \t\t\treturn -EINVAL;\n>\n> +\t\tstream->priv = static_cast<void *>(&streams_[i]);\n> +\t\tstreams_[i].format = format;\n\nYou are still not pushing back or emplacing a CameraStream to the\nvector but just using the there reserved memory. We have taken this\nextensively offline I'll just record it here :0\n\n\n> +\t\tstreams_[i].size = Size(stream->width, stream->height);\n\nI still find all the 'stream/streams/streams_' occurences confusing,\nbut the patch that adds streams_ and CameraStream is merged, so a\nrename would require a patch before this one :) too much churn and\nit's possibly better to do that once this is merged.\n\n> +\n> +\t\t/* Defer handling of MJPEG streams until all others are known. */\n> +\t\tif (format == formats::MJPEG) {\n> +\t\t\tLOG(HAL, Info) << \"Handling MJPEG stream\";\n> +\n> +\t\t\tstreams_[i].index = -1;\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n>  \t\tStreamConfiguration streamConfiguration;\n>\n>  \t\tstreamConfiguration.size.width = stream->width;\n> @@ -980,7 +1037,61 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>  \t\tconfig_->addConfiguration(streamConfiguration);\n>\n>  \t\tstreams_[i].index = streamIndex++;\n> -\t\tstream->priv = static_cast<void *>(&streams_[i]);\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\tCameraStream *cameraStream = &streams_[i];\n> +\n> +\t\t/* Only process MJPEG streams */\n> +\t\tif (cameraStream->format != formats::MJPEG)\n> +\t\t\tcontinue;\n> +\n> +\t\tbool match = false;\n> +\n> +\t\t/* Search for a compatible stream */\n> +\t\tfor (unsigned int j = 0; j < config_->size(); j++) {\n> +\t\t\tStreamConfiguration &cfg = config_->at(j);\n> +\n> +\t\t\t/*\n> +\t\t\t * The PixelFormat must also be compatible with the\n> +\t\t\t * encoder.\n> +\t\t\t */\n> +\t\t\tif (cfg.size == cameraStream->size) {\n> +\t\t\t\tLOG(HAL, Info)\n> +\t\t\t\t\t<< \"Stream \" << i\n> +\t\t\t\t\t<< \" using libcamera stream \"\n> +\t\t\t\t\t<< j;\n\nWon't this fit in 2 lines ?\n\t\t\t\tLOG(HAL, Info) << \"Stream \" << i\n\t\t\t\t\t       << \" using libcamera stream \" << j;\n\n> +\n> +\t\t\t\tmatch = true;\n> +\t\t\t\tcameraStream->index = j;\n> +\t\t\t}\n> +\t\t}\n> +\n> +\t\t/*\n> +\t\t * Without a compatible match for JPEG encoding we must\n> +\t\t * introduce a new stream to satisfy the request requirements.\n> +\t\t */\n> +\t\tif (!match) {\n> +\t\t\tStreamConfiguration streamConfiguration;\n> +\n> +\t\t\t/*\n> +\t\t\t * \\todo: The pixelFormat should be a 'best-fit' choice\n> +\t\t\t * and may require a validation cycle. This is not yet\n> +\t\t\t * handled, and should be considered as part of any\n> +\t\t\t * stream configuration reworks.\n> +\t\t\t */\n> +\t\t\tstreamConfiguration.size.width = stream->width;\n> +\t\t\tstreamConfiguration.size.height = stream->height;\n> +\t\t\tstreamConfiguration.pixelFormat = formats::NV12;\n> +\n> +\t\t\tLOG(HAL, Info) << \"Adding \" << streamConfiguration.toString()\n> +\t\t\t\t       << \" for MJPEG support\";\n> +\n> +\t\t\tconfig_->addConfiguration(streamConfiguration);\n> +\t\t\tstreams_[i].index = streamIndex++;\n> +\t\t}\n>  \t}\n>\n>  \tswitch (config_->validate()) {\n> @@ -1007,6 +1118,18 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\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 compressor 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 CompressorJPEG();\n> +\t\t\tcameraStream->jpeg->configure(cfg);\n\nJust recording what we've discussed off-line: this should be deleted\nwhen the CameraStream instance is deleted. As the CameraStream\ninstances are stored in the stream_ vector, clearing it at next\nconfiguration request and camera close time should be enough.\n\n> +\t\t} else {\n> +\t\t\t/* Either construct this correctly, or use a better interface */\n> +\t\t\tcameraStream->jpeg = nullptr;\n> +\t\t}\n>  \t}\n>\n>  \t/*\n> @@ -1101,6 +1224,10 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n>  \t\tdescriptor->buffers[i].stream = camera3Buffers[i].stream;\n>  \t\tdescriptor->buffers[i].buffer = camera3Buffers[i].buffer;\n>\n> +\t\t/* Software streams are handled after hardware streams complete. */\n> +\t\tif (cameraStream->format == formats::MJPEG)\n> +\t\t\tcontinue;\n> +\n>  \t\t/*\n>  \t\t * Create a libcamera buffer using the dmabuf descriptors of\n>  \t\t * the camera3Buffer for each stream. The FrameBuffer is\n> @@ -1157,9 +1284,72 @@ void CameraDevice::requestComplete(Request *request)\n>  \tresultMetadata = getResultMetadata(descriptor->frameNumber,\n>  \t\t\t\t\t   buffer->metadata().timestamp);\n>\n> -\t/* Prepare to call back the Android camera stack. */\n> +\t/* Handle any JPEG compression */\n> +\tfor (unsigned int i = 0; i < descriptor->numBuffers; ++i) {\n> +\t\tCameraStream *cameraStream =\n> +\t\t\tstatic_cast<CameraStream *>(descriptor->buffers[i].stream->priv);\n> +\t\tCompressor *compressor = cameraStream->jpeg;\n>\n> +\t\t/* Only handle streams with compression enabled. */\n> +\t\tif (!compressor)\n> +\t\t\tcontinue;\n\nI wonder if inspecting the format would avoid using a pointer as a\nflag. This is fine for now though.\n\n>\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) {\n> +\t\t\tLOG(HAL, Error) << \"Failed to find a source stream buffer\";\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\tMappedCamera3Buffer mapped(*descriptor->buffers[i].buffer, PROT_READ | PROT_WRITE);\n> +\t\tif (!mapped.isValid()) {\n> +\t\t\tLOG(HAL, Error) << \"Failed to mmap android blob buffer\";\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\tCompressedImage output;\n> +\t\toutput.data = static_cast<unsigned char *>(mapped.maps()[0].data());\n> +\t\toutput.length = mapped.maps()[0].size();\n> +\n> +\t\tint ret = compressor->compress(buffer, &output);\n> +\t\tif (ret) {\n> +\t\t\tLOG(HAL, Error) << \"Failed to compress stream image\";\n> +\t\t\tstatus = CAMERA3_BUFFER_STATUS_ERROR;\n> +\t\t}\n> +\n> +\t\t/* Fill in the JPEG blob header. */\n> +\t\t{\n> +\t\t\t/* The mapped size of the buffer is being returned as substantially larger\n> +\t\t\t * than the requested JPEG_MAX_SIZE (re-defined here).\n> +\t\t\t * Utilise this static size as a workaround to ensure the correct offset\n> +\t\t\t * of the blob is determined.\n> +\t\t\t *\n> +\t\t\t * \\todo Investigate and fix the root cause of this.\n> +\t\t\t */\n> +\t\t\tint32_t max_size = { 13 << 20 };\n> +\t\t\tuint8_t *resultPtr = static_cast<uint8_t *>(mapped.maps()[0].data()) +\n> +\t\t\t\t\t     max_size - sizeof(struct camera3_jpeg_blob);\n> +\t\t\tauto *blob = reinterpret_cast<struct camera3_jpeg_blob *>(resultPtr);\n> +\n> +\t\t\tblob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID;\n> +\t\t\tblob->jpeg_size = output.length;\n> +\t\t}\n\nWhy a separate scope block ? Not that it bothers me, just curious\n\n> +\n> +\t\t/* Update the JPEG result Metadata. */\n> +\t\tresultMetadata->addEntry(ANDROID_JPEG_SIZE,\n> +\t\t\t\t\t &output.length, 1);\n> +\n> +\t\tconst uint32_t jpeg_quality = 95;\n> +\t\tresultMetadata->addEntry(ANDROID_JPEG_QUALITY,\n> +\t\t\t\t\t &jpeg_quality, 1);\n> +\n> +\t\tconst uint32_t jpeg_orientation = 0;\n> +\t\tresultMetadata->addEntry(ANDROID_JPEG_ORIENTATION,\n> +\t\t\t\t\t &jpeg_orientation, 1);\n> +\t}\n> +\n> +\t/* Prepare to call back the Android camera stack. */\n>  \tcamera3_capture_result_t captureResult = {};\n>  \tcaptureResult.frame_number = descriptor->frameNumber;\n>  \tcaptureResult.num_output_buffers = descriptor->numBuffers;\n> @@ -1240,10 +1430,10 @@ std::unique_ptr<CameraMetadata> CameraDevice::getResultMetadata(int frame_number\n>  {\n>  \t/*\n>  \t * \\todo Keep this in sync with the actual number of entries.\n> -\t * Currently: 12 entries, 36 bytes\n> +\t * Currently: 17 entries, 58 bytes\n>  \t */\n>  \tstd::unique_ptr<CameraMetadata> resultMetadata =\n> -\t\tstd::make_unique<CameraMetadata>(15, 50);\n> +\t\tstd::make_unique<CameraMetadata>(17, 58);\n\nI see three entries more:\n        ANDROID_JPEG_SIZE, ANDROID_JPEG_QUALITY, ANDROID_JPEG_ORIENTATION\n\n>  \tif (!resultMetadata->isValid()) {\n>  \t\tLOG(HAL, Error) << \"Failed to allocate static metadata\";\n>  \t\treturn nullptr;\n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index 00472c219388..c9ece426a00e 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -23,6 +23,8 @@\n>  #include \"libcamera/internal/log.h\"\n>  #include \"libcamera/internal/message.h\"\n>\n> +#include \"jpeg/compressor_jpeg.h\"\n> +\n>  class CameraMetadata;\n>\n>  struct CameraStream {\n> @@ -32,6 +34,12 @@ struct CameraStream {\n>  \t * one or more streams to the Android framework.\n>  \t */\n>  \tunsigned int index;\n> +\n> +\tlibcamera::PixelFormat format;\n> +\tlibcamera::Size size;\n> +\n> +\t/* Make sure this gets destructed correctly */\n> +\tCompressorJPEG *jpeg;\n\nIn the struct or class destructor ?\n\nThanks\n  j\n\n>  };\n>\n>  class CameraDevice : protected libcamera::Loggable\n> --\n> 2.25.1\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 3C629BD87A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  4 Aug 2020 15:01:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A54686054A;\n\tTue,  4 Aug 2020 17:01:03 +0200 (CEST)","from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[217.70.183.197])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0163A60545\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  4 Aug 2020 17:01:01 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 233651C0005;\n\tTue,  4 Aug 2020 15:01:00 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Tue, 4 Aug 2020 17:04:42 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20200804150442.socusctwobbbnqxs@uno.localdomain>","References":"<20200803161816.107113-1-kieran.bingham@ideasonboard.com>\n\t<20200803161816.107113-13-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200803161816.107113-13-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 12/12] android: camera_device:\n\tSupport MJPEG stream construction","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>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11850,"web_url":"https://patchwork.libcamera.org/comment/11850/","msgid":"<3e867c49-ca88-d873-3e1e-6f3ffb520131@ideasonboard.com>","date":"2020-08-04T20:13:48","subject":"Re: [libcamera-devel] [PATCH v2 12/12] android: camera_device:\n\tSupport MJPEG stream construction","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 04/08/2020 16:04, Jacopo Mondi wrote:\n> Hi Kieran,\n> \n> On Mon, Aug 03, 2020 at 05:18:16PM +0100, Kieran Bingham wrote:\n>> MJPEG streams must be created referencing a libcamera stream.\n>> This stream may already be provided by the request configuration,\n>> in which case the existing stream is utilised.\n>>\n>> If no compatible stream is available to encode, a new stream is requested\n>> from the libcamera configuration.\n>>\n>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>> ---\n>>\n>> This is probably the main patch holding me up at the moment, as there\n>> are just so many bits I'm not happy with.\n>>\n>>  - Clean up the \"Fill in the JPEG blob header.\"\n>>  - Added streams for different MPEG requirements don't have their own\n>>    buffers (only just realised that today, so I think we should just\n>>    pull that out for a first integration)\n>>  - JPEG compression should run in a thread.\n>>  - I really dislike the current mapping between camera3_stream to\n>>    libcamera stream with the index\n>>  - The JPEG compressor is not currently destructed.\n>>    I was going to handle that by moving the CameraStream to a class\n>>    which would then destroy the compressor in it's destructor - but that\n>>    hasn't gone down that path, maybe it still should.\n>>  - Metadata additions are horrible to keep in sync (especially through\n>>    rebases)\n>>  - More stuff ;-)\n>>\n>> Anyway, here's the current WIP - and it works ... so hey that's a bonus\n>>   ;-)\n>>\n>>\n>>\n>>\n>>  src/android/camera_device.cpp | 202 +++++++++++++++++++++++++++++++++-\n>>  src/android/camera_device.h   |   8 ++\n>>  2 files changed, 204 insertions(+), 6 deletions(-)\n>>\n>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n>> index e23ab055d012..42b08cfc5fed 100644\n>> --- a/src/android/camera_device.cpp\n>> +++ b/src/android/camera_device.cpp\n>> @@ -8,6 +8,7 @@\n>>  #include \"camera_device.h\"\n>>  #include \"camera_ops.h\"\n>>\n>> +#include <sys/mman.h>\n>>  #include <tuple>\n>>  #include <vector>\n>>\n>> @@ -21,6 +22,8 @@\n>>  #include \"camera_metadata.h\"\n>>  #include \"system/graphics.h\"\n>>\n>> +#include \"jpeg/compressor_jpeg.h\"\n>> +\n>>  using namespace libcamera;\n>>\n>>  namespace {\n>> @@ -80,6 +83,40 @@ const std::map<int, const Camera3Format> camera3FormatsMap = {\n>>\n>>  LOG_DECLARE_CATEGORY(HAL);\n>>\n>> +class MappedCamera3Buffer : public MappedBuffer\n>> +{\n>> +public:\n>> +\tMappedCamera3Buffer(const buffer_handle_t camera3buffer, int flags)\n>> +\t{\n> \n> Isn't this a bit long to be inlined ? Or does inlining just happens if\n> the method is implemented in the class definition in an header file ?\n> I don't think so, as per the compiler perspective they're equivalent.\n> \n> And I would anyway place the implementation outside of the class\n> definition to save and indentation level, inlining apart.\n\n\nYes, it's not about inlining, it's just that the definition is all in one.\n\nI've made it:\n\nclass MappedCamera3Buffer : public MappedBuffer {\n    MappedCamera3Buffer(const buffer_handle_t camera3buffer, int flags)\n};\n\nMappedCamera3Buffer::MappedCamera3Buffer()\n{\n\t/* code */\n}\n\n\n\n> \n>> +\t\tmaps_.reserve(camera3buffer->numFds);\n>> +\t\terror_ = 0;\n>> +\n>> +\t\tfor (int i = 0; i < camera3buffer->numFds; i++) {\n>> +\t\t\tif (camera3buffer->data[i] == -1)\n>> +\t\t\t\tcontinue;\n>> +\n>> +\t\t\toff_t length = lseek(camera3buffer->data[i], 0, SEEK_END);\n>> +\t\t\tif (length < 0) {\n>> +\t\t\t\terror_ = errno;\n>> +\t\t\t\tLOG(HAL, Error) << \"Failed to query plane length\";\n>> +\t\t\t\tbreak;\n>> +\t\t\t}\n>> +\n>> +\t\t\tvoid *address = mmap(nullptr, length, flags, MAP_SHARED,\n>> +\t\t\t\t\t     camera3buffer->data[i], 0);\n>> +\t\t\tif (address == MAP_FAILED) {\n>> +\t\t\t\terror_ = errno;\n>> +\t\t\t\tLOG(HAL, Error) << \"Failed to mmap plane\";\n>> +\t\t\t\tbreak;\n>> +\t\t\t}\n>> +\n>> +\t\t\tmaps_.emplace_back(static_cast<uint8_t *>(address), static_cast<size_t>(length));\n>> +\t\t}\n>> +\n>> +\t\tvalid_ = error_ == 0;\n>> +\t}\n>> +};\n>> +\n>>  /*\n>>   * \\struct Camera3RequestDescriptor\n>>   *\n>> @@ -368,9 +405,9 @@ std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize()\n>>  {\n>>  \t/*\n>>  \t * \\todo Keep this in sync with the actual number of entries.\n>> -\t * Currently: 50 entries, 647 bytes of static metadata\n>> +\t * Currently: 51 entries, 651 bytes of static metadata\n>>  \t */\n>> -\tuint32_t numEntries = 50;\n>> +\tuint32_t numEntries = 51;\n>>  \tuint32_t byteSize = 651;\n> \n> The comment was clearly wrong, you should increase the actual size to\n> 667. You need 16 bytes more for:\n> - The uint32_t transported by ANDROID_JPEG_MAX_SIZE\n> - The three int32_t tags you add to availableResultKeys (see below)\n\nYup, those entries have been destroyed through multiple rebases :-(\n\nFixed,\n\n>>\n>>  \t/*\n>> @@ -527,6 +564,9 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n>>  \t\t\t\t  availableThumbnailSizes.data(),\n>>  \t\t\t\t  availableThumbnailSizes.size());\n>>\n>> +\tint32_t jpegMaxSize = 13 << 20; /* 13631488 from USB HAL */\n> \n> That's worth a\n>         \\todo Calculate the maximum JPEG buffer size by inspecting the\n>         largest image size and the jpeg compressor requirements\n> \n> or something similar, as I'm not sure the \"compressor requirements\"\n> make sense or not.\n\n\nYes, libjpeg has a function to provide the biggest buffer required,\ngiven a width/height so we can generate that (on top).\n\nDefinitely a good \\todo.\n\n> \n>> +\tstaticMetadata_->addEntry(ANDROID_JPEG_MAX_SIZE, &jpegMaxSize, 1);\n>> +\n>>  \t/* Sensor static metadata. */\n>>  \tint32_t pixelArraySize[] = {\n>>  \t\t2592, 1944,\n>> @@ -729,6 +769,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n>>  \t\tANDROID_CONTROL_AWB_LOCK_AVAILABLE,\n>>  \t\tANDROID_CONTROL_AVAILABLE_MODES,\n>>  \t\tANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES,\n>> +\t\tANDROID_JPEG_MAX_SIZE,\n>>  \t\tANDROID_SENSOR_INFO_PIXEL_ARRAY_SIZE,\n>>  \t\tANDROID_SENSOR_INFO_ACTIVE_ARRAY_SIZE,\n>>  \t\tANDROID_SENSOR_INFO_SENSITIVITY_RANGE,\n>> @@ -795,6 +836,9 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n>>  \t\tANDROID_SENSOR_EXPOSURE_TIME,\n>>  \t\tANDROID_STATISTICS_LENS_SHADING_MAP_MODE,\n>>  \t\tANDROID_STATISTICS_SCENE_FLICKER,\n>> +\t\tANDROID_JPEG_SIZE,\n>> +\t\tANDROID_JPEG_QUALITY,\n>> +\t\tANDROID_JPEG_ORIENTATION,\n> \n> You should add the size of 3 more integers to the static metadata pack\n> size if you add these here. (now commented above too)\n> \n>>  \t};\n>>  \tstaticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_RESULT_KEYS,\n>>  \t\t\t\t  availableResultKeys.data(),\n>> @@ -956,6 +1000,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>>  \t */\n>>  \tunsigned int streamIndex = 0;\n>>\n>> +\t/* First handle all non-MJPEG streams */\n>>  \tfor (unsigned int i = 0; i < stream_list->num_streams; ++i) {\n>>  \t\tcamera3_stream_t *stream = stream_list->streams[i];\n>>\n>> @@ -971,6 +1016,18 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>>  \t\tif (!format.isValid())\n>>  \t\t\treturn -EINVAL;\n>>\n>> +\t\tstream->priv = static_cast<void *>(&streams_[i]);\n>> +\t\tstreams_[i].format = format;\n> \n> You are still not pushing back or emplacing a CameraStream to the\n> vector but just using the there reserved memory. We have taken this\n> extensively offline I'll just record it here :0\n> \n\nNow finally updated in a way I hope you'll like ;-)\n\n> \n>> +\t\tstreams_[i].size = Size(stream->width, stream->height);\n> \n> I still find all the 'stream/streams/streams_' occurences confusing,\n> but the patch that adds streams_ and CameraStream is merged, so a\n> rename would require a patch before this one :) too much churn and\n> it's possibly better to do that once this is merged.\n> \n>> +\n>> +\t\t/* Defer handling of MJPEG streams until all others are known. */\n>> +\t\tif (format == formats::MJPEG) {\n>> +\t\t\tLOG(HAL, Info) << \"Handling MJPEG stream\";\n>> +\n>> +\t\t\tstreams_[i].index = -1;\n>> +\t\t\tcontinue;\n>> +\t\t}\n>> +\n>>  \t\tStreamConfiguration streamConfiguration;\n>>\n>>  \t\tstreamConfiguration.size.width = stream->width;\n>> @@ -980,7 +1037,61 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>>  \t\tconfig_->addConfiguration(streamConfiguration);\n>>\n>>  \t\tstreams_[i].index = streamIndex++;\n>> -\t\tstream->priv = static_cast<void *>(&streams_[i]);\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\tCameraStream *cameraStream = &streams_[i];\n>> +\n>> +\t\t/* Only process MJPEG streams */\n>> +\t\tif (cameraStream->format != formats::MJPEG)\n>> +\t\t\tcontinue;\n>> +\n>> +\t\tbool match = false;\n>> +\n>> +\t\t/* Search for a compatible stream */\n>> +\t\tfor (unsigned int j = 0; j < config_->size(); j++) {\n>> +\t\t\tStreamConfiguration &cfg = config_->at(j);\n>> +\n>> +\t\t\t/*\n>> +\t\t\t * The PixelFormat must also be compatible with the\n>> +\t\t\t * encoder.\n>> +\t\t\t */\n>> +\t\t\tif (cfg.size == cameraStream->size) {\n>> +\t\t\t\tLOG(HAL, Info)\n>> +\t\t\t\t\t<< \"Stream \" << i\n>> +\t\t\t\t\t<< \" using libcamera stream \"\n>> +\t\t\t\t\t<< j;\n> \n> Won't this fit in 2 lines ?\n> \t\t\t\tLOG(HAL, Info) << \"Stream \" << i\n> \t\t\t\t\t       << \" using libcamera stream \" << j;\n\n82 chars on the second... but thats' fine with me...\n\n> \n>> +\n>> +\t\t\t\tmatch = true;\n>> +\t\t\t\tcameraStream->index = j;\n>> +\t\t\t}\n>> +\t\t}\n>> +\n>> +\t\t/*\n>> +\t\t * Without a compatible match for JPEG encoding we must\n>> +\t\t * introduce a new stream to satisfy the request requirements.\n>> +\t\t */\n>> +\t\tif (!match) {\n>> +\t\t\tStreamConfiguration streamConfiguration;\n>> +\n>> +\t\t\t/*\n>> +\t\t\t * \\todo: The pixelFormat should be a 'best-fit' choice\n>> +\t\t\t * and may require a validation cycle. This is not yet\n>> +\t\t\t * handled, and should be considered as part of any\n>> +\t\t\t * stream configuration reworks.\n>> +\t\t\t */\n>> +\t\t\tstreamConfiguration.size.width = stream->width;\n>> +\t\t\tstreamConfiguration.size.height = stream->height;\n>> +\t\t\tstreamConfiguration.pixelFormat = formats::NV12;\n>> +\n>> +\t\t\tLOG(HAL, Info) << \"Adding \" << streamConfiguration.toString()\n>> +\t\t\t\t       << \" for MJPEG support\";\n>> +\n>> +\t\t\tconfig_->addConfiguration(streamConfiguration);\n>> +\t\t\tstreams_[i].index = streamIndex++;\n>> +\t\t}\n>>  \t}\n>>\n>>  \tswitch (config_->validate()) {\n>> @@ -1007,6 +1118,18 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\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 compressor 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 CompressorJPEG();\n>> +\t\t\tcameraStream->jpeg->configure(cfg);\n> \n> Just recording what we've discussed off-line: this should be deleted\n> when the CameraStream instance is deleted. As the CameraStream\n> instances are stored in the stream_ vector, clearing it at next\n> configuration request and camera close time should be enough.\n\nYes, handled by a destructor on the CameraStream now.\n\n\n> \n>> +\t\t} else {\n>> +\t\t\t/* Either construct this correctly, or use a better interface */\n>> +\t\t\tcameraStream->jpeg = nullptr;\n>> +\t\t}\n>>  \t}\n>>\n>>  \t/*\n>> @@ -1101,6 +1224,10 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n>>  \t\tdescriptor->buffers[i].stream = camera3Buffers[i].stream;\n>>  \t\tdescriptor->buffers[i].buffer = camera3Buffers[i].buffer;\n>>\n>> +\t\t/* Software streams are handled after hardware streams complete. */\n>> +\t\tif (cameraStream->format == formats::MJPEG)\n>> +\t\t\tcontinue;\n>> +\n>>  \t\t/*\n>>  \t\t * Create a libcamera buffer using the dmabuf descriptors of\n>>  \t\t * the camera3Buffer for each stream. The FrameBuffer is\n>> @@ -1157,9 +1284,72 @@ void CameraDevice::requestComplete(Request *request)\n>>  \tresultMetadata = getResultMetadata(descriptor->frameNumber,\n>>  \t\t\t\t\t   buffer->metadata().timestamp);\n>>\n>> -\t/* Prepare to call back the Android camera stack. */\n>> +\t/* Handle any JPEG compression */\n>> +\tfor (unsigned int i = 0; i < descriptor->numBuffers; ++i) {\n>> +\t\tCameraStream *cameraStream =\n>> +\t\t\tstatic_cast<CameraStream *>(descriptor->buffers[i].stream->priv);\n>> +\t\tCompressor *compressor = cameraStream->jpeg;\n>>\n>> +\t\t/* Only handle streams with compression enabled. */\n>> +\t\tif (!compressor)\n>> +\t\t\tcontinue;\n> \n> I wonder if inspecting the format would avoid using a pointer as a\n> flag. This is fine for now though.\n\nAh yes, that might indeed be better!\n\nAnd precedence is already used to use the format::MJPEG anyway !\n\n>>\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) {\n>> +\t\t\tLOG(HAL, Error) << \"Failed to find a source stream buffer\";\n>> +\t\t\tcontinue;\n>> +\t\t}\n>> +\n>> +\t\tMappedCamera3Buffer mapped(*descriptor->buffers[i].buffer, PROT_READ | PROT_WRITE);\n>> +\t\tif (!mapped.isValid()) {\n>> +\t\t\tLOG(HAL, Error) << \"Failed to mmap android blob buffer\";\n>> +\t\t\tcontinue;\n>> +\t\t}\n>> +\n>> +\t\tCompressedImage output;\n>> +\t\toutput.data = static_cast<unsigned char *>(mapped.maps()[0].data());\n>> +\t\toutput.length = mapped.maps()[0].size();\n>> +\n>> +\t\tint ret = compressor->compress(buffer, &output);\n>> +\t\tif (ret) {\n>> +\t\t\tLOG(HAL, Error) << \"Failed to compress stream image\";\n>> +\t\t\tstatus = CAMERA3_BUFFER_STATUS_ERROR;\n>> +\t\t}\n>> +\n>> +\t\t/* Fill in the JPEG blob header. */\n>> +\t\t{\n>> +\t\t\t/* The mapped size of the buffer is being returned as substantially larger\n>> +\t\t\t * than the requested JPEG_MAX_SIZE (re-defined here).\n>> +\t\t\t * Utilise this static size as a workaround to ensure the correct offset\n>> +\t\t\t * of the blob is determined.\n>> +\t\t\t *\n>> +\t\t\t * \\todo Investigate and fix the root cause of this.\n>> +\t\t\t */\n>> +\t\t\tint32_t max_size = { 13 << 20 };\n>> +\t\t\tuint8_t *resultPtr = static_cast<uint8_t *>(mapped.maps()[0].data()) +\n>> +\t\t\t\t\t     max_size - sizeof(struct camera3_jpeg_blob);\n>> +\t\t\tauto *blob = reinterpret_cast<struct camera3_jpeg_blob *>(resultPtr);\n>> +\n>> +\t\t\tblob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID;\n>> +\t\t\tblob->jpeg_size = output.length;\n>> +\t\t}\n> \n> Why a separate scope block ? Not that it bothers me, just curious\n\nJust scoped while I was developing, this was a part that was faulty\nuntil 'very' recently, so it was an add on patch that got squashed for\ninclusion in this review cycle.\n\nNo intent to keep it scoped, and it'll get a promotion now that it works.\n\nAlso moving the max_size to a member variable of the CameraDevice class\nso that it can be used identically between here and the setting of the\nstatic metadata, (along with the todo that it can be updated at\nconfiguration time).\n\n\n>> +\n>> +\t\t/* Update the JPEG result Metadata. */\n>> +\t\tresultMetadata->addEntry(ANDROID_JPEG_SIZE,\n>> +\t\t\t\t\t &output.length, 1);\n>> +\n>> +\t\tconst uint32_t jpeg_quality = 95;\n>> +\t\tresultMetadata->addEntry(ANDROID_JPEG_QUALITY,\n>> +\t\t\t\t\t &jpeg_quality, 1);\n>> +\n>> +\t\tconst uint32_t jpeg_orientation = 0;\n>> +\t\tresultMetadata->addEntry(ANDROID_JPEG_ORIENTATION,\n>> +\t\t\t\t\t &jpeg_orientation, 1);\n>> +\t}\n>> +\n>> +\t/* Prepare to call back the Android camera stack. */\n>>  \tcamera3_capture_result_t captureResult = {};\n>>  \tcaptureResult.frame_number = descriptor->frameNumber;\n>>  \tcaptureResult.num_output_buffers = descriptor->numBuffers;\n>> @@ -1240,10 +1430,10 @@ std::unique_ptr<CameraMetadata> CameraDevice::getResultMetadata(int frame_number\n>>  {\n>>  \t/*\n>>  \t * \\todo Keep this in sync with the actual number of entries.\n>> -\t * Currently: 12 entries, 36 bytes\n>> +\t * Currently: 17 entries, 58 bytes\n>>  \t */\n>>  \tstd::unique_ptr<CameraMetadata> resultMetadata =\n>> -\t\tstd::make_unique<CameraMetadata>(15, 50);\n>> +\t\tstd::make_unique<CameraMetadata>(17, 58);\n> \n> I see three entries more:\n>         ANDROID_JPEG_SIZE, ANDROID_JPEG_QUALITY, ANDROID_JPEG_ORIENTATION\n> \n\nAh yes, I'll add more, though something isn't being used, because the\nusage of this metadata doesn't get full, but that's not a problem.\n\n\n\n>>  \tif (!resultMetadata->isValid()) {\n>>  \t\tLOG(HAL, Error) << \"Failed to allocate static metadata\";\n>>  \t\treturn nullptr;\n>> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n>> index 00472c219388..c9ece426a00e 100644\n>> --- a/src/android/camera_device.h\n>> +++ b/src/android/camera_device.h\n>> @@ -23,6 +23,8 @@\n>>  #include \"libcamera/internal/log.h\"\n>>  #include \"libcamera/internal/message.h\"\n>>\n>> +#include \"jpeg/compressor_jpeg.h\"\n>> +\n>>  class CameraMetadata;\n>>\n>>  struct CameraStream {\n>> @@ -32,6 +34,12 @@ struct CameraStream {\n>>  \t * one or more streams to the Android framework.\n>>  \t */\n>>  \tunsigned int index;\n>> +\n>> +\tlibcamera::PixelFormat format;\n>> +\tlibcamera::Size size;\n>> +\n>> +\t/* Make sure this gets destructed correctly */\n>> +\tCompressorJPEG *jpeg;\n> \n> In the struct or class destructor ?\n> \n\nNow resolved with a destructor (of the CameraStream).\n\n> Thanks\n>   j\n> \n\n\nAll the above resolved I hope.\n\nThanks!\n\n--\nKieran\n\n\n>>  };\n>>\n>>  class CameraDevice : protected libcamera::Loggable\n>> --\n>> 2.25.1\n>>\n>> _______________________________________________\n>> libcamera-devel mailing list\n>> libcamera-devel@lists.libcamera.org\n>> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 A2E66BD86F\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  4 Aug 2020 20:13:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2D0216054A;\n\tTue,  4 Aug 2020 22:13:54 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1927D60545\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  4 Aug 2020 22:13:52 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6B37627B;\n\tTue,  4 Aug 2020 22:13:51 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"rwuBwh9c\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1596572031;\n\tbh=vQgbhaZgr6ZycnU1DVPUg6HZVsdnjh1y3AknJbwZGIE=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=rwuBwh9czrEVp+wkWJArbfAxRCXTH++ZIroBRzAfIhINPKhqT7MKDql9cE0dbAass\n\tMTsGf4JFWc6bTR32Sb0u+BGtXb1XPM3ltLmNLLMTpHd+s89ERePqGTCaeApd1vRrz5\n\tZ4wNDoMnVlaCS1u7kH4cXfoGrJ+twXrLG9JwyKRI=","To":"Jacopo Mondi <jacopo@jmondi.org>","References":"<20200803161816.107113-1-kieran.bingham@ideasonboard.com>\n\t<20200803161816.107113-13-kieran.bingham@ideasonboard.com>\n\t<20200804150442.socusctwobbbnqxs@uno.localdomain>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<3e867c49-ca88-d873-3e1e-6f3ffb520131@ideasonboard.com>","Date":"Tue, 4 Aug 2020 21:13:48 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20200804150442.socusctwobbbnqxs@uno.localdomain>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v2 12/12] android: camera_device:\n\tSupport MJPEG stream construction","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>","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]