[{"id":3164,"web_url":"https://patchwork.libcamera.org/comment/3164/","msgid":"<20191202100736.md3dyywngv2j7gpm@uno.localdomain>","date":"2019-12-02T10:07:36","subject":"Re: [libcamera-devel] [PATCH 16/30] libcamera: buffer: Buffer\n\tremove metadata information","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Wed, Nov 27, 2019 at 12:36:06AM +0100, Niklas Söderlund wrote:\n> All metadata information is now stored in the associated BufferInfo\n> object, remove all metadata information from the Buffer object.\n\ns/object/class\n\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n   j\n\n>\n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  include/libcamera/buffer.h | 14 ----------\n>  src/libcamera/buffer.cpp   | 53 +-------------------------------------\n>  2 files changed, 1 insertion(+), 66 deletions(-)\n>\n> diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h\n> index 2e5376fb8b53a4c5..acc876eec7d93873 100644\n> --- a/include/libcamera/buffer.h\n> +++ b/include/libcamera/buffer.h\n> @@ -105,12 +105,6 @@ private:\n>  class Buffer final\n>  {\n>  public:\n> -\tenum Status {\n> -\t\tBufferSuccess,\n> -\t\tBufferError,\n> -\t\tBufferCancelled,\n> -\t};\n> -\n>  \tBuffer(unsigned int index = -1, const Buffer *metadata = nullptr);\n>  \tBuffer(const Buffer &) = delete;\n>  \tBuffer &operator=(const Buffer &) = delete;\n> @@ -119,12 +113,8 @@ public:\n>  \tconst std::array<int, 3> &dmabufs() const { return dmabuf_; }\n>  \tBufferMemory *mem() { return mem_; }\n>\n> -\tunsigned int bytesused() const { return bytesused_; }\n> -\tuint64_t timestamp() const { return timestamp_; }\n> -\tunsigned int sequence() const { return sequence_; }\n>  \tconst BufferInfo &info() const { return info_; };\n>\n> -\tStatus status() const { return status_; }\n>  \tRequest *request() const { return request_; }\n>  \tStream *stream() const { return stream_; }\n>\n> @@ -140,12 +130,8 @@ private:\n>  \tstd::array<int, 3> dmabuf_;\n>  \tBufferMemory *mem_;\n>\n> -\tunsigned int bytesused_;\n> -\tuint64_t timestamp_;\n> -\tunsigned int sequence_;\n>  \tBufferInfo info_;\n>\n> -\tStatus status_;\n>  \tRequest *request_;\n>  \tStream *stream_;\n>  };\n> diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp\n> index d5a4815a0bb8c528..ab28e0d76b8c40f7 100644\n> --- a/src/libcamera/buffer.cpp\n> +++ b/src/libcamera/buffer.cpp\n> @@ -349,20 +349,6 @@ void BufferPool::destroyBuffers()\n>   * deleted automatically after the request complete handler returns.\n>   */\n>\n> -/**\n> - * \\enum Buffer::Status\n> - * Buffer completion status\n> - * \\var Buffer::BufferSuccess\n> - * The buffer has completed with success and contains valid data. All its other\n> - * metadata (such as bytesused(), timestamp() or sequence() number) are valid.\n> - * \\var Buffer::BufferError\n> - * The buffer has completed with an error and doesn't contain valid data. Its\n> - * other metadata are valid.\n> - * \\var Buffer::BufferCancelled\n> - * The buffer has been cancelled due to capture stop. Its other metadata are\n> - * invalid and shall not be used.\n> - */\n> -\n>  /**\n>   * \\brief Construct a buffer not associated with any stream\n>   *\n> @@ -371,8 +357,7 @@ void BufferPool::destroyBuffers()\n>   * for a stream with Stream::createBuffer().\n>   */\n>  Buffer::Buffer(unsigned int index, const Buffer *metadata)\n> -\t: index_(index), dmabuf_({ -1, -1, -1 }),\n> -\t  status_(Buffer::BufferSuccess), request_(nullptr),\n> +\t: index_(index), dmabuf_({ -1, -1, -1 }), request_(nullptr),\n>  \t  stream_(nullptr)\n>  {\n>  \tunsigned int sequence;\n> @@ -420,32 +405,6 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata)\n>   * \\return The BufferMemory this buffer is associated with\n>   */\n>\n> -/**\n> - * \\fn Buffer::bytesused()\n> - * \\brief Retrieve the number of bytes occupied by the data in the buffer\n> - * \\return Number of bytes occupied in the buffer\n> - */\n> -\n> -/**\n> - * \\fn Buffer::timestamp()\n> - * \\brief Retrieve the time when the buffer was processed\n> - *\n> - * The timestamp is expressed as a number of nanoseconds since the epoch.\n> - *\n> - * \\return Timestamp when the buffer was processed\n> - */\n> -\n> -/**\n> - * \\fn Buffer::sequence()\n> - * \\brief Retrieve the buffer sequence number\n> - *\n> - * The sequence number is a monotonically increasing number assigned to the\n> - * buffer processed by the stream. Gaps in the sequence numbers indicate\n> - * dropped frames.\n> - *\n> - * \\return Sequence number of the buffer\n> - */\n> -\n>  /**\n>   * \\fn Buffer::info()\n>   * \\brief Retrieve the buffer metadata information\n> @@ -456,16 +415,6 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata)\n>   * \\return Metadata of the buffer\n>   */\n>\n> -/**\n> - * \\fn Buffer::status()\n> - * \\brief Retrieve the buffer status\n> - *\n> - * The buffer status reports whether the buffer has completed successfully\n> - * (BufferSuccess) or if an error occurred (BufferError).\n> - *\n> - * \\return The buffer status\n> - */\n> -\n>  /**\n>   * \\fn Buffer::request()\n>   * \\brief Retrieve the request this buffer belongs to\n> --\n> 2.24.0\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay12.mail.gandi.net (relay12.mail.gandi.net\n\t[217.70.178.232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E298960BFF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  2 Dec 2019 11:05:27 +0100 (CET)","from uno.localdomain (93-34-114-233.ip49.fastwebnet.it\n\t[93.34.114.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay12.mail.gandi.net (Postfix) with ESMTPSA id 4EF7620000A;\n\tMon,  2 Dec 2019 10:05:27 +0000 (UTC)"],"Date":"Mon, 2 Dec 2019 11:07:36 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191202100736.md3dyywngv2j7gpm@uno.localdomain>","References":"<20191126233620.1695316-1-niklas.soderlund@ragnatech.se>\n\t<20191126233620.1695316-17-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"kggjvy55qh2j266m\"","Content-Disposition":"inline","In-Reply-To":"<20191126233620.1695316-17-niklas.soderlund@ragnatech.se>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 16/30] libcamera: buffer: Buffer\n\tremove metadata information","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":"Mon, 02 Dec 2019 10:05:28 -0000"}},{"id":3237,"web_url":"https://patchwork.libcamera.org/comment/3237/","msgid":"<20191209212203.GG18060@pendragon.ideasonboard.com>","date":"2019-12-09T21:22:03","subject":"Re: [libcamera-devel] [PATCH 16/30] libcamera: buffer: Buffer\n\tremove metadata information","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Mon, Dec 02, 2019 at 11:07:36AM +0100, Jacopo Mondi wrote:\n> On Wed, Nov 27, 2019 at 12:36:06AM +0100, Niklas Söderlund wrote:\n> > All metadata information is now stored in the associated BufferInfo\n> > object, remove all metadata information from the Buffer object.\n> \n> s/object/class\n> \n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nAnd I would squash this with 15/30. Reviewing 15/30 left me wondering if\nthere could be usages of the old fields that were missed, if you remove\nthem in the same patch review will be easier.\n\n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > ---\n> >  include/libcamera/buffer.h | 14 ----------\n> >  src/libcamera/buffer.cpp   | 53 +-------------------------------------\n> >  2 files changed, 1 insertion(+), 66 deletions(-)\n> >\n> > diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h\n> > index 2e5376fb8b53a4c5..acc876eec7d93873 100644\n> > --- a/include/libcamera/buffer.h\n> > +++ b/include/libcamera/buffer.h\n> > @@ -105,12 +105,6 @@ private:\n> >  class Buffer final\n> >  {\n> >  public:\n> > -\tenum Status {\n> > -\t\tBufferSuccess,\n> > -\t\tBufferError,\n> > -\t\tBufferCancelled,\n> > -\t};\n> > -\n> >  \tBuffer(unsigned int index = -1, const Buffer *metadata = nullptr);\n> >  \tBuffer(const Buffer &) = delete;\n> >  \tBuffer &operator=(const Buffer &) = delete;\n> > @@ -119,12 +113,8 @@ public:\n> >  \tconst std::array<int, 3> &dmabufs() const { return dmabuf_; }\n> >  \tBufferMemory *mem() { return mem_; }\n> >\n> > -\tunsigned int bytesused() const { return bytesused_; }\n> > -\tuint64_t timestamp() const { return timestamp_; }\n> > -\tunsigned int sequence() const { return sequence_; }\n> >  \tconst BufferInfo &info() const { return info_; };\n> >\n> > -\tStatus status() const { return status_; }\n> >  \tRequest *request() const { return request_; }\n> >  \tStream *stream() const { return stream_; }\n> >\n> > @@ -140,12 +130,8 @@ private:\n> >  \tstd::array<int, 3> dmabuf_;\n> >  \tBufferMemory *mem_;\n> >\n> > -\tunsigned int bytesused_;\n> > -\tuint64_t timestamp_;\n> > -\tunsigned int sequence_;\n> >  \tBufferInfo info_;\n> >\n> > -\tStatus status_;\n> >  \tRequest *request_;\n> >  \tStream *stream_;\n> >  };\n> > diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp\n> > index d5a4815a0bb8c528..ab28e0d76b8c40f7 100644\n> > --- a/src/libcamera/buffer.cpp\n> > +++ b/src/libcamera/buffer.cpp\n> > @@ -349,20 +349,6 @@ void BufferPool::destroyBuffers()\n> >   * deleted automatically after the request complete handler returns.\n> >   */\n> >\n> > -/**\n> > - * \\enum Buffer::Status\n> > - * Buffer completion status\n> > - * \\var Buffer::BufferSuccess\n> > - * The buffer has completed with success and contains valid data. All its other\n> > - * metadata (such as bytesused(), timestamp() or sequence() number) are valid.\n> > - * \\var Buffer::BufferError\n> > - * The buffer has completed with an error and doesn't contain valid data. Its\n> > - * other metadata are valid.\n> > - * \\var Buffer::BufferCancelled\n> > - * The buffer has been cancelled due to capture stop. Its other metadata are\n> > - * invalid and shall not be used.\n> > - */\n> > -\n> >  /**\n> >   * \\brief Construct a buffer not associated with any stream\n> >   *\n> > @@ -371,8 +357,7 @@ void BufferPool::destroyBuffers()\n> >   * for a stream with Stream::createBuffer().\n> >   */\n> >  Buffer::Buffer(unsigned int index, const Buffer *metadata)\n> > -\t: index_(index), dmabuf_({ -1, -1, -1 }),\n> > -\t  status_(Buffer::BufferSuccess), request_(nullptr),\n> > +\t: index_(index), dmabuf_({ -1, -1, -1 }), request_(nullptr),\n> >  \t  stream_(nullptr)\n> >  {\n> >  \tunsigned int sequence;\n> > @@ -420,32 +405,6 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata)\n> >   * \\return The BufferMemory this buffer is associated with\n> >   */\n> >\n> > -/**\n> > - * \\fn Buffer::bytesused()\n> > - * \\brief Retrieve the number of bytes occupied by the data in the buffer\n> > - * \\return Number of bytes occupied in the buffer\n> > - */\n> > -\n> > -/**\n> > - * \\fn Buffer::timestamp()\n> > - * \\brief Retrieve the time when the buffer was processed\n> > - *\n> > - * The timestamp is expressed as a number of nanoseconds since the epoch.\n> > - *\n> > - * \\return Timestamp when the buffer was processed\n> > - */\n> > -\n> > -/**\n> > - * \\fn Buffer::sequence()\n> > - * \\brief Retrieve the buffer sequence number\n> > - *\n> > - * The sequence number is a monotonically increasing number assigned to the\n> > - * buffer processed by the stream. Gaps in the sequence numbers indicate\n> > - * dropped frames.\n> > - *\n> > - * \\return Sequence number of the buffer\n> > - */\n> > -\n> >  /**\n> >   * \\fn Buffer::info()\n> >   * \\brief Retrieve the buffer metadata information\n> > @@ -456,16 +415,6 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata)\n> >   * \\return Metadata of the buffer\n> >   */\n> >\n> > -/**\n> > - * \\fn Buffer::status()\n> > - * \\brief Retrieve the buffer status\n> > - *\n> > - * The buffer status reports whether the buffer has completed successfully\n> > - * (BufferSuccess) or if an error occurred (BufferError).\n> > - *\n> > - * \\return The buffer status\n> > - */\n> > -\n> >  /**\n> >   * \\fn Buffer::request()\n> >   * \\brief Retrieve the request this buffer belongs to","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 98DB160BDB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  9 Dec 2019 22:22:10 +0100 (CET)","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 01E5911B7;\n\tMon,  9 Dec 2019 22:22:09 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1575926530;\n\tbh=DJEWhh9FfnqMkDnKc/NfwoWFoAx824309cfmVxwIMDc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FEYvWFiExqNaGovJ5ZcCynPkpQy+M5kcDxwnGxvQ7TaTPdbLrohTyH01cNj5N8xCb\n\tXaDckinPyTjEppIAWiN2rweLTJ3MY7d6GVuwTO8ehDTtwzQ/XwhGMFSD7Y+pIObFaf\n\t2KKibiKVqRXKLNrOnJVH8UjFovrJKRqvSnKI9msQ=","Date":"Mon, 9 Dec 2019 23:22:03 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"Jacopo Mondi <jacopo@jmondi.org>, libcamera-devel@lists.libcamera.org","Message-ID":"<20191209212203.GG18060@pendragon.ideasonboard.com>","References":"<20191126233620.1695316-1-niklas.soderlund@ragnatech.se>\n\t<20191126233620.1695316-17-niklas.soderlund@ragnatech.se>\n\t<20191202100736.md3dyywngv2j7gpm@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20191202100736.md3dyywngv2j7gpm@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 16/30] libcamera: buffer: Buffer\n\tremove metadata information","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":"Mon, 09 Dec 2019 21:22:10 -0000"}}]