[{"id":16134,"web_url":"https://patchwork.libcamera.org/comment/16134/","msgid":"<20210407080657.il6ich2uzor6jfgm@uno.localdomain>","date":"2021-04-07T08:06:57","subject":"Re: [libcamera-devel] [PATCH v2 1/2] android: CameraBuffer: Add a\n\tstatic function to check a buffer validness","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hiro,\n\nOn Wed, Apr 07, 2021 at 01:36:19PM +0900, Hirokazu Honda wrote:\n> This adds a static function to CameraBuffer class that checks if\n> a buffer_handle is valid with a platform dependent framework.\n> For example, the function validates a buffer using\n> cros::CameraBufferManager on ChromeOS.\n>\n> Signed-off-by: Hirokazu Honda \n> ---\n> src/android/camera_buffer.h | 6 ++++++\n> src/android/mm/cros_camera_buffer.cpp | 19 +++++++++++++++++++\n> src/android/mm/generic_camera_buffer.cpp | 9 +++++++++\n> 3 files changed, 34 insertions(+)\n>\n> diff --git a/src/android/camera_buffer.h b/src/android/camera_buffer.h\n> index 7e8970b4..3bb9a435 100644\n> --- a/src/android/camera_buffer.h\n> +++ b/src/android/camera_buffer.h\n> @@ -20,6 +20,8 @@ public:\n> \tCameraBuffer(buffer_handle_t camera3Buffer, int flags);\n> \t~CameraBuffer();\n>\n> +\tstatic bool isValidBuffer(buffer_handle_t camera3Buffer);\n> +\n> \tbool isValid() const;\n>\n> \tunsigned int numPlanes() const;\n> @@ -38,6 +40,10 @@ CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer, int flags)\t\\\n> CameraBuffer::~CameraBuffer()\t\t\t\t\t\t\\\n> {\t\t\t\t\t\t\t\t\t\\\n> }\t\t\t\t\t\t\t\t\t\\\n> +bool CameraBuffer::isValidBuffer(buffer_handle_t camera3Buffer)\t\t\\\n\n\nThe closing '\\' renders un-aligned in my mail client\n\n> +{\t\t\t\t\t\t\t\t\t\\\n> +\treturn Private::isValidBuffer(camera3Buffer);\t\t\t\\\n> +}\t\t\t\t\t\t\t\t\t\\\n> bool CameraBuffer::isValid() const\t\t\t\t\t\\\n> {\t\t\t\t\t\t\t\t\t\\\n> \tconst Private *const d = LIBCAMERA_D_PTR();\t\t\t\\\n> diff --git a/src/android/mm/cros_camera_buffer.cpp b/src/android/mm/cros_camera_buffer.cpp\n> index 1a4fd5d1..371a2511 100644\n> --- a/src/android/mm/cros_camera_buffer.cpp\n> +++ b/src/android/mm/cros_camera_buffer.cpp\n> @@ -24,6 +24,8 @@ public:\n> \t\tbuffer_handle_t camera3Buffer, int flags);\n> \t~Private();\n>\n> +\tstatic bool isValidBuffer(buffer_handle_t camera3Buffer);\n> +\n> \tbool isValid() const { return valid_; }\n>\n> \tunsigned int numPlanes() const;\n> @@ -133,4 +135,21 @@ size_t CameraBuffer::Private::jpegBufferSize(size_t maxJpegBufferSize) const\n> \treturn bufferManager_->GetPlaneSize(handle_, 0);\n> }\n>\n> +// static\n\nNo C++ comments please\n\n> +bool CameraBuffer::Private::isValidBuffer(buffer_handle_t camera3Buffer)\n> +{\n> +\tcros::CameraBufferManager * bufferManager =\n\n*bufferManager\n\n> +\t\tcros::CameraBufferManager::GetInstance();\n> +\n> +\tint ret = bufferManager->Register(camera3Buffer);\n> +\tif (ret) {\n> +\t\treturn false;\n> +\t}\n\nNo enclosing braces for 1-liners\n\n> +\n> +\tbufferManager->Deregister(camera3Buffer);\n> +\n> +\treturn true;\n> +}\n> +\n\nDouble empty line\n\n> +\n> PUBLIC_CAMERA_BUFFER_IMPLEMENTATION\n> diff --git a/src/android/mm/generic_camera_buffer.cpp b/src/android/mm/generic_camera_buffer.cpp\n> index 929e078a..0c52e196 100644\n> --- a/src/android/mm/generic_camera_buffer.cpp\n> +++ b/src/android/mm/generic_camera_buffer.cpp\n> @@ -24,6 +24,8 @@ public:\n> \t\tbuffer_handle_t camera3Buffer, int flags);\n> \t~Private();\n>\n> +\tstatic bool isValidBuffer(buffer_handle_t camera3Buffer);\n> +\n> \tunsigned int numPlanes() const;\n>\n> \tSpan plane(unsigned int plane);\n> @@ -85,4 +87,11 @@ size_t CameraBuffer::Private::jpegBufferSize(size_t maxJpegBufferSize) const\n> \t\t\t\t maxJpegBufferSize);\n> }\n>\n> +// static\n\nNo C++ comments please\n\n> +bool CameraBuffer::Private::isValidBuffer(\n> + [[maybe_unused]] buffer_handle_t camera3Buffer)\n> +{\n> +\treturn true;\n\nDouble empty line\n> +\n> +}\n\nPlease make sure to run checkstyle.py before submitting. You can\n$ cp utils/hooks/post-commit .git/hooks\nto automatize that.\n\nThe patch itself looks good\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> PUBLIC_CAMERA_BUFFER_IMPLEMENTATION\n> --\n> 2.31.0.208.g409f899ff0-goog\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","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 217EFBD224\n\tfor ;\n\tWed, 7 Apr 2021 08:06:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8C31F687A2;\n\tWed, 7 Apr 2021 10:06:22 +0200 (CEST)","from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D719968795\n\tfor ;\n\tWed, 7 Apr 2021 10:06:20 +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 relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 5A2A6C0008;\n\tWed, 7 Apr 2021 08:06:20 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Wed, 7 Apr 2021 10:06:57 +0200","From":"Jacopo Mondi ","To":"Hirokazu Honda ","Message-ID":"<20210407080657.il6ich2uzor6jfgm@uno.localdomain>","References":"<20210407043620.1756347-1-hiroh@chromium.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210407043620.1756347-1-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [PATCH v2 1/2] android: CameraBuffer: Add a\n\tstatic function to check a buffer validness","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","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\" "}},{"id":16150,"web_url":"https://patchwork.libcamera.org/comment/16150/","msgid":"","date":"2021-04-08T03:12:28","subject":"Re: [libcamera-devel] [PATCH v2 1/2] android: CameraBuffer: Add a\n\tstatic function to check a buffer validness","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Jacopo, thanks for reviewing.\n\nOn Wed, Apr 7, 2021 at 5:06 PM Jacopo Mondi wrote:\n>\n> Hi Hiro,\n>\n> On Wed, Apr 07, 2021 at 01:36:19PM +0900, Hirokazu Honda wrote:\n> > This adds a static function to CameraBuffer class that checks if\n> > a buffer_handle is valid with a platform dependent framework.\n> > For example, the function validates a buffer using\n> > cros::CameraBufferManager on ChromeOS.\n> >\n> > Signed-off-by: Hirokazu Honda \n> > ---\n> > src/android/camera_buffer.h | 6 ++++++\n> > src/android/mm/cros_camera_buffer.cpp | 19 +++++++++++++++++++\n> > src/android/mm/generic_camera_buffer.cpp | 9 +++++++++\n> > 3 files changed, 34 insertions(+)\n> >\n> > diff --git a/src/android/camera_buffer.h b/src/android/camera_buffer.h\n> > index 7e8970b4..3bb9a435 100644\n> > --- a/src/android/camera_buffer.h\n> > +++ b/src/android/camera_buffer.h\n> > @@ -20,6 +20,8 @@ public:\n> > CameraBuffer(buffer_handle_t camera3Buffer, int flags);\n> > ~CameraBuffer();\n> >\n> > + static bool isValidBuffer(buffer_handle_t camera3Buffer);\n> > +\n> > bool isValid() const;\n> >\n> > unsigned int numPlanes() const;\n> > @@ -38,6 +40,10 @@ CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer, int flags) \\\n> > CameraBuffer::~CameraBuffer() \\\n> > { \\\n> > } \\\n> > +bool CameraBuffer::isValidBuffer(buffer_handle_t camera3Buffer) \\\n>\n>\n> The closing '\\' renders un-aligned in my mail client\n>\n> > +{ \\\n> > + return Private::isValidBuffer(camera3Buffer); \\\n> > +} \\\n> > bool CameraBuffer::isValid() const \\\n> > { \\\n> > const Private *const d = LIBCAMERA_D_PTR(); \\\n> > diff --git a/src/android/mm/cros_camera_buffer.cpp b/src/android/mm/cros_camera_buffer.cpp\n> > index 1a4fd5d1..371a2511 100644\n> > --- a/src/android/mm/cros_camera_buffer.cpp\n> > +++ b/src/android/mm/cros_camera_buffer.cpp\n> > @@ -24,6 +24,8 @@ public:\n> > buffer_handle_t camera3Buffer, int flags);\n> > ~Private();\n> >\n> > + static bool isValidBuffer(buffer_handle_t camera3Buffer);\n> > +\n> > bool isValid() const { return valid_; }\n> >\n> > unsigned int numPlanes() const;\n> > @@ -133,4 +135,21 @@ size_t CameraBuffer::Private::jpegBufferSize(size_t maxJpegBufferSize) const\n> > return bufferManager_->GetPlaneSize(handle_, 0);\n> > }\n> >\n> > +// static\n>\n> No C++ comments please\n>\n> > +bool CameraBuffer::Private::isValidBuffer(buffer_handle_t camera3Buffer)\n> > +{\n> > + cros::CameraBufferManager * bufferManager =\n>\n> *bufferManager\n>\n> > + cros::CameraBufferManager::GetInstance();\n> > +\n> > + int ret = bufferManager->Register(camera3Buffer);\n> > + if (ret) {\n> > + return false;\n> > + }\n>\n> No enclosing braces for 1-liners\n>\n> > +\n> > + bufferManager->Deregister(camera3Buffer);\n> > +\n> > + return true;\n> > +}\n> > +\n>\n> Double empty line\n>\n> > +\n> > PUBLIC_CAMERA_BUFFER_IMPLEMENTATION\n> > diff --git a/src/android/mm/generic_camera_buffer.cpp b/src/android/mm/generic_camera_buffer.cpp\n> > index 929e078a..0c52e196 100644\n> > --- a/src/android/mm/generic_camera_buffer.cpp\n> > +++ b/src/android/mm/generic_camera_buffer.cpp\n> > @@ -24,6 +24,8 @@ public:\n> > buffer_handle_t camera3Buffer, int flags);\n> > ~Private();\n> >\n> > + static bool isValidBuffer(buffer_handle_t camera3Buffer);\n> > +\n> > unsigned int numPlanes() const;\n> >\n> > Span plane(unsigned int plane);\n> > @@ -85,4 +87,11 @@ size_t CameraBuffer::Private::jpegBufferSize(size_t maxJpegBufferSize) const\n> > maxJpegBufferSize);\n> > }\n> >\n> > +// static\n>\n> No C++ comments please\n>\n> > +bool CameraBuffer::Private::isValidBuffer(\n> > + [[maybe_unused]] buffer_handle_t camera3Buffer)\n> > +{\n> > + return true;\n>\n> Double empty line\n> > +\n> > +}\n>\n> Please make sure to run checkstyle.py before submitting. You can\n> $ cp utils/hooks/post-commit .git/hooks\n> to automatize that.\n>\n\nThanks for telling me the track. I manually run checkstyle.py and\nforgot sometimes.\nIt is definitely helpful for me.\n\n> The patch itself looks good\n> Reviewed-by: Jacopo Mondi \n>\n> Thanks\n> j\n>\n> > PUBLIC_CAMERA_BUFFER_IMPLEMENTATION\n> > --\n> > 2.31.0.208.g409f899ff0-goog\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","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 0004DBD1F6\n\tfor ;\n\tThu, 8 Apr 2021 03:12:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B1B5B687F3;\n\tThu, 8 Apr 2021 05:12:40 +0200 (CEST)","from mail-ed1-x534.google.com (mail-ed1-x534.google.com\n\t[IPv6:2a00:1450:4864:20::534])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 71F8368414\n\tfor ;\n\tThu, 8 Apr 2021 05:12:39 +0200 (CEST)","by mail-ed1-x534.google.com with SMTP id z1so567760edb.8\n\tfor ;\n\tWed, 07 Apr 2021 20:12:39 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"dWwcx91J\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=X3hN1K0CDeyPJooc3HgmMi8cBNsshVPJ5AZt24qEwMY=;\n\tb=dWwcx91J0e8OVrDgMm9hOiUZ+BUYrrSW2UXJKV2Af2HzCvO+J85SL1EiKJAqPH2LEV\n\treCjdU2+L8X3lSlMJjILaE9ckuWOwb04Zn6H2JJAqA1t1tgM85b/q3J4ZaSGX3kVGNlJ\n\tXdbz5jrbGFIZXrcZJeyaHRbJ6zXstvTEIUurk=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=X3hN1K0CDeyPJooc3HgmMi8cBNsshVPJ5AZt24qEwMY=;\n\tb=fqXxWrfuPMScQWOQzCd0ma32J4q0ntL+H9nTQYzHfUx4uDvw/ApWVdHZeLUhsgsBcw\n\twa1AzEzuMWYUVNWtASrGcMoqtPuOeYkur14GcPsI4CNK29dQLHAT92a/4EzI3QGWhLtt\n\tx3ovMybd/kLNJz7yr8uW458X+qk6qR+LJFdcUgYV5vSIIR10o76rH4wnonJZt4RJHuE8\n\tZhVkdvL8bJWrinb6o11Solh57xrRM+EA2a+eJ/ZZDI9rcw3KRdu4gKev+rOBsemZxeCF\n\t01U/ta2n+A4/Aa/cJtsBvA1QIggf6XcyU2DU4DLxjsYA67prLHbukzmsassq/6chjr1u\n\tL4lA==","X-Gm-Message-State":"AOAM533jCC42Jp+WUn03HERymc8DWPqg1MEA+8cDfewMiftd5pshyCMZ\n\tOWSqih1unbXrgQn/Fo62QaFu25Xvtt4BdjxuaCoiPtYN5SU=","X-Google-Smtp-Source":"ABdhPJzl4O3Tyk/OyNV8Tm3Yc1zZhhGjT6bxBJPaJckyGpv/k0AOvRP7DIDf5vW2XlmH9lAFeIP4NoxOCNVFYrvi2FM=","X-Received":"by 2002:a05:6402:51cd:: with SMTP id\n\tr13mr8466603edd.116.1617851559079; \n\tWed, 07 Apr 2021 20:12:39 -0700 (PDT)","MIME-Version":"1.0","References":"<20210407043620.1756347-1-hiroh@chromium.org>\n\t<20210407080657.il6ich2uzor6jfgm@uno.localdomain>","In-Reply-To":"<20210407080657.il6ich2uzor6jfgm@uno.localdomain>","From":"Hirokazu Honda ","Date":"Thu, 8 Apr 2021 12:12:28 +0900","Message-ID":"","To":"Jacopo Mondi ","Subject":"Re: [libcamera-devel] [PATCH v2 1/2] android: CameraBuffer: Add a\n\tstatic function to check a buffer validness","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]