[{"id":12846,"web_url":"https://patchwork.libcamera.org/comment/12846/","msgid":"<20200929012824.GI14614@pendragon.ideasonboard.com>","date":"2020-09-29T01:28:24","subject":"Re: [libcamera-devel] [PATCH v3 1/8] android: camera_device: Add\n\tCameraStream::Type","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Sep 22, 2020 at 11:47:31AM +0200, Jacopo Mondi wrote:\n> Define the CameraStream::Type enumeration and assign it to\n> each CameraStream instance at construction time.\n> \n> The CameraStream type will be used to decide if memory needs to be\n> allocated on its behalf or if the stream is backed by memory externally\n> allocated by the Android framework.\n> \n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> Reviewed-by: Hirokazu Honda <hiroh@chromium.org>\n> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  src/android/camera_device.cpp | 14 ++++--\n>  src/android/camera_device.h   | 87 ++++++++++++++++++++++++++++++++++-\n>  2 files changed, 96 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 70d77a17ef43..e4ffbc02c2da 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -169,9 +169,11 @@ MappedCamera3Buffer::MappedCamera3Buffer(const buffer_handle_t camera3buffer,\n>  \t}\n>  }\n>  \n> -CameraStream::CameraStream(PixelFormat format, Size size,\n> +CameraStream::CameraStream(PixelFormat format, Size size, Type type,\n>  \t\t\t   unsigned int index, Encoder *encoder)\n> -\t: format_(format), size_(size), index_(index), encoder_(encoder)\n> +\n> +\t: format_(format), size_(size), type_(type), index_(index),\n> +\t  encoder_(encoder)\n>  {\n>  }\n>  \n> @@ -1222,12 +1224,14 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>  \n>  \t\tconfig_->addConfiguration(streamConfiguration);\n>  \t\tunsigned int index = config_->size() - 1;\n> -\t\tstreams_.emplace_back(format, size, index);\n> +\t\tstreams_.emplace_back(format, size, CameraStream::Type::Direct,\n> +\t\t\t\t      index);\n>  \t\tstream->priv = static_cast<void *>(&streams_.back());\n>  \t}\n>  \n>  \t/* Now handle the MJPEG streams, adding a new stream if required. */\n>  \tif (jpegStream) {\n> +\t\tCameraStream::Type type;\n>  \t\tint index = -1;\n>  \n>  \t\t/* Search for a compatible stream in the non-JPEG ones. */\n> @@ -1245,6 +1249,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>  \t\t\tLOG(HAL, Info)\n>  \t\t\t\t<< \"Android JPEG stream mapped to libcamera stream \" << i;\n>  \n> +\t\t\ttype = CameraStream::Type::Mapped;\n>  \t\t\tindex = i;\n>  \t\t\tbreak;\n>  \t\t}\n> @@ -1269,6 +1274,7 @@ 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\ttype = CameraStream::Type::Internal;\n>  \t\t\tconfig_->addConfiguration(streamConfiguration);\n>  \t\t\tindex = config_->size() - 1;\n>  \t\t}\n> @@ -1287,7 +1293,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>  \t\t\treturn ret;\n>  \t\t}\n>  \n> -\t\tstreams_.emplace_back(formats::MJPEG, cfg.size, index, encoder);\n> +\t\tstreams_.emplace_back(formats::MJPEG, cfg.size, type, index, encoder);\n>  \t\tjpegStream->priv = static_cast<void *>(&streams_.back());\n>  \t}\n>  \n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index 1837748d2efc..9a9406cc257c 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -30,17 +30,102 @@ class CameraMetadata;\n>  class CameraStream\n>  {\n>  public:\n> +\t/*\n> +\t * Enumeration of CameraStream types.\n> +\t *\n> +\t * A camera stream associates an Android stream to a libcamera stream.\n> +\t * This enumeration describes how the two streams are associated and how\n> +\t * and where data produced from libcamera are delivered to the\n> +\t * Android framework.\n> +\t *\n> +\t * Direct:\n> +\t *\n> +\t * The Android stream is directly mapped onto a libcamera stream: frames\n> +\t * are delivered by the library directly in the memory location\n> +\t * specified by the Android stream (buffer_handle_t->data) and provided\n> +\t * to the framework as they are. The Android stream characteristics are\n> +\t * directly translated to the libcamera stream configuration.\n> +\t *\n> +\t * +-----+                +-----+\n> +\t * |  A  |                |  L  |\n> +\t * +-----+                +-----+\n> +\t *    |                      |\n> +\t *    V                      V\n> +\t * +-----+                +------+\n> +\t * |  B  |<---------------|  FB  |\n> +\t * +-----+                +------+\n> +\t *\n> +\t *\n> +\t * Internal:\n> +\t *\n> +\t * Data for the Android stream is produced by processing a libcamera\n> +\t * stream created by the HAL for that purpose. The libcamera stream\n> +\t * needs to be supplied with intermediate buffers where the library\n> +\t * delivers frames to be processed and then provided to the framework.\n> +\t * The libcamera stream configuration is not a direct translation of the\n> +\t * Android stream characteristics, but it describes the format and size\n> +\t * required for the processing procedure to produce frames in the\n> +\t * Android required format.\n> +\t *\n> +\t * +-----+                +-----+    +-----+\n> +\t * |  A  |                |  L  |--->|  FB |\n> +\t * +-----+                +-----+    +-----+\n> +\t *    |                      |         |\n> +\t *    V                      V         |\n> +\t * +-----+                +------+     |\n> +\t * |  B  |                |   B  |<----+\n> +\t * +-----+                +------+\n> +\t *   ^                       |\n> +\t *   |-------Processing------|\n\nMaybe there's something I don't get here, but isn't the processing from\nFB to B, not from B to B ? Shouldn't it look like this ?\n\n\t * +-----+                +-----+\n\t * |  A  |                |  L  |\n\t * +-----+                +-----+\n\t *    |                      |\n\t *    V                      V\n\t * +-----+                +------+\n\t * |  B  |                |  FB  |\n\t * +-----+                +------+\n\t *    ^                      |\n\t *    |------Processing------|\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> +\t *\n> +\t *\n> +\t * Mapped:\n> +\t *\n> +\t * Data for the Android stream is produced by processing a libcamera\n> +\t * stream associated with another CameraStream. Mapped camera streams do\n> +\t * not need any memory to be reserved for them as they process data\n> +\t * produced by libcamera for a different stream whose format and size\n> +\t * is compatible with the processing procedure requirements to produce\n> +\t * frames in the Android required format.\n> +\t *\n> +\t * +-----+      +-----+          +-----+\n> +\t * |  A  |      |  A' |          |  L  |\n> +\t * +-----+      +-----+          +-----+\n> +\t *    |            |                |\n> +\t *    V            V                V\n> +\t * +-----+      +-----+          +------+\n> +\t * |  B  |      |  B' |<---------|  FB  |\n> +\t * +-----+      +-----+          +------+\n> +\t *   ^              |\n> +\t *   |--Processing--|\n> +\t *\n> +\t *\n> +\t * --------------------------------------------------------------------\n> +\t * A  = Android stream\n> +\t * L  = libcamera stream\n> +\t * B  = memory buffer\n> +\t * FB = libcamera FrameBuffer\n> +\t * \"Processing\" = Frame processing procedure (Encoding, scaling etc)\n> +\t */\n> +\tenum class Type {\n> +\t\tDirect,\n> +\t\tInternal,\n> +\t\tMapped,\n> +\t};\n> +\n>  \tCameraStream(libcamera::PixelFormat format, libcamera::Size size,\n> -\t\t     unsigned int index, Encoder *encoder = nullptr);\n> +\t\t     Type type, unsigned int index, Encoder *encoder = nullptr);\n>  \n>  \tconst libcamera::PixelFormat &format() const { return format_; }\n>  \tconst libcamera::Size &size() const { return size_; }\n> +\tType type() const { return type_; }\n>  \tunsigned int index() const { return index_; }\n>  \tEncoder *encoder() const { return encoder_.get(); }\n>  \n>  private:\n>  \tlibcamera::PixelFormat format_;\n>  \tlibcamera::Size size_;\n> +\tType type_;\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","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 35F4CC3B5B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 29 Sep 2020 01:29:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AD15C62033;\n\tTue, 29 Sep 2020 03:29:01 +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 4156260365\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 29 Sep 2020 03:29:00 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A5CF9540;\n\tTue, 29 Sep 2020 03:28:59 +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=\"Pn9Fohu+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1601342939;\n\tbh=G+TIz9eG67K9hw0YFHYlTb6yItJC5q9vQUyRolav8qs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Pn9Fohu+J9e5grxQke0kUpoUjLBD3aUFk6905bLI+Ajm5BRvNv/As0ZHsXUAYc4CO\n\tFUulyH0GMGHlknI0KcU1bJt6JPWybhtXPuGjV9cFYXCKtbSuPCBtN9Y40wxTHZXU9+\n\to45MfQAyTRBoxJG7OflvBjziln6mt+QGRG1TY1FU=","Date":"Tue, 29 Sep 2020 04:28:24 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<20200929012824.GI14614@pendragon.ideasonboard.com>","References":"<20200922094738.5327-1-jacopo@jmondi.org>\n\t<20200922094738.5327-2-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200922094738.5327-2-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v3 1/8] android: camera_device: Add\n\tCameraStream::Type","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":"hanlinchen@chromium.org, libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12900,"web_url":"https://patchwork.libcamera.org/comment/12900/","msgid":"<20200930091443.6kolei74osa7zmto@uno.localdomain>","date":"2020-09-30T09:14:43","subject":"Re: [libcamera-devel] [PATCH v3 1/8] android: camera_device: Add\n\tCameraStream::Type","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Tue, Sep 29, 2020 at 04:28:24AM +0300, Laurent Pinchart wrote:\n> On Tue, Sep 22, 2020 at 11:47:31AM +0200, Jacopo Mondi wrote:\n> > Define the CameraStream::Type enumeration and assign it to\n> > each CameraStream instance at construction time.\n> >\n> > The CameraStream type will be used to decide if memory needs to be\n> > allocated on its behalf or if the stream is backed by memory externally\n> > allocated by the Android framework.\n> >\n> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > Reviewed-by: Hirokazu Honda <hiroh@chromium.org>\n> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > ---\n> >  src/android/camera_device.cpp | 14 ++++--\n> >  src/android/camera_device.h   | 87 ++++++++++++++++++++++++++++++++++-\n> >  2 files changed, 96 insertions(+), 5 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 70d77a17ef43..e4ffbc02c2da 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -169,9 +169,11 @@ MappedCamera3Buffer::MappedCamera3Buffer(const buffer_handle_t camera3buffer,\n> >  \t}\n> >  }\n> >\n> > -CameraStream::CameraStream(PixelFormat format, Size size,\n> > +CameraStream::CameraStream(PixelFormat format, Size size, Type type,\n> >  \t\t\t   unsigned int index, Encoder *encoder)\n> > -\t: format_(format), size_(size), index_(index), encoder_(encoder)\n> > +\n> > +\t: format_(format), size_(size), type_(type), index_(index),\n> > +\t  encoder_(encoder)\n> >  {\n> >  }\n> >\n> > @@ -1222,12 +1224,14 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> >\n> >  \t\tconfig_->addConfiguration(streamConfiguration);\n> >  \t\tunsigned int index = config_->size() - 1;\n> > -\t\tstreams_.emplace_back(format, size, index);\n> > +\t\tstreams_.emplace_back(format, size, CameraStream::Type::Direct,\n> > +\t\t\t\t      index);\n> >  \t\tstream->priv = static_cast<void *>(&streams_.back());\n> >  \t}\n> >\n> >  \t/* Now handle the MJPEG streams, adding a new stream if required. */\n> >  \tif (jpegStream) {\n> > +\t\tCameraStream::Type type;\n> >  \t\tint index = -1;\n> >\n> >  \t\t/* Search for a compatible stream in the non-JPEG ones. */\n> > @@ -1245,6 +1249,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> >  \t\t\tLOG(HAL, Info)\n> >  \t\t\t\t<< \"Android JPEG stream mapped to libcamera stream \" << i;\n> >\n> > +\t\t\ttype = CameraStream::Type::Mapped;\n> >  \t\t\tindex = i;\n> >  \t\t\tbreak;\n> >  \t\t}\n> > @@ -1269,6 +1274,7 @@ 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\ttype = CameraStream::Type::Internal;\n> >  \t\t\tconfig_->addConfiguration(streamConfiguration);\n> >  \t\t\tindex = config_->size() - 1;\n> >  \t\t}\n> > @@ -1287,7 +1293,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> >  \t\t\treturn ret;\n> >  \t\t}\n> >\n> > -\t\tstreams_.emplace_back(formats::MJPEG, cfg.size, index, encoder);\n> > +\t\tstreams_.emplace_back(formats::MJPEG, cfg.size, type, index, encoder);\n> >  \t\tjpegStream->priv = static_cast<void *>(&streams_.back());\n> >  \t}\n> >\n> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > index 1837748d2efc..9a9406cc257c 100644\n> > --- a/src/android/camera_device.h\n> > +++ b/src/android/camera_device.h\n> > @@ -30,17 +30,102 @@ class CameraMetadata;\n> >  class CameraStream\n> >  {\n> >  public:\n> > +\t/*\n> > +\t * Enumeration of CameraStream types.\n> > +\t *\n> > +\t * A camera stream associates an Android stream to a libcamera stream.\n> > +\t * This enumeration describes how the two streams are associated and how\n> > +\t * and where data produced from libcamera are delivered to the\n> > +\t * Android framework.\n> > +\t *\n> > +\t * Direct:\n> > +\t *\n> > +\t * The Android stream is directly mapped onto a libcamera stream: frames\n> > +\t * are delivered by the library directly in the memory location\n> > +\t * specified by the Android stream (buffer_handle_t->data) and provided\n> > +\t * to the framework as they are. The Android stream characteristics are\n> > +\t * directly translated to the libcamera stream configuration.\n> > +\t *\n> > +\t * +-----+                +-----+\n> > +\t * |  A  |                |  L  |\n> > +\t * +-----+                +-----+\n> > +\t *    |                      |\n> > +\t *    V                      V\n> > +\t * +-----+                +------+\n> > +\t * |  B  |<---------------|  FB  |\n> > +\t * +-----+                +------+\n> > +\t *\n> > +\t *\n> > +\t * Internal:\n> > +\t *\n> > +\t * Data for the Android stream is produced by processing a libcamera\n> > +\t * stream created by the HAL for that purpose. The libcamera stream\n> > +\t * needs to be supplied with intermediate buffers where the library\n> > +\t * delivers frames to be processed and then provided to the framework.\n> > +\t * The libcamera stream configuration is not a direct translation of the\n> > +\t * Android stream characteristics, but it describes the format and size\n> > +\t * required for the processing procedure to produce frames in the\n> > +\t * Android required format.\n> > +\t *\n> > +\t * +-----+                +-----+    +-----+\n> > +\t * |  A  |                |  L  |--->|  FB |\n> > +\t * +-----+                +-----+    +-----+\n> > +\t *    |                      |         |\n> > +\t *    V                      V         |\n> > +\t * +-----+                +------+     |\n> > +\t * |  B  |                |   B  |<----+\n> > +\t * +-----+                +------+\n> > +\t *   ^                       |\n> > +\t *   |-------Processing------|\n>\n> Maybe there's something I don't get here, but isn't the processing from\n> FB to B, not from B to B ? Shouldn't it look like this ?\n>\n> \t * +-----+                +-----+\n> \t * |  A  |                |  L  |\n> \t * +-----+                +-----+\n> \t *    |                      |\n> \t *    V                      V\n> \t * +-----+                +------+\n> \t * |  B  |                |  FB  |\n> \t * +-----+                +------+\n> \t *    ^                      |\n> \t *    |------Processing------|\n\nWell, as we haven't really formalized what the arrow means at all, and\nthis diagram is just for reference, I guess we can play it as we like\nthe most. The original meant that that libcamera creates a framebuffer\nthat wraps a buffer allocated by libcamera. I can remove the right\npart of the drawing if it's confusing.\n\n>\n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nThanks\n  j\n\n>\n> > +\t *\n> > +\t *\n> > +\t * Mapped:\n> > +\t *\n> > +\t * Data for the Android stream is produced by processing a libcamera\n> > +\t * stream associated with another CameraStream. Mapped camera streams do\n> > +\t * not need any memory to be reserved for them as they process data\n> > +\t * produced by libcamera for a different stream whose format and size\n> > +\t * is compatible with the processing procedure requirements to produce\n> > +\t * frames in the Android required format.\n> > +\t *\n> > +\t * +-----+      +-----+          +-----+\n> > +\t * |  A  |      |  A' |          |  L  |\n> > +\t * +-----+      +-----+          +-----+\n> > +\t *    |            |                |\n> > +\t *    V            V                V\n> > +\t * +-----+      +-----+          +------+\n> > +\t * |  B  |      |  B' |<---------|  FB  |\n> > +\t * +-----+      +-----+          +------+\n> > +\t *   ^              |\n> > +\t *   |--Processing--|\n> > +\t *\n> > +\t *\n> > +\t * --------------------------------------------------------------------\n> > +\t * A  = Android stream\n> > +\t * L  = libcamera stream\n> > +\t * B  = memory buffer\n> > +\t * FB = libcamera FrameBuffer\n> > +\t * \"Processing\" = Frame processing procedure (Encoding, scaling etc)\n> > +\t */\n> > +\tenum class Type {\n> > +\t\tDirect,\n> > +\t\tInternal,\n> > +\t\tMapped,\n> > +\t};\n> > +\n> >  \tCameraStream(libcamera::PixelFormat format, libcamera::Size size,\n> > -\t\t     unsigned int index, Encoder *encoder = nullptr);\n> > +\t\t     Type type, unsigned int index, Encoder *encoder = nullptr);\n> >\n> >  \tconst libcamera::PixelFormat &format() const { return format_; }\n> >  \tconst libcamera::Size &size() const { return size_; }\n> > +\tType type() const { return type_; }\n> >  \tunsigned int index() const { return index_; }\n> >  \tEncoder *encoder() const { return encoder_.get(); }\n> >\n> >  private:\n> >  \tlibcamera::PixelFormat format_;\n> >  \tlibcamera::Size size_;\n> > +\tType type_;\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>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 8B726C3B5C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 30 Sep 2020 09:10:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1503A6220F;\n\tWed, 30 Sep 2020 11:10:49 +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 9A7EF621FC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 30 Sep 2020 11:10:47 +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 relay9-d.mail.gandi.net (Postfix) with ESMTPSA id D5603FF811;\n\tWed, 30 Sep 2020 09:10:46 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Wed, 30 Sep 2020 11:14:43 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20200930091443.6kolei74osa7zmto@uno.localdomain>","References":"<20200922094738.5327-1-jacopo@jmondi.org>\n\t<20200922094738.5327-2-jacopo@jmondi.org>\n\t<20200929012824.GI14614@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200929012824.GI14614@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 1/8] android: camera_device: Add\n\tCameraStream::Type","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":"hanlinchen@chromium.org, libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12925,"web_url":"https://patchwork.libcamera.org/comment/12925/","msgid":"<20201002015821.GS3722@pendragon.ideasonboard.com>","date":"2020-10-02T01:58:21","subject":"Re: [libcamera-devel] [PATCH v3 1/8] android: camera_device: Add\n\tCameraStream::Type","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Wed, Sep 30, 2020 at 11:14:43AM +0200, Jacopo Mondi wrote:\n> On Tue, Sep 29, 2020 at 04:28:24AM +0300, Laurent Pinchart wrote:\n> > On Tue, Sep 22, 2020 at 11:47:31AM +0200, Jacopo Mondi wrote:\n> > > Define the CameraStream::Type enumeration and assign it to\n> > > each CameraStream instance at construction time.\n> > >\n> > > The CameraStream type will be used to decide if memory needs to be\n> > > allocated on its behalf or if the stream is backed by memory externally\n> > > allocated by the Android framework.\n> > >\n> > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > > Reviewed-by: Hirokazu Honda <hiroh@chromium.org>\n> > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > > ---\n> > >  src/android/camera_device.cpp | 14 ++++--\n> > >  src/android/camera_device.h   | 87 ++++++++++++++++++++++++++++++++++-\n> > >  2 files changed, 96 insertions(+), 5 deletions(-)\n> > >\n> > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > index 70d77a17ef43..e4ffbc02c2da 100644\n> > > --- a/src/android/camera_device.cpp\n> > > +++ b/src/android/camera_device.cpp\n> > > @@ -169,9 +169,11 @@ MappedCamera3Buffer::MappedCamera3Buffer(const buffer_handle_t camera3buffer,\n> > >  \t}\n> > >  }\n> > >\n> > > -CameraStream::CameraStream(PixelFormat format, Size size,\n> > > +CameraStream::CameraStream(PixelFormat format, Size size, Type type,\n> > >  \t\t\t   unsigned int index, Encoder *encoder)\n> > > -\t: format_(format), size_(size), index_(index), encoder_(encoder)\n> > > +\n> > > +\t: format_(format), size_(size), type_(type), index_(index),\n> > > +\t  encoder_(encoder)\n> > >  {\n> > >  }\n> > >\n> > > @@ -1222,12 +1224,14 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > >\n> > >  \t\tconfig_->addConfiguration(streamConfiguration);\n> > >  \t\tunsigned int index = config_->size() - 1;\n> > > -\t\tstreams_.emplace_back(format, size, index);\n> > > +\t\tstreams_.emplace_back(format, size, CameraStream::Type::Direct,\n> > > +\t\t\t\t      index);\n> > >  \t\tstream->priv = static_cast<void *>(&streams_.back());\n> > >  \t}\n> > >\n> > >  \t/* Now handle the MJPEG streams, adding a new stream if required. */\n> > >  \tif (jpegStream) {\n> > > +\t\tCameraStream::Type type;\n> > >  \t\tint index = -1;\n> > >\n> > >  \t\t/* Search for a compatible stream in the non-JPEG ones. */\n> > > @@ -1245,6 +1249,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > >  \t\t\tLOG(HAL, Info)\n> > >  \t\t\t\t<< \"Android JPEG stream mapped to libcamera stream \" << i;\n> > >\n> > > +\t\t\ttype = CameraStream::Type::Mapped;\n> > >  \t\t\tindex = i;\n> > >  \t\t\tbreak;\n> > >  \t\t}\n> > > @@ -1269,6 +1274,7 @@ 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\ttype = CameraStream::Type::Internal;\n> > >  \t\t\tconfig_->addConfiguration(streamConfiguration);\n> > >  \t\t\tindex = config_->size() - 1;\n> > >  \t\t}\n> > > @@ -1287,7 +1293,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > >  \t\t\treturn ret;\n> > >  \t\t}\n> > >\n> > > -\t\tstreams_.emplace_back(formats::MJPEG, cfg.size, index, encoder);\n> > > +\t\tstreams_.emplace_back(formats::MJPEG, cfg.size, type, index, encoder);\n> > >  \t\tjpegStream->priv = static_cast<void *>(&streams_.back());\n> > >  \t}\n> > >\n> > > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > > index 1837748d2efc..9a9406cc257c 100644\n> > > --- a/src/android/camera_device.h\n> > > +++ b/src/android/camera_device.h\n> > > @@ -30,17 +30,102 @@ class CameraMetadata;\n> > >  class CameraStream\n> > >  {\n> > >  public:\n> > > +\t/*\n> > > +\t * Enumeration of CameraStream types.\n> > > +\t *\n> > > +\t * A camera stream associates an Android stream to a libcamera stream.\n> > > +\t * This enumeration describes how the two streams are associated and how\n> > > +\t * and where data produced from libcamera are delivered to the\n> > > +\t * Android framework.\n> > > +\t *\n> > > +\t * Direct:\n> > > +\t *\n> > > +\t * The Android stream is directly mapped onto a libcamera stream: frames\n> > > +\t * are delivered by the library directly in the memory location\n> > > +\t * specified by the Android stream (buffer_handle_t->data) and provided\n> > > +\t * to the framework as they are. The Android stream characteristics are\n> > > +\t * directly translated to the libcamera stream configuration.\n> > > +\t *\n> > > +\t * +-----+                +-----+\n> > > +\t * |  A  |                |  L  |\n> > > +\t * +-----+                +-----+\n> > > +\t *    |                      |\n> > > +\t *    V                      V\n> > > +\t * +-----+                +------+\n> > > +\t * |  B  |<---------------|  FB  |\n> > > +\t * +-----+                +------+\n> > > +\t *\n> > > +\t *\n> > > +\t * Internal:\n> > > +\t *\n> > > +\t * Data for the Android stream is produced by processing a libcamera\n> > > +\t * stream created by the HAL for that purpose. The libcamera stream\n> > > +\t * needs to be supplied with intermediate buffers where the library\n> > > +\t * delivers frames to be processed and then provided to the framework.\n> > > +\t * The libcamera stream configuration is not a direct translation of the\n> > > +\t * Android stream characteristics, but it describes the format and size\n> > > +\t * required for the processing procedure to produce frames in the\n> > > +\t * Android required format.\n> > > +\t *\n> > > +\t * +-----+                +-----+    +-----+\n> > > +\t * |  A  |                |  L  |--->|  FB |\n> > > +\t * +-----+                +-----+    +-----+\n> > > +\t *    |                      |         |\n> > > +\t *    V                      V         |\n> > > +\t * +-----+                +------+     |\n> > > +\t * |  B  |                |   B  |<----+\n> > > +\t * +-----+                +------+\n> > > +\t *   ^                       |\n> > > +\t *   |-------Processing------|\n> >\n> > Maybe there's something I don't get here, but isn't the processing from\n> > FB to B, not from B to B ? Shouldn't it look like this ?\n> >\n> > \t * +-----+                +-----+\n> > \t * |  A  |                |  L  |\n> > \t * +-----+                +-----+\n> > \t *    |                      |\n> > \t *    V                      V\n> > \t * +-----+                +------+\n> > \t * |  B  |                |  FB  |\n> > \t * +-----+                +------+\n> > \t *    ^                      |\n> > \t *    |------Processing------|\n> \n> Well, as we haven't really formalized what the arrow means at all, and\n> this diagram is just for reference, I guess we can play it as we like\n> the most. The original meant that that libcamera creates a framebuffer\n> that wraps a buffer allocated by libcamera. I can remove the right\n> part of the drawing if it's confusing.\n\nAh I get it now. I don't mind keeping it, but I think the arrows should\nbe explained then. I was confused :-)\n\n> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n> Thanks\n> \n> > > +\t *\n> > > +\t *\n> > > +\t * Mapped:\n> > > +\t *\n> > > +\t * Data for the Android stream is produced by processing a libcamera\n> > > +\t * stream associated with another CameraStream. Mapped camera streams do\n> > > +\t * not need any memory to be reserved for them as they process data\n> > > +\t * produced by libcamera for a different stream whose format and size\n> > > +\t * is compatible with the processing procedure requirements to produce\n> > > +\t * frames in the Android required format.\n> > > +\t *\n> > > +\t * +-----+      +-----+          +-----+\n> > > +\t * |  A  |      |  A' |          |  L  |\n> > > +\t * +-----+      +-----+          +-----+\n> > > +\t *    |            |                |\n> > > +\t *    V            V                V\n> > > +\t * +-----+      +-----+          +------+\n> > > +\t * |  B  |      |  B' |<---------|  FB  |\n> > > +\t * +-----+      +-----+          +------+\n> > > +\t *   ^              |\n> > > +\t *   |--Processing--|\n> > > +\t *\n> > > +\t *\n> > > +\t * --------------------------------------------------------------------\n> > > +\t * A  = Android stream\n> > > +\t * L  = libcamera stream\n> > > +\t * B  = memory buffer\n> > > +\t * FB = libcamera FrameBuffer\n> > > +\t * \"Processing\" = Frame processing procedure (Encoding, scaling etc)\n> > > +\t */\n> > > +\tenum class Type {\n> > > +\t\tDirect,\n> > > +\t\tInternal,\n> > > +\t\tMapped,\n> > > +\t};\n> > > +\n> > >  \tCameraStream(libcamera::PixelFormat format, libcamera::Size size,\n> > > -\t\t     unsigned int index, Encoder *encoder = nullptr);\n> > > +\t\t     Type type, unsigned int index, Encoder *encoder = nullptr);\n> > >\n> > >  \tconst libcamera::PixelFormat &format() const { return format_; }\n> > >  \tconst libcamera::Size &size() const { return size_; }\n> > > +\tType type() const { return type_; }\n> > >  \tunsigned int index() const { return index_; }\n> > >  \tEncoder *encoder() const { return encoder_.get(); }\n> > >\n> > >  private:\n> > >  \tlibcamera::PixelFormat format_;\n> > >  \tlibcamera::Size size_;\n> > > +\tType type_;\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","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 E3509C3B5C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  2 Oct 2020 01:59:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7A0476229F;\n\tFri,  2 Oct 2020 03:59:00 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2857C60BCD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  2 Oct 2020 03:58:59 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8F3FC528;\n\tFri,  2 Oct 2020 03:58:58 +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=\"WtVsBfIl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1601603938;\n\tbh=FhcBFNgZFos6PTpiCLlgXOXEeNOb+SIvzYlHOnHiDuo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=WtVsBfIlOS3oXU0sVIPYEeXV92w2oMNuF9AsUmIAu0jFUjWir6LddwPjRXTzY4Jbw\n\tALtC4/2W2hcJkYGEi20MfoEbciQ4saFwrNaydI+3MZRR2X0ZMMjsHi0IYf8f1Vjtow\n\tcuJs3ha7C9Te8VYMxIcyKpbIA2vKmcq5Ku1BSTnE=","Date":"Fri, 2 Oct 2020 04:58:21 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<20201002015821.GS3722@pendragon.ideasonboard.com>","References":"<20200922094738.5327-1-jacopo@jmondi.org>\n\t<20200922094738.5327-2-jacopo@jmondi.org>\n\t<20200929012824.GI14614@pendragon.ideasonboard.com>\n\t<20200930091443.6kolei74osa7zmto@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200930091443.6kolei74osa7zmto@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v3 1/8] android: camera_device: Add\n\tCameraStream::Type","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":"hanlinchen@chromium.org, libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12940,"web_url":"https://patchwork.libcamera.org/comment/12940/","msgid":"<20201002140649.j4bqx7seubtuegmi@uno.localdomain>","date":"2020-10-02T14:06:49","subject":"Re: [libcamera-devel] [PATCH v3 1/8] android: camera_device: Add\n\tCameraStream::Type","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Fri, Oct 02, 2020 at 04:58:21AM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Wed, Sep 30, 2020 at 11:14:43AM +0200, Jacopo Mondi wrote:\n> > On Tue, Sep 29, 2020 at 04:28:24AM +0300, Laurent Pinchart wrote:\n> > > On Tue, Sep 22, 2020 at 11:47:31AM +0200, Jacopo Mondi wrote:\n> > > > Define the CameraStream::Type enumeration and assign it to\n> > > > each CameraStream instance at construction time.\n> > > >\n> > > > The CameraStream type will be used to decide if memory needs to be\n> > > > allocated on its behalf or if the stream is backed by memory externally\n> > > > allocated by the Android framework.\n> > > >\n> > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > > > Reviewed-by: Hirokazu Honda <hiroh@chromium.org>\n> > > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > > > ---\n> > > >  src/android/camera_device.cpp | 14 ++++--\n> > > >  src/android/camera_device.h   | 87 ++++++++++++++++++++++++++++++++++-\n> > > >  2 files changed, 96 insertions(+), 5 deletions(-)\n> > > >\n> > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > > index 70d77a17ef43..e4ffbc02c2da 100644\n> > > > --- a/src/android/camera_device.cpp\n> > > > +++ b/src/android/camera_device.cpp\n> > > > @@ -169,9 +169,11 @@ MappedCamera3Buffer::MappedCamera3Buffer(const buffer_handle_t camera3buffer,\n> > > >  \t}\n> > > >  }\n> > > >\n> > > > -CameraStream::CameraStream(PixelFormat format, Size size,\n> > > > +CameraStream::CameraStream(PixelFormat format, Size size, Type type,\n> > > >  \t\t\t   unsigned int index, Encoder *encoder)\n> > > > -\t: format_(format), size_(size), index_(index), encoder_(encoder)\n> > > > +\n> > > > +\t: format_(format), size_(size), type_(type), index_(index),\n> > > > +\t  encoder_(encoder)\n> > > >  {\n> > > >  }\n> > > >\n> > > > @@ -1222,12 +1224,14 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > > >\n> > > >  \t\tconfig_->addConfiguration(streamConfiguration);\n> > > >  \t\tunsigned int index = config_->size() - 1;\n> > > > -\t\tstreams_.emplace_back(format, size, index);\n> > > > +\t\tstreams_.emplace_back(format, size, CameraStream::Type::Direct,\n> > > > +\t\t\t\t      index);\n> > > >  \t\tstream->priv = static_cast<void *>(&streams_.back());\n> > > >  \t}\n> > > >\n> > > >  \t/* Now handle the MJPEG streams, adding a new stream if required. */\n> > > >  \tif (jpegStream) {\n> > > > +\t\tCameraStream::Type type;\n> > > >  \t\tint index = -1;\n> > > >\n> > > >  \t\t/* Search for a compatible stream in the non-JPEG ones. */\n> > > > @@ -1245,6 +1249,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > > >  \t\t\tLOG(HAL, Info)\n> > > >  \t\t\t\t<< \"Android JPEG stream mapped to libcamera stream \" << i;\n> > > >\n> > > > +\t\t\ttype = CameraStream::Type::Mapped;\n> > > >  \t\t\tindex = i;\n> > > >  \t\t\tbreak;\n> > > >  \t\t}\n> > > > @@ -1269,6 +1274,7 @@ 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\ttype = CameraStream::Type::Internal;\n> > > >  \t\t\tconfig_->addConfiguration(streamConfiguration);\n> > > >  \t\t\tindex = config_->size() - 1;\n> > > >  \t\t}\n> > > > @@ -1287,7 +1293,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > > >  \t\t\treturn ret;\n> > > >  \t\t}\n> > > >\n> > > > -\t\tstreams_.emplace_back(formats::MJPEG, cfg.size, index, encoder);\n> > > > +\t\tstreams_.emplace_back(formats::MJPEG, cfg.size, type, index, encoder);\n> > > >  \t\tjpegStream->priv = static_cast<void *>(&streams_.back());\n> > > >  \t}\n> > > >\n> > > > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > > > index 1837748d2efc..9a9406cc257c 100644\n> > > > --- a/src/android/camera_device.h\n> > > > +++ b/src/android/camera_device.h\n> > > > @@ -30,17 +30,102 @@ class CameraMetadata;\n> > > >  class CameraStream\n> > > >  {\n> > > >  public:\n> > > > +\t/*\n> > > > +\t * Enumeration of CameraStream types.\n> > > > +\t *\n> > > > +\t * A camera stream associates an Android stream to a libcamera stream.\n> > > > +\t * This enumeration describes how the two streams are associated and how\n> > > > +\t * and where data produced from libcamera are delivered to the\n> > > > +\t * Android framework.\n> > > > +\t *\n> > > > +\t * Direct:\n> > > > +\t *\n> > > > +\t * The Android stream is directly mapped onto a libcamera stream: frames\n> > > > +\t * are delivered by the library directly in the memory location\n> > > > +\t * specified by the Android stream (buffer_handle_t->data) and provided\n> > > > +\t * to the framework as they are. The Android stream characteristics are\n> > > > +\t * directly translated to the libcamera stream configuration.\n> > > > +\t *\n> > > > +\t * +-----+                +-----+\n> > > > +\t * |  A  |                |  L  |\n> > > > +\t * +-----+                +-----+\n> > > > +\t *    |                      |\n> > > > +\t *    V                      V\n> > > > +\t * +-----+                +------+\n> > > > +\t * |  B  |<---------------|  FB  |\n> > > > +\t * +-----+                +------+\n> > > > +\t *\n> > > > +\t *\n> > > > +\t * Internal:\n> > > > +\t *\n> > > > +\t * Data for the Android stream is produced by processing a libcamera\n> > > > +\t * stream created by the HAL for that purpose. The libcamera stream\n> > > > +\t * needs to be supplied with intermediate buffers where the library\n> > > > +\t * delivers frames to be processed and then provided to the framework.\n> > > > +\t * The libcamera stream configuration is not a direct translation of the\n> > > > +\t * Android stream characteristics, but it describes the format and size\n> > > > +\t * required for the processing procedure to produce frames in the\n> > > > +\t * Android required format.\n> > > > +\t *\n> > > > +\t * +-----+                +-----+    +-----+\n> > > > +\t * |  A  |                |  L  |--->|  FB |\n> > > > +\t * +-----+                +-----+    +-----+\n> > > > +\t *    |                      |         |\n> > > > +\t *    V                      V         |\n> > > > +\t * +-----+                +------+     |\n> > > > +\t * |  B  |                |   B  |<----+\n> > > > +\t * +-----+                +------+\n> > > > +\t *   ^                       |\n> > > > +\t *   |-------Processing------|\n> > >\n> > > Maybe there's something I don't get here, but isn't the processing from\n> > > FB to B, not from B to B ? Shouldn't it look like this ?\n> > >\n> > > \t * +-----+                +-----+\n> > > \t * |  A  |                |  L  |\n> > > \t * +-----+                +-----+\n> > > \t *    |                      |\n> > > \t *    V                      V\n> > > \t * +-----+                +------+\n> > > \t * |  B  |                |  FB  |\n> > > \t * +-----+                +------+\n> > > \t *    ^                      |\n> > > \t *    |------Processing------|\n> >\n> > Well, as we haven't really formalized what the arrow means at all, and\n> > this diagram is just for reference, I guess we can play it as we like\n> > the most. The original meant that that libcamera creates a framebuffer\n> > that wraps a buffer allocated by libcamera. I can remove the right\n> > part of the drawing if it's confusing.\n>\n> Ah I get it now. I don't mind keeping it, but I think the arrows should\n> be explained then. I was confused :-)\n\nNo worries, I've adopted what you have proposed in v4 which I sent to\nthe list!\n\nThanks\n  j\n\n>\n> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> >\n> > Thanks\n> >\n> > > > +\t *\n> > > > +\t *\n> > > > +\t * Mapped:\n> > > > +\t *\n> > > > +\t * Data for the Android stream is produced by processing a libcamera\n> > > > +\t * stream associated with another CameraStream. Mapped camera streams do\n> > > > +\t * not need any memory to be reserved for them as they process data\n> > > > +\t * produced by libcamera for a different stream whose format and size\n> > > > +\t * is compatible with the processing procedure requirements to produce\n> > > > +\t * frames in the Android required format.\n> > > > +\t *\n> > > > +\t * +-----+      +-----+          +-----+\n> > > > +\t * |  A  |      |  A' |          |  L  |\n> > > > +\t * +-----+      +-----+          +-----+\n> > > > +\t *    |            |                |\n> > > > +\t *    V            V                V\n> > > > +\t * +-----+      +-----+          +------+\n> > > > +\t * |  B  |      |  B' |<---------|  FB  |\n> > > > +\t * +-----+      +-----+          +------+\n> > > > +\t *   ^              |\n> > > > +\t *   |--Processing--|\n> > > > +\t *\n> > > > +\t *\n> > > > +\t * --------------------------------------------------------------------\n> > > > +\t * A  = Android stream\n> > > > +\t * L  = libcamera stream\n> > > > +\t * B  = memory buffer\n> > > > +\t * FB = libcamera FrameBuffer\n> > > > +\t * \"Processing\" = Frame processing procedure (Encoding, scaling etc)\n> > > > +\t */\n> > > > +\tenum class Type {\n> > > > +\t\tDirect,\n> > > > +\t\tInternal,\n> > > > +\t\tMapped,\n> > > > +\t};\n> > > > +\n> > > >  \tCameraStream(libcamera::PixelFormat format, libcamera::Size size,\n> > > > -\t\t     unsigned int index, Encoder *encoder = nullptr);\n> > > > +\t\t     Type type, unsigned int index, Encoder *encoder = nullptr);\n> > > >\n> > > >  \tconst libcamera::PixelFormat &format() const { return format_; }\n> > > >  \tconst libcamera::Size &size() const { return size_; }\n> > > > +\tType type() const { return type_; }\n> > > >  \tunsigned int index() const { return index_; }\n> > > >  \tEncoder *encoder() const { return encoder_.get(); }\n> > > >\n> > > >  private:\n> > > >  \tlibcamera::PixelFormat format_;\n> > > >  \tlibcamera::Size size_;\n> > > > +\tType type_;\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>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 4E646C3B5C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  2 Oct 2020 14:02:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B38F163B6E;\n\tFri,  2 Oct 2020 16:02:53 +0200 (CEST)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EF67260360\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  2 Oct 2020 16:02:52 +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 relay11.mail.gandi.net (Postfix) with ESMTPSA id 0E2FF100012;\n\tFri,  2 Oct 2020 14:02:51 +0000 (UTC)"],"Date":"Fri, 2 Oct 2020 16:06:49 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20201002140649.j4bqx7seubtuegmi@uno.localdomain>","References":"<20200922094738.5327-1-jacopo@jmondi.org>\n\t<20200922094738.5327-2-jacopo@jmondi.org>\n\t<20200929012824.GI14614@pendragon.ideasonboard.com>\n\t<20200930091443.6kolei74osa7zmto@uno.localdomain>\n\t<20201002015821.GS3722@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201002015821.GS3722@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 1/8] android: camera_device: Add\n\tCameraStream::Type","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":"hanlinchen@chromium.org, libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]