[{"id":14707,"web_url":"https://patchwork.libcamera.org/comment/14707/","msgid":"<fb7ec7e9-f76a-0391-5e06-45574dc810fd@ideasonboard.com>","date":"2021-01-22T12:51:27","subject":"Re: [libcamera-devel] [PATCH 1/2] android: post_processor: Change\n\tthe type destination in process()","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Hiro,\n\nOn 22/01/2021 09:23, Hirokazu Honda wrote:\n> The type of the destination buffer in PostProcessor::process() is\n> libcamera::Span. libcamera::Span is used for one dimension buffer\n> (e.g. blob buffer). The destination can be multiple dimensions\n> buffer (e.g. yuv frame). Therefore, this changes the type of the\n> destination buffer to MappedFrameBuffer.\n> \n> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>\n> ---\n>  src/android/camera_stream.cpp            | 5 +++--\n>  src/android/camera_stream.h              | 5 +++--\n>  src/android/jpeg/post_processor_jpeg.cpp | 7 ++++---\n>  src/android/jpeg/post_processor_jpeg.h   | 2 +-\n>  src/android/post_processor.h             | 3 ++-\n>  5 files changed, 13 insertions(+), 9 deletions(-)\n> \n> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> index dba351a4..33d0e005 100644\n> --- a/src/android/camera_stream.cpp\n> +++ b/src/android/camera_stream.cpp\n> @@ -96,12 +96,13 @@ int CameraStream::configure()\n>  }\n> \n>  int CameraStream::process(const libcamera::FrameBuffer &source,\n> -\t\t\t  MappedCamera3Buffer *dest, CameraMetadata *metadata)\n> +\t\t\t  libcamera::MappedBuffer *destination,\n> +\t\t\t  CameraMetadata *metadata)\n>  {\n>  \tif (!postProcessor_)\n>  \t\treturn 0;\n> \n> -\treturn postProcessor_->process(source, dest->maps()[0], metadata);\n> +\treturn postProcessor_->process(source, destination, metadata);\n>  }\n> \n>  FrameBuffer *CameraStream::getBuffer()\n> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> index cc9d5470..7d7b2b16 100644\n> --- a/src/android/camera_stream.h\n> +++ b/src/android/camera_stream.h\n> @@ -17,11 +17,11 @@\n>  #include <libcamera/camera.h>\n>  #include <libcamera/framebuffer_allocator.h>\n>  #include <libcamera/geometry.h>\n> +#include <libcamera/internal/buffer.h>\n\ninternal includes should have their own include block separately, after\nthe other libcamera headers.\n\n\n>  #include <libcamera/pixel_format.h>\n> \n\nso it would go here. with a blank line each side.\n\n>  class CameraDevice;\n>  class CameraMetadata;\n> -class MappedCamera3Buffer;\n>  class PostProcessor;\n> \n>  class CameraStream\n> @@ -119,7 +119,8 @@ public:\n> \n>  \tint configure();\n>  \tint process(const libcamera::FrameBuffer &source,\n> -\t\t    MappedCamera3Buffer *dest, CameraMetadata *metadata);\n> +\t\t    libcamera::MappedBuffer *destination,\n> +\t\t    CameraMetadata *metadata);\n>  \tlibcamera::FrameBuffer *getBuffer();\n>  \tvoid putBuffer(libcamera::FrameBuffer *buffer);\n> \n> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp\n> index 436a50f8..2374716d 100644\n> --- a/src/android/jpeg/post_processor_jpeg.cpp\n> +++ b/src/android/jpeg/post_processor_jpeg.cpp\n> @@ -79,7 +79,7 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,\n>  }\n> \n>  int PostProcessorJpeg::process(const FrameBuffer &source,\n> -\t\t\t       Span<uint8_t> destination,\n> +\t\t\t       libcamera::MappedBuffer *destination,\n>  \t\t\t       CameraMetadata *metadata)\n>  {\n>  \tif (!encoder_)\n> @@ -107,7 +107,8 @@ int PostProcessorJpeg::process(const FrameBuffer &source,\n>  \tif (exif.generate() != 0)\n>  \t\tLOG(JPEG, Error) << \"Failed to generate valid EXIF data\";\n> \n> -\tint jpeg_size = encoder_->encode(source, destination, exif.data());\n> +\tint jpeg_size = encoder_->encode(source, destination->maps()[0],\n> +\t\t\t\t\t exif.data());\n>  \tif (jpeg_size < 0) {\n>  \t\tLOG(JPEG, Error) << \"Failed to encode stream image\";\n>  \t\treturn jpeg_size;\n> @@ -125,7 +126,7 @@ int PostProcessorJpeg::process(const FrameBuffer &source,\n>  \t * \\todo Investigate if the buffer size mismatch is an issue or\n>  \t * expected behaviour.\n>  \t */\n> -\tuint8_t *resultPtr = destination.data() +\n> +\tuint8_t *resultPtr = destination->maps()[0].data() +\n>  \t\t\t     cameraDevice_->maxJpegBufferSize() -\n>  \t\t\t     sizeof(struct camera3_jpeg_blob);\n>  \tauto *blob = reinterpret_cast<struct camera3_jpeg_blob *>(resultPtr);\n> diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h\n> index 5afa831c..68a38410 100644\n> --- a/src/android/jpeg/post_processor_jpeg.h\n> +++ b/src/android/jpeg/post_processor_jpeg.h\n> @@ -25,7 +25,7 @@ public:\n>  \tint configure(const libcamera::StreamConfiguration &incfg,\n>  \t\t      const libcamera::StreamConfiguration &outcfg) override;\n>  \tint process(const libcamera::FrameBuffer &source,\n> -\t\t    libcamera::Span<uint8_t> destination,\n> +\t\t    libcamera::MappedBuffer *destination,\n>  \t\t    CameraMetadata *metadata) override;\n> \n>  private:\n> diff --git a/src/android/post_processor.h b/src/android/post_processor.h\n> index e0f91880..368e0fe5 100644\n> --- a/src/android/post_processor.h\n> +++ b/src/android/post_processor.h\n> @@ -7,6 +7,7 @@\n>  #ifndef __ANDROID_POST_PROCESSOR_H__\n>  #define __ANDROID_POST_PROCESSOR_H__\n> \n> +#include <libcamera/internal/buffer.h>\n\nSame here, this should go in it's own block after this one.\n\nExcept for the header locations, which is quite trivial, this looks ok\nto me.\n\nWe might expect discussion of using internal headers in the Android\nlayer, but actually we are already using internal components there so I\ndon't think this is an issue.\n\nIn fact of course we're already using the MappedBuffers as part of the\nMappedCamera3Buffer so I think this is fine all round.\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n>  #include <libcamera/buffer.h>\n>  #include <libcamera/span.h>\n>  #include <libcamera/stream.h>> @@ -21,7 +22,7 @@ public:\n>  \tvirtual int configure(const libcamera::StreamConfiguration &inCfg,\n>  \t\t\t      const libcamera::StreamConfiguration &outCfg) = 0;\n>  \tvirtual int process(const libcamera::FrameBuffer &source,\n> -\t\t\t    libcamera::Span<uint8_t> destination,\n> +\t\t\t    libcamera::MappedBuffer *destination,\n>  \t\t\t    CameraMetadata *metadata) = 0;\n>  };\n> \n> --\n> 2.30.0.280.ga3ce27912f-goog\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel\n>","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 38166C0F2A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 22 Jan 2021 12:51:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AF8AC6826B;\n\tFri, 22 Jan 2021 13:51:32 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9E5526825B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 22 Jan 2021 13:51:30 +0100 (CET)","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 1060E4FB;\n\tFri, 22 Jan 2021 13:51:30 +0100 (CET)"],"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=\"LYTD6woE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1611319890;\n\tbh=FeIB6y9j0e9uxZM0xpVlw2XR8Ad9D/TLjggq+MqlsbU=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=LYTD6woE4Bo4Q8VvD55zh+km83iBVM9mt7QMUEl4QVTXW9ULOKP2SkBr/h9XwikHw\n\tSGL4aoqp3tY1y3PQEBye7eZ/CD0bL/prJyqeEocssfXFDVJgwJkAKt99s2Dm8AbVmu\n\tqh4Y9mUNzML5PH+QDZHMzrhmwRom8vSttPZFwVJI=","To":"Hirokazu Honda <hiroh@chromium.org>, libcamera-devel@lists.libcamera.org","References":"<20210122092335.254626-1-hiroh@chromium.org>","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":"<fb7ec7e9-f76a-0391-5e06-45574dc810fd@ideasonboard.com>","Date":"Fri, 22 Jan 2021 12:51:27 +0000","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":"<20210122092335.254626-1-hiroh@chromium.org>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 1/2] android: post_processor: Change\n\tthe type destination in process()","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","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":14775,"web_url":"https://patchwork.libcamera.org/comment/14775/","msgid":"<YA/ZI1P8Syq0ZflK@pendragon.ideasonboard.com>","date":"2021-01-26T08:56:03","subject":"Re: [libcamera-devel] [PATCH 1/2] android: post_processor: Change\n\tthe type destination in process()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Hiro,\n\nThank you for the patch.\n\nOn Fri, Jan 22, 2021 at 12:51:27PM +0000, Kieran Bingham wrote:\n> On 22/01/2021 09:23, Hirokazu Honda wrote:\n> > The type of the destination buffer in PostProcessor::process() is\n> > libcamera::Span. libcamera::Span is used for one dimension buffer\n> > (e.g. blob buffer). The destination can be multiple dimensions\n> > buffer (e.g. yuv frame). Therefore, this changes the type of the\n> > destination buffer to MappedFrameBuffer.\n> > \n> > Signed-off-by: Hirokazu Honda <hiroh@chromium.org>\n> > ---\n> >  src/android/camera_stream.cpp            | 5 +++--\n> >  src/android/camera_stream.h              | 5 +++--\n> >  src/android/jpeg/post_processor_jpeg.cpp | 7 ++++---\n> >  src/android/jpeg/post_processor_jpeg.h   | 2 +-\n> >  src/android/post_processor.h             | 3 ++-\n> >  5 files changed, 13 insertions(+), 9 deletions(-)\n> > \n> > diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> > index dba351a4..33d0e005 100644\n> > --- a/src/android/camera_stream.cpp\n> > +++ b/src/android/camera_stream.cpp\n> > @@ -96,12 +96,13 @@ int CameraStream::configure()\n> >  }\n> > \n> >  int CameraStream::process(const libcamera::FrameBuffer &source,\n> > -\t\t\t  MappedCamera3Buffer *dest, CameraMetadata *metadata)\n> > +\t\t\t  libcamera::MappedBuffer *destination,\n> > +\t\t\t  CameraMetadata *metadata)\n> >  {\n> >  \tif (!postProcessor_)\n> >  \t\treturn 0;\n> > \n> > -\treturn postProcessor_->process(source, dest->maps()[0], metadata);\n> > +\treturn postProcessor_->process(source, destination, metadata);\n> >  }\n> > \n> >  FrameBuffer *CameraStream::getBuffer()\n> > diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> > index cc9d5470..7d7b2b16 100644\n> > --- a/src/android/camera_stream.h\n> > +++ b/src/android/camera_stream.h\n> > @@ -17,11 +17,11 @@\n> >  #include <libcamera/camera.h>\n> >  #include <libcamera/framebuffer_allocator.h>\n> >  #include <libcamera/geometry.h>\n> > +#include <libcamera/internal/buffer.h>\n> \n> internal includes should have their own include block separately, after\n> the other libcamera headers.\n> \n> >  #include <libcamera/pixel_format.h>\n> > \n> \n> so it would go here. with a blank line each side.\n> \n> >  class CameraDevice;\n> >  class CameraMetadata;\n> > -class MappedCamera3Buffer;\n> >  class PostProcessor;\n> > \n> >  class CameraStream\n> > @@ -119,7 +119,8 @@ public:\n> > \n> >  \tint configure();\n> >  \tint process(const libcamera::FrameBuffer &source,\n> > -\t\t    MappedCamera3Buffer *dest, CameraMetadata *metadata);\n> > +\t\t    libcamera::MappedBuffer *destination,\n> > +\t\t    CameraMetadata *metadata);\n> >  \tlibcamera::FrameBuffer *getBuffer();\n> >  \tvoid putBuffer(libcamera::FrameBuffer *buffer);\n> > \n> > diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp\n> > index 436a50f8..2374716d 100644\n> > --- a/src/android/jpeg/post_processor_jpeg.cpp\n> > +++ b/src/android/jpeg/post_processor_jpeg.cpp\n> > @@ -79,7 +79,7 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,\n> >  }\n> > \n> >  int PostProcessorJpeg::process(const FrameBuffer &source,\n> > -\t\t\t       Span<uint8_t> destination,\n> > +\t\t\t       libcamera::MappedBuffer *destination,\n> >  \t\t\t       CameraMetadata *metadata)\n> >  {\n> >  \tif (!encoder_)\n> > @@ -107,7 +107,8 @@ int PostProcessorJpeg::process(const FrameBuffer &source,\n> >  \tif (exif.generate() != 0)\n> >  \t\tLOG(JPEG, Error) << \"Failed to generate valid EXIF data\";\n> > \n> > -\tint jpeg_size = encoder_->encode(source, destination, exif.data());\n> > +\tint jpeg_size = encoder_->encode(source, destination->maps()[0],\n> > +\t\t\t\t\t exif.data());\n> >  \tif (jpeg_size < 0) {\n> >  \t\tLOG(JPEG, Error) << \"Failed to encode stream image\";\n> >  \t\treturn jpeg_size;\n> > @@ -125,7 +126,7 @@ int PostProcessorJpeg::process(const FrameBuffer &source,\n> >  \t * \\todo Investigate if the buffer size mismatch is an issue or\n> >  \t * expected behaviour.\n> >  \t */\n> > -\tuint8_t *resultPtr = destination.data() +\n> > +\tuint8_t *resultPtr = destination->maps()[0].data() +\n> >  \t\t\t     cameraDevice_->maxJpegBufferSize() -\n> >  \t\t\t     sizeof(struct camera3_jpeg_blob);\n> >  \tauto *blob = reinterpret_cast<struct camera3_jpeg_blob *>(resultPtr);\n> > diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h\n> > index 5afa831c..68a38410 100644\n> > --- a/src/android/jpeg/post_processor_jpeg.h\n> > +++ b/src/android/jpeg/post_processor_jpeg.h\n> > @@ -25,7 +25,7 @@ public:\n> >  \tint configure(const libcamera::StreamConfiguration &incfg,\n> >  \t\t      const libcamera::StreamConfiguration &outcfg) override;\n> >  \tint process(const libcamera::FrameBuffer &source,\n> > -\t\t    libcamera::Span<uint8_t> destination,\n> > +\t\t    libcamera::MappedBuffer *destination,\n> >  \t\t    CameraMetadata *metadata) override;\n> > \n> >  private:\n> > diff --git a/src/android/post_processor.h b/src/android/post_processor.h\n> > index e0f91880..368e0fe5 100644\n> > --- a/src/android/post_processor.h\n> > +++ b/src/android/post_processor.h\n> > @@ -7,6 +7,7 @@\n> >  #ifndef __ANDROID_POST_PROCESSOR_H__\n> >  #define __ANDROID_POST_PROCESSOR_H__\n> > \n> > +#include <libcamera/internal/buffer.h>\n> \n> Same here, this should go in it's own block after this one.\n> \n> Except for the header locations, which is quite trivial, this looks ok\n> to me.\n> \n> We might expect discussion of using internal headers in the Android\n> layer, but actually we are already using internal components there so I\n> don't think this is an issue.\n\nGiven that the HAL is compiled in the same shared object, I don't think\nit's a big issue :-) We may split it to a separate library in the\nfuture, but it would still be developed in lockstep with the libcamera\ncore, so I don't have any problem using the internal API. The only issue\nI can think about now would be if we wanted to hide the internal API\nfrom the symbols exported by the libcamera core, using\nhttps://gcc.gnu.org/wiki/Visibility.\n\n> In fact of course we're already using the MappedBuffers as part of the\n> MappedCamera3Buffer so I think this is fine all round.\n> \n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>\n> >  #include <libcamera/buffer.h>\n> >  #include <libcamera/span.h>\n\nYou can drop span.h.\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> >  #include <libcamera/stream.h>> @@ -21,7 +22,7 @@ public:\n> >  \tvirtual int configure(const libcamera::StreamConfiguration &inCfg,\n> >  \t\t\t      const libcamera::StreamConfiguration &outCfg) = 0;\n> >  \tvirtual int process(const libcamera::FrameBuffer &source,\n> > -\t\t\t    libcamera::Span<uint8_t> destination,\n> > +\t\t\t    libcamera::MappedBuffer *destination,\n> >  \t\t\t    CameraMetadata *metadata) = 0;\n> >  };\n> >","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 60E84BD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 26 Jan 2021 08:56:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C861C682F1;\n\tTue, 26 Jan 2021 09:56:24 +0100 (CET)","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 A1D73682E6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 26 Jan 2021 09:56:23 +0100 (CET)","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 00BB72C1;\n\tTue, 26 Jan 2021 09:56:22 +0100 (CET)"],"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=\"aYMiJe4X\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1611651383;\n\tbh=gn89wDXmZKBFHAZ7q+zf+OhNo5ZHlGgWX2kGlD2i8YQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=aYMiJe4Xyrmqy4P6ThBloR1C/wZ3OqwqDNDH4fo42Uou0lIJVPHijsvP3aOIM8Kun\n\tDiJQxJ1gDfhpsC5rX5h85OuDbN5TpSYuvycUsL2I3IP6UOlyQTbQEBSEwJ6FvYw+Yi\n\tvgNjgnJAmpDT7kz6ssLMqiZqgSTw5DqjYPLZuV0Y=","Date":"Tue, 26 Jan 2021 10:56:03 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Hirokazu Honda <hiroh@chromium.org>","Message-ID":"<YA/ZI1P8Syq0ZflK@pendragon.ideasonboard.com>","References":"<20210122092335.254626-1-hiroh@chromium.org>\n\t<fb7ec7e9-f76a-0391-5e06-45574dc810fd@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<fb7ec7e9-f76a-0391-5e06-45574dc810fd@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 1/2] android: post_processor: Change\n\tthe type destination in process()","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@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>"}}]