[{"id":11675,"web_url":"https://patchwork.libcamera.org/comment/11675/","msgid":"<20200728165821.le2iljhh7sucmp2r@uno.localdomain>","date":"2020-07-28T16:58:21","subject":"Re: [libcamera-devel] [RFC PATCH 4/6] 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   sorry I got here later than expected\n\nOn Tue, Jul 21, 2020 at 11:01:24PM +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>  src/android/camera_device.cpp | 158 +++++++++++++++++++++++++++++++++-\n>  src/android/camera_device.h   |   8 ++\n>  2 files changed, 162 insertions(+), 4 deletions(-)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 56652ac57676..7323d4e58f68 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> @@ -1004,6 +1007,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> @@ -1019,6 +1023,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\nDumb question, you called reserve on the stream_ vector, and that\nreserves space for the container, but shouldn't you call push_back()\nto increase its size and make sure all its internal counters are\ncorrect. From the STL documentation:\n\nreserve() does not change the size of the vector.\n\n> +\t\tstreams_[i].format = format;\n> +\t\tstreams_[i].size = Size(stream->width, stream->height);\n> +\n\nThat's maybe me, and I know I like longer names which are not always\ngood, but a stream_ vector of CameraStreams, where we already have\ndaa types of Camera3... type adds confusion to a class which already\nrelies on the names \"streams\" and \"camera\" a bit too much...\n\nWhat about\nstruct StreamMap {\n        int streamIndex;\n\tlibcamera::PixelFormat format;\n\tlibcamera::Size size;\n\tCompressorJPEG *jpeg;\n};\n\nor something similar, and a\n\n        std::vector<StreamMap> streamMaps_;\n\nI know they are not std::map, but they map Android streams to\nlibcamera streams, and conveying that could be nice to read.\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> @@ -1028,7 +1044,61 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>  \t\tconfig_->addConfiguration(streamConfiguration);\n>\n>  \t\tstreams_[i].index = streamIndex++;\n\nI had this question if you could get rid of index and store a pointer\nto the StreamConfiguration. Since I recall I already asked that, as an\nexercize I tried getting rid of it and I failed, so no need to ask :)\nBut I leave here my exercize for reference, expecially for the\npart that actually emplaces in the vector the CameraStream.\n\n        unsigned int streamIndex = 0;\n\tfor (unsigned int i = 0; i < stream_list->num_streams; ++i) {\n\t\tcamera3_stream_t *stream = stream_list->streams[i];\n\t\tPixelFormat format = toPixelFormat(stream->format);\n\n\t\tLOG(HAL, Info) << \"Stream #\" << i\n\t\t\t       << \", direction: \" << stream->stream_type\n\t\t\t       << \", width: \" << stream->width\n\t\t\t       << \", height: \" << stream->height\n\t\t\t       << \", format: \" << utils::hex(stream->format)\n\t\t\t       << \" (\" << format.toString() << \")\";\n\n\t\tif (!format.isValid())\n\t\t\treturn -EINVAL;\n\n                streams_.emplace_back(-1, format,\n                                      { stream->width, stream->height }, nullptr);\n                stream->priv = static_cast<void *>(&streams_[i]);\n\n\t\t/* Defer handling of MJPEG streams until all others are known. */\n\t\tif (format == formats::MJPEG)\n\t\t\tcontinue;\n\n\t\tStreamConfiguration streamConfiguration;\n\t\tstreamConfiguration.size.width = stream->width;\n\t\tstreamConfiguration.size.height = stream->height;\n\t\tstreamConfiguration.pixelFormat = format;\n\t\tconfig_->addConfiguration(streamConfiguration);\n\n\t\tstreams_[i].index = streamIndex++;\n         }\n\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\nHere I would avoid doing this. Another thing being named as a\ncombination of the terms 'camera' and 'stream' is confusing. Can you\nuse streams[i] directly and avoid a confusing indirection ? We are\nclearly running out of terms here :)\n\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\t    LOG(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> +\t\t\t\t    match = true;\n> +\t\t\t\t    cameraStream->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> @@ -1059,6 +1129,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> +\t\t} else {\n> +\t\t\t/* Either construct this correctly, or use a better interface */\n> +\t\t\tcameraStream->jpeg = nullptr;\n\nI would be tempted to ask if we could create a JPEGCameraStream which\nderives from CameraStream, store pointers to CameraStream base class\nin stream_ and create the correct derived depending on the format to\navoid cameraStream->jpeg = nullptr.\n\nBut I suspect this is used as a flag somewhere in the next patches.\n\n> +\t\t}\n>  \t}\n>\n>  \t/*\n> @@ -1112,6 +1194,9 @@ FrameBuffer *CameraDevice::createFrameBuffer(const buffer_handle_t camera3buffer\n>\n>  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Request)\n>  {\n> +\tLOG(HAL, Error) << \"Received request \" << camera3Request->frame_number\n> +\t\t\t<< \" with \" << camera3Request->num_output_buffers << \" buffers\";\n> +\n>  \tif (!camera3Request->num_output_buffers) {\n>  \t\tLOG(HAL, Error) << \"No output buffers provided\";\n>  \t\treturn -EINVAL;\n> @@ -1158,6 +1243,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> @@ -1190,11 +1279,40 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n>  \treturn 0;\n>  }\n>\n> +static CompressedImage mapAndroidBlobBuffer(const buffer_handle_t camera3buffer)\n> +{\n> +\tCompressedImage img;\n> +\n> +\t/* ANDROID_JPEG_MAX_SIZE */\n> +\tunsigned int length = int32_t{13 << 20};\n> +\n> +\t/* Take only the first plane */\n> +\tvoid *memory = mmap(NULL, length, PROT_READ|PROT_WRITE, MAP_SHARED,\n> +\t\t\t    camera3buffer->data[0], 0);\n> +\n> +\timg.length = length;\n> +\timg.data = static_cast<unsigned char*>(memory);\n> +\n> +\treturn img;\n> +}\n> +\n> +static void unmapAndroidBlobBuffer(CompressedImage *img)\n> +{\n> +\tmunmap(img->data, img->length);\n> +\timg->data = nullptr;\n> +\timg->length = 0;\n> +}\n> +\n>  void CameraDevice::requestComplete(Request *request)\n>  {\n>  \tconst std::map<Stream *, FrameBuffer *> &buffers = request->buffers();\n>  \tcamera3_buffer_status status = CAMERA3_BUFFER_STATUS_OK;\n>  \tstd::unique_ptr<CameraMetadata> resultMetadata;\n> +\tCamera3RequestDescriptor *descriptor =\n> +\t\treinterpret_cast<Camera3RequestDescriptor *>(request->cookie());\n> +\n> +\n> +\tLOG(HAL, Error) << \"Request completed:...\";\n\nWhy move this up ?\n\n>\n>  \tif (request->status() != Request::RequestComplete) {\n>  \t\tLOG(HAL, Error) << \"Request not succesfully completed: \"\n> @@ -1202,10 +1320,41 @@ void CameraDevice::requestComplete(Request *request)\n>  \t\tstatus = CAMERA3_BUFFER_STATUS_ERROR;\n>  \t}\n>\n> -\t/* Prepare to call back the Android camera stack. */\n> -\tCamera3RequestDescriptor *descriptor =\n> -\t\treinterpret_cast<Camera3RequestDescriptor *>(request->cookie());\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\nAh yes, it is used as flag here. Maybe the idea of subclassing is an\noverkill.\n\n> +\n> +\t\t/* \\todo: Optimise this dance routine, just to get the stream/buffer ... */\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\tCompressedImage output = mapAndroidBlobBuffer(*descriptor->buffers[i].buffer);\n> +\t\tif (output.data == MAP_FAILED) {\n> +\t\t\tLOG(HAL, Error) << \"Failed to mmap android blob buffer of length \" << output.length;\n> +\t\t\tcontinue;\n> +\t\t}\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\tunmapAndroidBlobBuffer(&output);\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> @@ -1246,6 +1395,7 @@ void CameraDevice::requestComplete(Request *request)\n>  \t\t\t    descriptor->buffers[0].stream);\n>  \t}\n>\n> +\n>  \tcallbacks_->process_capture_result(callbacks_, &captureResult);\n>\n>  \tdelete descriptor;\n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index 5b8b9c3e26e2..1973adaa2b4b 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\nLooking at this now, it seems all it serves is actually for JPEG\ncompression. Maybe convey that in the name. I know I've suggested map,\nbut that's really a too generic name, and my head got twisted a few\ntimes when trying to keep track of all the 'streams' we have in this\nclass.\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 D4A0CBD86F\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 28 Jul 2020 16:54:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 29C7B617AF;\n\tTue, 28 Jul 2020 18:54:43 +0200 (CEST)","from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0C59F60923\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 28 Jul 2020 18:54:42 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 71AEDFF803;\n\tTue, 28 Jul 2020 16:54:41 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Tue, 28 Jul 2020 18:58:21 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20200728165821.le2iljhh7sucmp2r@uno.localdomain>","References":"<20200721220126.202065-1-kieran.bingham@ideasonboard.com>\n\t<20200721220126.202065-5-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200721220126.202065-5-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH 4/6] 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":11816,"web_url":"https://patchwork.libcamera.org/comment/11816/","msgid":"<282f5dbd-2df6-6357-b278-12728f278637@ideasonboard.com>","date":"2020-08-04T10:55:39","subject":"Re: [libcamera-devel] [RFC PATCH 4/6] 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 28/07/2020 17:58, Jacopo Mondi wrote:\n> Hi Kieran,\n>    sorry I got here later than expected\n\nNo worries, thanks for your time.\n\n\n> On Tue, Jul 21, 2020 at 11:01:24PM +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>>  src/android/camera_device.cpp | 158 +++++++++++++++++++++++++++++++++-\n>>  src/android/camera_device.h   |   8 ++\n>>  2 files changed, 162 insertions(+), 4 deletions(-)\n>>\n>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n>> index 56652ac57676..7323d4e58f68 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>> @@ -1004,6 +1007,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>> @@ -1019,6 +1023,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> \n> Dumb question, you called reserve on the stream_ vector, and that\n> reserves space for the container, but shouldn't you call push_back()\n> to increase its size and make sure all its internal counters are\n> correct. From the STL documentation:\n> \n> reserve() does not change the size of the vector.\n> \n>> +\t\tstreams_[i].format = format;\n>> +\t\tstreams_[i].size = Size(stream->width, stream->height);\n>> +\n\n\nHrm, I guess by reserving the space I can write to it with the array\nindex, but indeed, I didn't think as to whether it has automatically\nincreased the size to the max of the accessed index, so it probably is\nincorrect.\n\nI wanted to use the indexes directly, because we are skipping mjpeg\nstreams in this loop iteration, so using push_back would break the indices.\n\nMaybe I just need to rethink this, (or see if it's still necessary).\n\n\n~~~~~~\n\nSo, I need to be able to map from the buffers in a given request, back\nto the streams which handle that buffer.\n\nThe index here is important currently. I'll have to see if it's really\nrequired, or if I was just using it for convenience.\n\n\n\n> That's maybe me, and I know I like longer names which are not always\n> good, but a stream_ vector of CameraStreams, where we already have\n> daa types of Camera3... type adds confusion to a class which already\n> relies on the names \"streams\" and \"camera\" a bit too much...\n> \n> What about\n> struct StreamMap {\n>         int streamIndex;\n> \tlibcamera::PixelFormat format;\n> \tlibcamera::Size size;\n> \tCompressorJPEG *jpeg;\n> };\n> \n> or something similar, and a\n> \n>         std::vector<StreamMap> streamMaps_;\n> \n> I know they are not std::map, but they map Android streams to\n> libcamera streams, and conveying that could be nice to read.\n\nWell, in fact it could be a map from camera3_stream_t, to our internal\nrepresentation.\n\nAlthough, I'm not sure if the map gives a benefit yet, as I store the\npointer in the camera3_stream_t->priv which is a faster access than\nlooking up in a map, so I guess it's only if the reverse is needed ...\n\nHrm...\n\nIn fact, if I store the pointer, maybe I don't need the index to match\nanyway, as the access route is there (until I see I need to go\nbackwards, which could be handled by a reverse pointer anyway).\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>> @@ -1028,7 +1044,61 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>>  \t\tconfig_->addConfiguration(streamConfiguration);\n>>\n>>  \t\tstreams_[i].index = streamIndex++;\n> \n> I had this question if you could get rid of index and store a pointer\n> to the StreamConfiguration. Since I recall I already asked that, as an\n> exercize I tried getting rid of it and I failed, so no need to ask :)\n> But I leave here my exercize for reference, expecially for the\n> part that actually emplaces in the vector the CameraStream.\n\nWe can't store a pointer to the StreamConfiguration, because they change\nwhen they are added, and the address isn't guaranteed to stay constant.\n\n\n\n>         unsigned int streamIndex = 0;\n> \tfor (unsigned int i = 0; i < stream_list->num_streams; ++i) {\n> \t\tcamera3_stream_t *stream = stream_list->streams[i];\n> \t\tPixelFormat format = toPixelFormat(stream->format);\n> \n> \t\tLOG(HAL, Info) << \"Stream #\" << i\n> \t\t\t       << \", direction: \" << stream->stream_type\n> \t\t\t       << \", width: \" << stream->width\n> \t\t\t       << \", height: \" << stream->height\n> \t\t\t       << \", format: \" << utils::hex(stream->format)\n> \t\t\t       << \" (\" << format.toString() << \")\";\n> \n> \t\tif (!format.isValid())\n> \t\t\treturn -EINVAL;\n> \n>                 streams_.emplace_back(-1, format,\n>                                       { stream->width, stream->height }, nullptr);\n>                 stream->priv = static_cast<void *>(&streams_[i]);\n> \n> \t\t/* Defer handling of MJPEG streams until all others are known. */\n> \t\tif (format == formats::MJPEG)\n> \t\t\tcontinue;\n> \n> \t\tStreamConfiguration streamConfiguration;\n> \t\tstreamConfiguration.size.width = stream->width;\n> \t\tstreamConfiguration.size.height = stream->height;\n> \t\tstreamConfiguration.pixelFormat = format;\n> \t\tconfig_->addConfiguration(streamConfiguration);\n> \n> \t\tstreams_[i].index = streamIndex++;\n>          }\n> \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> Here I would avoid doing this. Another thing being named as a\n> combination of the terms 'camera' and 'stream' is confusing. Can you\n> use streams[i] directly and avoid a confusing indirection ? We are\n> clearly running out of terms here :)\n\nWell the point was the CameraStream was supposed to be 'our'\nrepresentation of the 'stream'.\n\nAlmost makes me want to continue down the path that all streams get\ngiven a CameraStream, which has the camera3_stream_t stored in it at\ncreation, so that we only then iterate 'our' streams, and reference the\nandroid type when needed.\n\nMaybe that would be clearer ...\n\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\t    LOG(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>> +\t\t\t\t    match = true;\n>> +\t\t\t\t    cameraStream->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\nHrm, seems I forgot all about the fact that if I add my own libcamera\nstream here, I'm giong to be responsible for allocating buffers for that\nstream, and queueing them appropriately when the request is queued! Ugh.\n\n\n>> +\t\t}\n>>  \t}\n>>\n>>  \tswitch (config_->validate()) {\n>> @@ -1059,6 +1129,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>> +\t\t} else {\n>> +\t\t\t/* Either construct this correctly, or use a better interface */\n>> +\t\t\tcameraStream->jpeg = nullptr;\n> \n> I would be tempted to ask if we could create a JPEGCameraStream which\n> derives from CameraStream, store pointers to CameraStream base class\n> in stream_ and create the correct derived depending on the format to\n> avoid cameraStream->jpeg = nullptr.\n> \n> But I suspect this is used as a flag somewhere in the next patches.\n\nYes, this bit needs rework (hence the comment) but is used as a flag.\n\nI'm not fond of using the pointer as a flag, but it's sort of convenient\nI guess as it also needs to be stored anyway.\n\n\n> \n>> +\t\t}\n>>  \t}\n>>\n>>  \t/*\n>> @@ -1112,6 +1194,9 @@ FrameBuffer *CameraDevice::createFrameBuffer(const buffer_handle_t camera3buffer\n>>\n>>  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Request)\n>>  {\n>> +\tLOG(HAL, Error) << \"Received request \" << camera3Request->frame_number\n>> +\t\t\t<< \" with \" << camera3Request->num_output_buffers << \" buffers\";\n>> +\n>>  \tif (!camera3Request->num_output_buffers) {\n>>  \t\tLOG(HAL, Error) << \"No output buffers provided\";\n>>  \t\treturn -EINVAL;\n>> @@ -1158,6 +1243,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>> @@ -1190,11 +1279,40 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n>>  \treturn 0;\n>>  }\n>>\n>> +static CompressedImage mapAndroidBlobBuffer(const buffer_handle_t camera3buffer)\n>> +{\n>> +\tCompressedImage img;\n>> +\n>> +\t/* ANDROID_JPEG_MAX_SIZE */\n>> +\tunsigned int length = int32_t{13 << 20};\n>> +\n>> +\t/* Take only the first plane */\n>> +\tvoid *memory = mmap(NULL, length, PROT_READ|PROT_WRITE, MAP_SHARED,\n>> +\t\t\t    camera3buffer->data[0], 0);\n>> +\n>> +\timg.length = length;\n>> +\timg.data = static_cast<unsigned char*>(memory);\n>> +\n>> +\treturn img;\n>> +}\n>> +\n>> +static void unmapAndroidBlobBuffer(CompressedImage *img)\n>> +{\n>> +\tmunmap(img->data, img->length);\n>> +\timg->data = nullptr;\n>> +\timg->length = 0;\n>> +}\n>> +\n>>  void CameraDevice::requestComplete(Request *request)\n>>  {\n>>  \tconst std::map<Stream *, FrameBuffer *> &buffers = request->buffers();\n>>  \tcamera3_buffer_status status = CAMERA3_BUFFER_STATUS_OK;\n>>  \tstd::unique_ptr<CameraMetadata> resultMetadata;\n>> +\tCamera3RequestDescriptor *descriptor =\n>> +\t\treinterpret_cast<Camera3RequestDescriptor *>(request->cookie());\n>> +\n>> +\n>> +\tLOG(HAL, Error) << \"Request completed:...\";\n> \n> Why move this up ?\n\n\nBecause I need to add fields to the resultMetaData, so I had to move\nthat up earlier. Looks like the two corresponding changes got put in\nseparate patches. I've split that so it's clear why it moves.\n\n\n\n> \n>>\n>>  \tif (request->status() != Request::RequestComplete) {\n>>  \t\tLOG(HAL, Error) << \"Request not succesfully completed: \"\n>> @@ -1202,10 +1320,41 @@ void CameraDevice::requestComplete(Request *request)\n>>  \t\tstatus = CAMERA3_BUFFER_STATUS_ERROR;\n>>  \t}\n>>\n>> -\t/* Prepare to call back the Android camera stack. */\n>> -\tCamera3RequestDescriptor *descriptor =\n>> -\t\treinterpret_cast<Camera3RequestDescriptor *>(request->cookie());\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> Ah yes, it is used as flag here. Maybe the idea of subclassing is an\n> overkill.\n> \n>> +\n>> +\t\t/* \\todo: Optimise this dance routine, just to get the stream/buffer ... */\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\tCompressedImage output = mapAndroidBlobBuffer(*descriptor->buffers[i].buffer);\n>> +\t\tif (output.data == MAP_FAILED) {\n>> +\t\t\tLOG(HAL, Error) << \"Failed to mmap android blob buffer of length \" << output.length;\n>> +\t\t\tcontinue;\n>> +\t\t}\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\tunmapAndroidBlobBuffer(&output);\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>> @@ -1246,6 +1395,7 @@ void CameraDevice::requestComplete(Request *request)\n>>  \t\t\t    descriptor->buffers[0].stream);\n>>  \t}\n>>\n>> +\n>>  \tcallbacks_->process_capture_result(callbacks_, &captureResult);\n>>\n>>  \tdelete descriptor;\n>> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n>> index 5b8b9c3e26e2..1973adaa2b4b 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> Looking at this now, it seems all it serves is actually for JPEG\n> compression. Maybe convey that in the name. I know I've suggested map,\n> but that's really a too generic name, and my head got twisted a few\n> times when trying to keep track of all the 'streams' we have in this\n> class.\n\nI guess so, except I think I was envisaging that this object would\nrepresent both JPEG streams and 'pure' libcamera streams.\n\nBut I maybe I can rename it to make it as JPEGStream to make it clearer\nfor now. I'll have to check if I'm actually using it for non-jpeg\nstreams though. I suspect I'm not (or not essentially).\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 8511EBD86F\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  4 Aug 2020 10:55:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 14E8260495;\n\tTue,  4 Aug 2020 12:55:47 +0200 (CEST)","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 CEF44603C9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  4 Aug 2020 12:55:45 +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 30EA827B;\n\tTue,  4 Aug 2020 12:55:42 +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=\"WJWQDPGT\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1596538542;\n\tbh=/89ScZRcD0Uo7vXbnEqNBXue3BFhjBctyo+tvCd8aRk=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=WJWQDPGTKkZ9Y9rfjLogWqI6yx1vzkF/X3dVTg+et5h/b49LTyRawkJjQY6UJ+W1k\n\tumDvzQG7iYw/S5m7eKfEC2uMCo5zGd+vcor5nJY+Thm+v1Rkh6SziiXT1+3dRIayRU\n\tYG4I86xQrSD3JKM5iBp06lnEJ5POeKhUHL7p4etU=","To":"Jacopo Mondi <jacopo@jmondi.org>","References":"<20200721220126.202065-1-kieran.bingham@ideasonboard.com>\n\t<20200721220126.202065-5-kieran.bingham@ideasonboard.com>\n\t<20200728165821.le2iljhh7sucmp2r@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":"<282f5dbd-2df6-6357-b278-12728f278637@ideasonboard.com>","Date":"Tue, 4 Aug 2020 11:55:39 +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":"<20200728165821.le2iljhh7sucmp2r@uno.localdomain>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [RFC PATCH 4/6] 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>"}}]