[{"id":5000,"web_url":"https://patchwork.libcamera.org/comment/5000/","msgid":"<20200604023942.GI27695@pendragon.ideasonboard.com>","date":"2020-06-04T02:39:42","subject":"Re: [libcamera-devel] [PATCH 5/8] android: camera_device: Calculate\n\tmetadata size","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Tue, May 26, 2020 at 04:22:34PM +0200, Jacopo Mondi wrote:\n> As we move to have more and more dynamically generated static metadata\n> entries, the size of the metadata buffer has to be calculated\n> dynamically inspecting the information collected from the camera.\n> \n> Provide a method to perform metadata buffers size calculation and\n> use it when generating camera static metadata.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  src/android/camera_device.cpp | 42 ++++++++++++++++++++++++++++++-----\n>  src/android/camera_device.h   |  2 ++\n>  2 files changed, 38 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 534bfb1df1ef..6cc377820210 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -331,6 +331,40 @@ void CameraDevice::setCallbacks(const camera3_callback_ops_t *callbacks)\n>  /*\n>   * Return static information for the camera.\n>   */\n> +std::pair<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize()\n> +{\n> +\t/*\n> +\t * \\todo Keep this in sync with the actual number of entries.\n> +\t * Currently: 50 entries, 647 bytes of static metadata\n> +\t */\n> +\tstd::pair<uint32_t, uint32_t> metadataSize;\n> +\tmetadataSize.first = 50;\n> +\tmetadataSize.second = 647;\n> +\n\nDo you think we will end up reworking the CameraMetadata class to avoid\npre-allocation ?\n\n> +\t/*\n> +\t * Calculate space occupation in bytes for dynamically built metadata\n> +\t * entries.\n> +\t */\n> +\n> +\t/* std::forward_list does not provide a size() method :( */\n\nLet's make it a vector :-)\n\n> +\tfor (const auto &entry : streamConfigurations_) {\n> +\t\t/* Just please the compiler, otherwise entry is not used. */\n> +\t\tswitch (entry.androidScalerCode) {\n> +\t\tdefault:\n> +\t\t\tbreak;\n> +\t\t}\n\nThis hack can be dropped if you use a vector.\n\n> +\n> +\t\t/*\n> +\t\t * 4 32bits integers for ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS\n> +\t\t * 1 32bits integer for ANDROID_SCALER_AVAILABLE_FORMATS\n> +\t\t * 4 64bits integers for ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS\n> +\t\t */\n> +\t\tmetadataSize.second += 52;\n> +\t}\n\nWe currently have 700 bytes, with a comment stating 666 bytes, and the\ncode here will return 699 bytes when there's a single stream\nconfiguration. Which of those, if any, is correct ? :-) Could you add a\nnote about this to the commit message ?\n\n> +\n> +\treturn metadataSize;\n> +}\n> +\n>  const camera_metadata_t *CameraDevice::getStaticMetadata()\n>  {\n>  \tif (staticMetadata_)\n> @@ -341,12 +375,8 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n>  \t * example application, but a real camera implementation will require\n>  \t * more.\n>  \t */\n> -\n> -\t/*\n> -\t * \\todo Keep this in sync with the actual number of entries.\n> -\t * Currently: 50 entries, 666 bytes\n> -\t */\n> -\tstaticMetadata_ = new CameraMetadata(50, 700);\n> +\tconst std::pair<uint32_t, uint32_t> sizes = calculateStaticMetadataSize();\n> +\tstaticMetadata_ = new CameraMetadata(sizes.first, sizes.second);\n\nReturning a pair is confusing, we could easily swap the first and second\narguments. Creating a struct would be best, but maybe using a std::tuple\ncould be an acceptable solution as you could write\n\n\tuint32_t numEntries;\n\tuint32_t numBytes;\n\tstd::tie(numEntries, numBytes) = calculateStaticMetadataSize();\n\nThere's still a possibility of swapping the arguments, but at least\nthey're not called first and second.\n\n>  \tif (!staticMetadata_->isValid()) {\n>  \t\tLOG(HAL, Error) << \"Failed to allocate static metadata\";\n>  \t\tdelete staticMetadata_;\n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index 95bd39f590ab..f22a6e3e6c28 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -10,6 +10,7 @@\n>  #include <forward_list>\n>  #include <map>\n>  #include <memory>\n> +#include <utility>\n>  \n>  #include <hardware/camera3.h>\n>  \n> @@ -69,6 +70,7 @@ private:\n>  \t};\n>  \n>  \tint initializeFormats();\n> +\tstd::pair<uint32_t, uint32_t> calculateStaticMetadataSize();\n>  \tvoid notifyShutter(uint32_t frameNumber, uint64_t timestamp);\n>  \tvoid notifyError(uint32_t frameNumber, camera3_stream_t *stream);\n>  \tstd::unique_ptr<CameraMetadata> getResultMetadata(int frame_number,","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["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 633F261012\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  4 Jun 2020 04:39:59 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D277029B;\n\tThu,  4 Jun 2020 04:39:58 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Up2UUpjM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1591238399;\n\tbh=NjZ67/9B7OV695sDi1kywv2lYiJWCAiXhgOb+f+4EoA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Up2UUpjMrjsV4dejoWb9o6Yc2Xbg2nz1VvnJ4bs2qItx9z6fzUiMTxXq3vd5qzzuO\n\t4DdG2+gcj43he/8Bs4Fig7bk74CNTipX9dwPCSHqFVV8ygiQ5+DXL5+ihJMb4vivfj\n\t3pDhzMqR++LQ6CRSejFfnpgv1UnG4mWDt/LJWejc=","Date":"Thu, 4 Jun 2020 05:39:42 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200604023942.GI27695@pendragon.ideasonboard.com>","References":"<20200526142237.407557-1-jacopo@jmondi.org>\n\t<20200526142237.407557-6-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200526142237.407557-6-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH 5/8] android: camera_device: Calculate\n\tmetadata size","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>","X-List-Received-Date":"Thu, 04 Jun 2020 02:39:59 -0000"}},{"id":5021,"web_url":"https://patchwork.libcamera.org/comment/5021/","msgid":"<20200604123856.4mdfejdlfr63vslq@uno.localdomain>","date":"2020-06-04T12:38:56","subject":"Re: [libcamera-devel] [PATCH 5/8] android: camera_device: Calculate\n\tmetadata size","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Thu, Jun 04, 2020 at 05:39:42AM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Tue, May 26, 2020 at 04:22:34PM +0200, Jacopo Mondi wrote:\n> > As we move to have more and more dynamically generated static metadata\n> > entries, the size of the metadata buffer has to be calculated\n> > dynamically inspecting the information collected from the camera.\n> >\n> > Provide a method to perform metadata buffers size calculation and\n> > use it when generating camera static metadata.\n> >\n> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > ---\n> >  src/android/camera_device.cpp | 42 ++++++++++++++++++++++++++++++-----\n> >  src/android/camera_device.h   |  2 ++\n> >  2 files changed, 38 insertions(+), 6 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 534bfb1df1ef..6cc377820210 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -331,6 +331,40 @@ void CameraDevice::setCallbacks(const camera3_callback_ops_t *callbacks)\n> >  /*\n> >   * Return static information for the camera.\n> >   */\n> > +std::pair<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize()\n> > +{\n> > +\t/*\n> > +\t * \\todo Keep this in sync with the actual number of entries.\n> > +\t * Currently: 50 entries, 647 bytes of static metadata\n> > +\t */\n> > +\tstd::pair<uint32_t, uint32_t> metadataSize;\n> > +\tmetadataSize.first = 50;\n> > +\tmetadataSize.second = 647;\n> > +\n>\n> Do you think we will end up reworking the CameraMetadata class to avoid\n> pre-allocation ?\n>\n\nHow so ? If I'm not mistaken the Android camera_metadata library we\nwrap requires pre-allocation.\n\nWe could add all metadata one by one to a temporary storage in CameraMetadata,\nand calculate the size requirements as we add them. When done we could\nwrite them to camera_metadata... I'm not sure it's worth the effort\nthough..\n\n> > +\t/*\n> > +\t * Calculate space occupation in bytes for dynamically built metadata\n> > +\t * entries.\n> > +\t */\n> > +\n> > +\t/* std::forward_list does not provide a size() method :( */\n>\n> Let's make it a vector :-)\n>\n> > +\tfor (const auto &entry : streamConfigurations_) {\n> > +\t\t/* Just please the compiler, otherwise entry is not used. */\n> > +\t\tswitch (entry.androidScalerCode) {\n> > +\t\tdefault:\n> > +\t\t\tbreak;\n> > +\t\t}\n>\n> This hack can be dropped if you use a vector.\n>\n\nYes indeed, much better\n\n> > +\n> > +\t\t/*\n> > +\t\t * 4 32bits integers for ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS\n> > +\t\t * 1 32bits integer for ANDROID_SCALER_AVAILABLE_FORMATS\n> > +\t\t * 4 64bits integers for ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS\n> > +\t\t */\n> > +\t\tmetadataSize.second += 52;\n> > +\t}\n>\n> We currently have 700 bytes, with a comment stating 666 bytes, and the\n\n666 ( \\m/ ) was mentioned in a removed comment as well as 700 bytes (I\nguess we were 'larger' on purpose).\n\n> code here will return 699 bytes when there's a single stream\n> configuration. Which of those, if any, is correct ? :-) Could you add a\n> note about this to the commit message ?\n\nWhat's now reported is that we currently have 50 entries and 647 bytes\nof static information, plus 52 bytes for each stream configuration\nentry. Not sure what's wrong there.\n\n>\n> > +\n> > +\treturn metadataSize;\n> > +}\n> > +\n> >  const camera_metadata_t *CameraDevice::getStaticMetadata()\n> >  {\n> >  \tif (staticMetadata_)\n> > @@ -341,12 +375,8 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> >  \t * example application, but a real camera implementation will require\n> >  \t * more.\n> >  \t */\n> > -\n> > -\t/*\n> > -\t * \\todo Keep this in sync with the actual number of entries.\n> > -\t * Currently: 50 entries, 666 bytes\n> > -\t */\n> > -\tstaticMetadata_ = new CameraMetadata(50, 700);\n> > +\tconst std::pair<uint32_t, uint32_t> sizes = calculateStaticMetadataSize();\n> > +\tstaticMetadata_ = new CameraMetadata(sizes.first, sizes.second);\n>\n> Returning a pair is confusing, we could easily swap the first and second\n> arguments. Creating a struct would be best, but maybe using a std::tuple\n> could be an acceptable solution as you could write\n>\n> \tuint32_t numEntries;\n> \tuint32_t numBytes;\n> \tstd::tie(numEntries, numBytes) = calculateStaticMetadataSize();\n>\n> There's still a possibility of swapping the arguments, but at least\n> they're not called first and second.\n\nWell, that's not API code, but if it makes you sleep more peacefully I\ncould try with a tuple.\n\n>\n> >  \tif (!staticMetadata_->isValid()) {\n> >  \t\tLOG(HAL, Error) << \"Failed to allocate static metadata\";\n> >  \t\tdelete staticMetadata_;\n> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > index 95bd39f590ab..f22a6e3e6c28 100644\n> > --- a/src/android/camera_device.h\n> > +++ b/src/android/camera_device.h\n> > @@ -10,6 +10,7 @@\n> >  #include <forward_list>\n> >  #include <map>\n> >  #include <memory>\n> > +#include <utility>\n> >\n> >  #include <hardware/camera3.h>\n> >\n> > @@ -69,6 +70,7 @@ private:\n> >  \t};\n> >\n> >  \tint initializeFormats();\n> > +\tstd::pair<uint32_t, uint32_t> calculateStaticMetadataSize();\n> >  \tvoid notifyShutter(uint32_t frameNumber, uint64_t timestamp);\n> >  \tvoid notifyError(uint32_t frameNumber, camera3_stream_t *stream);\n> >  \tstd::unique_ptr<CameraMetadata> getResultMetadata(int frame_number,\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[217.70.183.200])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 57B3461027\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  4 Jun 2020 14:35:34 +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 relay7-d.mail.gandi.net (Postfix) with ESMTPSA id B3D2920012;\n\tThu,  4 Jun 2020 12:35:33 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Thu, 4 Jun 2020 14:38:56 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200604123856.4mdfejdlfr63vslq@uno.localdomain>","References":"<20200526142237.407557-1-jacopo@jmondi.org>\n\t<20200526142237.407557-6-jacopo@jmondi.org>\n\t<20200604023942.GI27695@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200604023942.GI27695@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 5/8] android: camera_device: Calculate\n\tmetadata size","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>","X-List-Received-Date":"Thu, 04 Jun 2020 12:35:34 -0000"}},{"id":5083,"web_url":"https://patchwork.libcamera.org/comment/5083/","msgid":"<20200606001124.GW26752@pendragon.ideasonboard.com>","date":"2020-06-06T00:11:24","subject":"Re: [libcamera-devel] [PATCH 5/8] android: camera_device: Calculate\n\tmetadata size","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Thu, Jun 04, 2020 at 02:38:56PM +0200, Jacopo Mondi wrote:\n> On Thu, Jun 04, 2020 at 05:39:42AM +0300, Laurent Pinchart wrote:\n> > On Tue, May 26, 2020 at 04:22:34PM +0200, Jacopo Mondi wrote:\n> > > As we move to have more and more dynamically generated static metadata\n> > > entries, the size of the metadata buffer has to be calculated\n> > > dynamically inspecting the information collected from the camera.\n> > >\n> > > Provide a method to perform metadata buffers size calculation and\n> > > use it when generating camera static metadata.\n> > >\n> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > > ---\n> > >  src/android/camera_device.cpp | 42 ++++++++++++++++++++++++++++++-----\n> > >  src/android/camera_device.h   |  2 ++\n> > >  2 files changed, 38 insertions(+), 6 deletions(-)\n> > >\n> > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > index 534bfb1df1ef..6cc377820210 100644\n> > > --- a/src/android/camera_device.cpp\n> > > +++ b/src/android/camera_device.cpp\n> > > @@ -331,6 +331,40 @@ void CameraDevice::setCallbacks(const camera3_callback_ops_t *callbacks)\n> > >  /*\n> > >   * Return static information for the camera.\n> > >   */\n> > > +std::pair<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize()\n> > > +{\n> > > +\t/*\n> > > +\t * \\todo Keep this in sync with the actual number of entries.\n> > > +\t * Currently: 50 entries, 647 bytes of static metadata\n> > > +\t */\n> > > +\tstd::pair<uint32_t, uint32_t> metadataSize;\n> > > +\tmetadataSize.first = 50;\n> > > +\tmetadataSize.second = 647;\n> > > +\n> >\n> > Do you think we will end up reworking the CameraMetadata class to avoid\n> > pre-allocation ?\n> \n> How so ? If I'm not mistaken the Android camera_metadata library we\n> wrap requires pre-allocation.\n> \n> We could add all metadata one by one to a temporary storage in CameraMetadata,\n> and calculate the size requirements as we add them. When done we could\n> write them to camera_metadata... I'm not sure it's worth the effort\n> though..\n\nThat's exactly what I meant :-)\n\nI'm also not sure it's worth the effort, it was really an open question.\nI'm tempted to get rid of the Android metadata library copy we have in\nlibcamera, to avoid having to analyze the LGPL-2.1-or-later + Apache-2.0\ncompatibility. One easy option for that would be to link against the\nAndroid or Chrome OS versions of the code, but that would reduce our\nability to compile-test the Android HAL on a Linux distribution.\nReimplementing the parts we need with an API that would be nicer to use\ncould also be an option.\n\n> > > +\t/*\n> > > +\t * Calculate space occupation in bytes for dynamically built metadata\n> > > +\t * entries.\n> > > +\t */\n> > > +\n> > > +\t/* std::forward_list does not provide a size() method :( */\n> >\n> > Let's make it a vector :-)\n> >\n> > > +\tfor (const auto &entry : streamConfigurations_) {\n> > > +\t\t/* Just please the compiler, otherwise entry is not used. */\n> > > +\t\tswitch (entry.androidScalerCode) {\n> > > +\t\tdefault:\n> > > +\t\t\tbreak;\n> > > +\t\t}\n> >\n> > This hack can be dropped if you use a vector.\n> \n> Yes indeed, much better\n> \n> > > +\n> > > +\t\t/*\n> > > +\t\t * 4 32bits integers for ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS\n> > > +\t\t * 1 32bits integer for ANDROID_SCALER_AVAILABLE_FORMATS\n> > > +\t\t * 4 64bits integers for ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS\n> > > +\t\t */\n> > > +\t\tmetadataSize.second += 52;\n> > > +\t}\n> >\n> > We currently have 700 bytes, with a comment stating 666 bytes, and the\n> \n> 666 ( \\m/ ) was mentioned in a removed comment as well as 700 bytes (I\n> guess we were 'larger' on purpose).\n> \n> > code here will return 699 bytes when there's a single stream\n> > configuration. Which of those, if any, is correct ? :-) Could you add a\n> > note about this to the commit message ?\n> \n> What's now reported is that we currently have 50 entries and 647 bytes\n> of static information, plus 52 bytes for each stream configuration\n> entry. Not sure what's wrong there.\n\nI'm not saying the new code is wrong, but it matches neither the old\ncomment, nor the old code, so I was wondering what was going out.\n\nThis is annoying to maintain manually :-S\n\n> > > +\n> > > +\treturn metadataSize;\n> > > +}\n> > > +\n> > >  const camera_metadata_t *CameraDevice::getStaticMetadata()\n> > >  {\n> > >  \tif (staticMetadata_)\n> > > @@ -341,12 +375,8 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> > >  \t * example application, but a real camera implementation will require\n> > >  \t * more.\n> > >  \t */\n> > > -\n> > > -\t/*\n> > > -\t * \\todo Keep this in sync with the actual number of entries.\n> > > -\t * Currently: 50 entries, 666 bytes\n> > > -\t */\n> > > -\tstaticMetadata_ = new CameraMetadata(50, 700);\n> > > +\tconst std::pair<uint32_t, uint32_t> sizes = calculateStaticMetadataSize();\n> > > +\tstaticMetadata_ = new CameraMetadata(sizes.first, sizes.second);\n> >\n> > Returning a pair is confusing, we could easily swap the first and second\n> > arguments. Creating a struct would be best, but maybe using a std::tuple\n> > could be an acceptable solution as you could write\n> >\n> > \tuint32_t numEntries;\n> > \tuint32_t numBytes;\n> > \tstd::tie(numEntries, numBytes) = calculateStaticMetadataSize();\n> >\n> > There's still a possibility of swapping the arguments, but at least\n> > they're not called first and second.\n> \n> Well, that's not API code, but if it makes you sleep more peacefully I\n> could try with a tuple.\n\nI'll aim for just sleeping before even thinking about sleeping\npeacefully :-) I see you've tried the tuple, what do you think about it\n?\n\n> > >  \tif (!staticMetadata_->isValid()) {\n> > >  \t\tLOG(HAL, Error) << \"Failed to allocate static metadata\";\n> > >  \t\tdelete staticMetadata_;\n> > > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > > index 95bd39f590ab..f22a6e3e6c28 100644\n> > > --- a/src/android/camera_device.h\n> > > +++ b/src/android/camera_device.h\n> > > @@ -10,6 +10,7 @@\n> > >  #include <forward_list>\n> > >  #include <map>\n> > >  #include <memory>\n> > > +#include <utility>\n> > >\n> > >  #include <hardware/camera3.h>\n> > >\n> > > @@ -69,6 +70,7 @@ private:\n> > >  \t};\n> > >\n> > >  \tint initializeFormats();\n> > > +\tstd::pair<uint32_t, uint32_t> calculateStaticMetadataSize();\n> > >  \tvoid notifyShutter(uint32_t frameNumber, uint64_t timestamp);\n> > >  \tvoid notifyError(uint32_t frameNumber, camera3_stream_t *stream);\n> > >  \tstd::unique_ptr<CameraMetadata> getResultMetadata(int frame_number,","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3EA10600F7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat,  6 Jun 2020 02:11:45 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3B21727C;\n\tSat,  6 Jun 2020 02:11:44 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"e0ZER9AI\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1591402304;\n\tbh=X3P58OfGspf8GdEBa0pGFY5htBy/IJ9tOHdq7kalP2Y=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=e0ZER9AIxgycTd7eue8jCDgc8k8mJ62Mu3M8Uvhay5BKFSXsOyaDZ3jiU7Ettger4\n\t5rM+lLMlt77FkSsqBirnTlfH1yBCOfA5HRTgrn+MWJAma7XS4jVdUYmc5akYFpLOE3\n\tyuTza/wpEte7/rVRF4NLGAQxMqeAexWK353E80sk=","Date":"Sat, 6 Jun 2020 03:11:24 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200606001124.GW26752@pendragon.ideasonboard.com>","References":"<20200526142237.407557-1-jacopo@jmondi.org>\n\t<20200526142237.407557-6-jacopo@jmondi.org>\n\t<20200604023942.GI27695@pendragon.ideasonboard.com>\n\t<20200604123856.4mdfejdlfr63vslq@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200604123856.4mdfejdlfr63vslq@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 5/8] android: camera_device: Calculate\n\tmetadata size","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>","X-List-Received-Date":"Sat, 06 Jun 2020 00:11:45 -0000"}}]