[{"id":27723,"web_url":"https://patchwork.libcamera.org/comment/27723/","msgid":"<169381809181.277971.9881759822074537202@ping.linuxembedded.co.uk>","date":"2023-09-04T09:01:31","subject":"Re: [libcamera-devel] [PATCH v2 3/4] pipeline: rpi: Rename\n\tRPi::Stream::setExternalBuffer()","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Naushir Patuck via libcamera-devel (2023-07-25 09:55:39)\n> Since we don't distinguish between externally and internally allocated\n> dma bufs, rename this function to setExportedBuffer() to clearer on its\n> function.\n> \n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> ---\n>  src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 2 +-\n>  src/libcamera/pipeline/rpi/common/rpi_stream.cpp    | 2 +-\n>  src/libcamera/pipeline/rpi/common/rpi_stream.h      | 2 +-\n>  3 files changed, 3 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index f244edc68a85..fc0c0ec3c53c 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -764,7 +764,7 @@ int PipelineHandlerBase::queueRequestDevice(Camera *camera, Request *request)\n>                          * outside the v4l2 device. Store it in the stream buffer list\n>                          * so we can track it.\n>                          */\n> -                       stream->setExternalBuffer(buffer);\n> +                       stream->setExportedBuffer(buffer);\n\nI'm a bit confused.\n\nAren't these (externally allocated buffers) 'imported' buffers ?\n\nDoes this conflict with the 'RPi::Stream::setExportedBuffers() call?\n\nThat said - both setExportedBuffer and setExportedBuffers perform the\nsame action (on each buffer) - so it seems this is expected behaviour...\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\nThere's a comment in Stream::prepareBuffers()\n   /* Add these exported buffers to the internal/external buffer list. */\n\nIs that incorrect/redundant now? Is this list really tracking\ninternal/external buffers?\n\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n\n\n>                 }\n>  \n>                 /*\n> diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp\n> index 74b5abf447c7..e1858c731f57 100644\n> --- a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp\n> @@ -76,7 +76,7 @@ unsigned int Stream::getBufferId(FrameBuffer *buffer) const\n>         return it->first;\n>  }\n>  \n> -void Stream::setExternalBuffer(FrameBuffer *buffer)\n> +void Stream::setExportedBuffer(FrameBuffer *buffer)\n>  {\n>         bufferMap_.emplace(id_.get(), buffer);\n>  }\n> diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.h b/src/libcamera/pipeline/rpi/common/rpi_stream.h\n> index ca591f99cc45..d1289c4679b9 100644\n> --- a/src/libcamera/pipeline/rpi/common/rpi_stream.h\n> +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.h\n> @@ -76,7 +76,7 @@ public:\n>         const BufferMap &getBuffers() const;\n>         unsigned int getBufferId(FrameBuffer *buffer) const;\n>  \n> -       void setExternalBuffer(FrameBuffer *buffer);\n> +       void setExportedBuffer(FrameBuffer *buffer);\n>  \n>         int prepareBuffers(unsigned int count);\n>         int queueBuffer(FrameBuffer *buffer);\n> -- \n> 2.34.1\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 6136FBE080\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  4 Sep 2023 09:01:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 82D43628E4;\n\tMon,  4 Sep 2023 11:01:36 +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 0965B61DF7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  4 Sep 2023 11:01:35 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DC90838A8;\n\tMon,  4 Sep 2023 11:00:08 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1693818096;\n\tbh=WIIJnjH5cE/mRG9tMznIzFurpa29oq5McbAkXrVfdLk=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=OZ2LFPsZVUxSNV19DyvZtCmvYgniOGF/u8rP2Fb0P/o6tVmCL4C3cP8/LtMyrs7VO\n\tW0pM1g8q4HyxWwZDtqga/70IOQVxKfoGJ88tL27vhfHg1FXhqql+nukPXyluuz/m7r\n\tnnmEqpcvHt6Ppw4u7dpBH+Osjxe2NqXih7vdyjrdfcx8LxylkE6i3ee+a8mNAByhfx\n\tUOJLDKYwmjwyvsWMx51QFfTz6pOSji4CXGKs/hMiK7/EWZdrvqSw127uM22fJ7zW4G\n\tooKtNTcer+qaCHv2+XRGpsiohOtHsabQSFvVcUcO55YnggVGxPiIEtUkLPXmtgAV6e\n\tNoh3FdqGcOJYw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1693818009;\n\tbh=WIIJnjH5cE/mRG9tMznIzFurpa29oq5McbAkXrVfdLk=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=mxalTnApv7qsoKKa1tdNyv5T53c9gnvuXqY2LYxckBh4BR5ISKQzed1MPvoAES3u3\n\tdcyZ6BpMAsl6f/ucfv/z1+FjFxJp1TL/6uzlOF7JJI6paJRat6uoIyJo/RGqGJZBoJ\n\tBLujLLsct0Y0cKfaemPprfzMrLUice+jmWhr3FMo="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"mxalTnAp\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20230725085540.24863-4-naush@raspberrypi.com>","References":"<20230725085540.24863-1-naush@raspberrypi.com>\n\t<20230725085540.24863-4-naush@raspberrypi.com>","To":"Naushir Patuck <naush@raspberrypi.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 04 Sep 2023 10:01:31 +0100","Message-ID":"<169381809181.277971.9881759822074537202@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 3/4] pipeline: rpi: Rename\n\tRPi::Stream::setExternalBuffer()","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>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27726,"web_url":"https://patchwork.libcamera.org/comment/27726/","msgid":"<CAEmqJPokGawKe0MmJdHyDD4eTs8gHMFCrFULAFytDR4MWOY1Pw@mail.gmail.com>","date":"2023-09-04T09:35:06","subject":"Re: [libcamera-devel] [PATCH v2 3/4] pipeline: rpi: Rename\n\tRPi::Stream::setExternalBuffer()","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Kieran,\n\nThank you for the review.\n\nOn Mon, 4 Sept 2023 at 10:01, Kieran Bingham\n<kieran.bingham@ideasonboard.com> wrote:\n>\n> Quoting Naushir Patuck via libcamera-devel (2023-07-25 09:55:39)\n> > Since we don't distinguish between externally and internally allocated\n> > dma bufs, rename this function to setExportedBuffer() to clearer on its\n> > function.\n> >\n> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > ---\n> >  src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 2 +-\n> >  src/libcamera/pipeline/rpi/common/rpi_stream.cpp    | 2 +-\n> >  src/libcamera/pipeline/rpi/common/rpi_stream.h      | 2 +-\n> >  3 files changed, 3 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > index f244edc68a85..fc0c0ec3c53c 100644\n> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > @@ -764,7 +764,7 @@ int PipelineHandlerBase::queueRequestDevice(Camera *camera, Request *request)\n> >                          * outside the v4l2 device. Store it in the stream buffer list\n> >                          * so we can track it.\n> >                          */\n> > -                       stream->setExternalBuffer(buffer);\n> > +                       stream->setExportedBuffer(buffer);\n>\n> I'm a bit confused.\n>\n> Aren't these (externally allocated buffers) 'imported' buffers ?\n\nI use \"exported\" as they are exported from the application.  I guess I\nwas trying to follow the convention from\nPipelineHandler::exportFrameBuffer that gets called through the\nFrameBufferAllocator route.  I do kind of agree that for both sound\nlike \"import\" might be the better term, but I can live it with :-)\n\n>\n> Does this conflict with the 'RPi::Stream::setExportedBuffers() call?\n\nDifferent prototype, but does exactly the same function - it adds one\nor more buffers to the availableBuffers_ vector.\n\n>\n> That said - both setExportedBuffer and setExportedBuffers perform the\n> same action (on each buffer) - so it seems this is expected behaviour...\n>\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>\n> There's a comment in Stream::prepareBuffers()\n>    /* Add these exported buffers to the internal/external buffer list. */\n>\n> Is that incorrect/redundant now? Is this list really tracking\n> internal/external buffers?\n\nNo, I think that comment is still valid as availableBuffers_ holds all\nthe buffers (internal or externally allocated) that the stream may\nhandle during its lifetime.\nIf any of that doesn't make sense, please shout!\n\nCheers,\nNaush\n\n>\n>\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>\n>\n>\n> >                 }\n> >\n> >                 /*\n> > diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp\n> > index 74b5abf447c7..e1858c731f57 100644\n> > --- a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp\n> > +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp\n> > @@ -76,7 +76,7 @@ unsigned int Stream::getBufferId(FrameBuffer *buffer) const\n> >         return it->first;\n> >  }\n> >\n> > -void Stream::setExternalBuffer(FrameBuffer *buffer)\n> > +void Stream::setExportedBuffer(FrameBuffer *buffer)\n> >  {\n> >         bufferMap_.emplace(id_.get(), buffer);\n> >  }\n> > diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.h b/src/libcamera/pipeline/rpi/common/rpi_stream.h\n> > index ca591f99cc45..d1289c4679b9 100644\n> > --- a/src/libcamera/pipeline/rpi/common/rpi_stream.h\n> > +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.h\n> > @@ -76,7 +76,7 @@ public:\n> >         const BufferMap &getBuffers() const;\n> >         unsigned int getBufferId(FrameBuffer *buffer) const;\n> >\n> > -       void setExternalBuffer(FrameBuffer *buffer);\n> > +       void setExportedBuffer(FrameBuffer *buffer);\n> >\n> >         int prepareBuffers(unsigned int count);\n> >         int queueBuffer(FrameBuffer *buffer);\n> > --\n> > 2.34.1\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 7E873BE080\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  4 Sep 2023 09:35:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B0922628EA;\n\tMon,  4 Sep 2023 11:35:44 +0200 (CEST)","from mail-yw1-x112f.google.com (mail-yw1-x112f.google.com\n\t[IPv6:2607:f8b0:4864:20::112f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8577361DF7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  4 Sep 2023 11:35:43 +0200 (CEST)","by mail-yw1-x112f.google.com with SMTP id\n\t00721157ae682-5920efd91c7so11861067b3.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 04 Sep 2023 02:35:43 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1693820144;\n\tbh=LyEHvVS+JgdPypyHyT+gq3PkGZm3yHSIaEFmPfcOsdc=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=qb3vMhvXrvHhbRlsBGo3wXg/2fIrSGMQQVKx/q7ZFEyrLkDIWg5bCcuwYTXd2pNoq\n\tFH5WoKWoyfTUBbhksW8c4390s/iToWDE7KOpzESlVsXjGcnvKjDmEtelIlXJIc0Etk\n\toAtf8CpBVf5txipAmRB/pYf59+RQ1Vyz1H80hDjqh3LIvMawDM7QBhu+/rozaB3iIh\n\t/22GcgPN7oqz5RhdXrZdOrsmNMStjES+G9ZgMjg46QnJOHlg3CYzwCC8MEe3/3GyC9\n\tYTgFhlJOV5yt+p8zSEsewiY/YoOXOMCpAdc7FTA4bkkyQA+NevNSSA+VS8Fon4kYfV\n\t7gA3mhx+kuaMQ==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1693820142; x=1694424942;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=lVcSNOnTT5ZtDuEtd7AFYyLn4+GDBxT2qqfyVEd6J4g=;\n\tb=CponNyX9ErSdWeOb4YLBahpF4HQTGgeI12K44z3o5rMwV6eyaBNWpm0KIpbYP9qJ8c\n\toA2VNxsBcIPYBsLJQ6W6zqeUKx1k6ocsOv3Pmk9rUqSwry9RLCVe/lRjCtcpp4ln5ZnU\n\tjPEtpjAnt8aFOA6EwW6q+TphaWqa4f65a2Nmjo4rtgtBYgoiYJVBViT/5U683lkGyhfC\n\t0ntDUJ2Dmu6q4dJqxL3gEP6MhlSFaYhMxjrPQZssUNUYW/dvogZEnjA5ZCNM6fPT9goZ\n\tNyM1ahgyEjw22wiTBti/KZvCcpaL8dohg76P9ZVQEmPfgXzadtIfcZ6/w4oYNubT3d0O\n\tIjPQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"CponNyX9\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1693820142; x=1694424942;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=lVcSNOnTT5ZtDuEtd7AFYyLn4+GDBxT2qqfyVEd6J4g=;\n\tb=YmwKX3jjWURbsw/oHajuDHQBPhTC5Vsry8orb8f5dnMk9JySgUMjOhMMn9ybOYAbmB\n\tbqvHUSHy7JD0qhM9/u3gy1AKHfpGkHi1MV6FNPABW0EYjlJkGcQWCGfPvRr57RznPihB\n\tVc008vK9PBDFl+3PVzp8F5Rk2VYQougI4C6YeAfsphC2h7/TQtPc0HkdA7Rv9Wkvba16\n\tTJr9Ak2CIDKMLmT3JcsOeFWQuxMeN63sZC9Qacgudib7yn9sk0JsRSVjH8rMFb24cIQ+\n\tcVIvyE6YtzTFyVA2xSUf9FH9sd22VTWJhFb3Evh52NVZjrQ4jTBn8LZoVkxwAFethMT4\n\tjDpQ==","X-Gm-Message-State":"AOJu0Yy0AA3XzKXckw0k5CzXaIy5P3QIiZL+r3LDieoieIxwVhH+wg50\n\takUw9NXNmNyI4OMcAI1wwcUki64Fpi2Tfzgd0IXZ5tKF+e/MQQ57KBlVuA==","X-Google-Smtp-Source":"AGHT+IGvUt1/dxeL7z5X2iu2SZho8LOF2PwNfblRoY6acJdFZr/Gn3iFW6TL4GTHXTh/7itGTKbUV/na00Mthvt26QM=","X-Received":"by 2002:a0d:d50c:0:b0:570:6d74:21d5 with SMTP id\n\tx12-20020a0dd50c000000b005706d7421d5mr10722162ywd.13.1693820142311;\n\tMon, 04 Sep 2023 02:35:42 -0700 (PDT)","MIME-Version":"1.0","References":"<20230725085540.24863-1-naush@raspberrypi.com>\n\t<20230725085540.24863-4-naush@raspberrypi.com>\n\t<169381809181.277971.9881759822074537202@ping.linuxembedded.co.uk>","In-Reply-To":"<169381809181.277971.9881759822074537202@ping.linuxembedded.co.uk>","Date":"Mon, 4 Sep 2023 10:35:06 +0100","Message-ID":"<CAEmqJPokGawKe0MmJdHyDD4eTs8gHMFCrFULAFytDR4MWOY1Pw@mail.gmail.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 3/4] pipeline: rpi: Rename\n\tRPi::Stream::setExternalBuffer()","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>","From":"Naushir Patuck via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27727,"web_url":"https://patchwork.libcamera.org/comment/27727/","msgid":"<169382218999.277971.10417705259707488788@ping.linuxembedded.co.uk>","date":"2023-09-04T10:09:49","subject":"Re: [libcamera-devel] [PATCH v2 3/4] pipeline: rpi: Rename\n\tRPi::Stream::setExternalBuffer()","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Naushir Patuck (2023-09-04 10:35:06)\n> Hi Kieran,\n> \n> Thank you for the review.\n> \n> On Mon, 4 Sept 2023 at 10:01, Kieran Bingham\n> <kieran.bingham@ideasonboard.com> wrote:\n> >\n> > Quoting Naushir Patuck via libcamera-devel (2023-07-25 09:55:39)\n> > > Since we don't distinguish between externally and internally allocated\n> > > dma bufs, rename this function to setExportedBuffer() to clearer on its\n> > > function.\n> > >\n> > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > > ---\n> > >  src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 2 +-\n> > >  src/libcamera/pipeline/rpi/common/rpi_stream.cpp    | 2 +-\n> > >  src/libcamera/pipeline/rpi/common/rpi_stream.h      | 2 +-\n> > >  3 files changed, 3 insertions(+), 3 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > index f244edc68a85..fc0c0ec3c53c 100644\n> > > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > @@ -764,7 +764,7 @@ int PipelineHandlerBase::queueRequestDevice(Camera *camera, Request *request)\n> > >                          * outside the v4l2 device. Store it in the stream buffer list\n> > >                          * so we can track it.\n> > >                          */\n> > > -                       stream->setExternalBuffer(buffer);\n> > > +                       stream->setExportedBuffer(buffer);\n> >\n> > I'm a bit confused.\n> >\n> > Aren't these (externally allocated buffers) 'imported' buffers ?\n> \n> I use \"exported\" as they are exported from the application.  I guess I\n> was trying to follow the convention from\n> PipelineHandler::exportFrameBuffer that gets called through the\n> FrameBufferAllocator route.  I do kind of agree that for both sound\n> like \"import\" might be the better term, but I can live it with :-)\n> \n> >\n> > Does this conflict with the 'RPi::Stream::setExportedBuffers() call?\n> \n> Different prototype, but does exactly the same function - it adds one\n> or more buffers to the availableBuffers_ vector.\n> \n> >\n> > That said - both setExportedBuffer and setExportedBuffers perform the\n> > same action (on each buffer) - so it seems this is expected behaviour...\n> >\n> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> >\n> > There's a comment in Stream::prepareBuffers()\n> >    /* Add these exported buffers to the internal/external buffer list. */\n> >\n> > Is that incorrect/redundant now? Is this list really tracking\n> > internal/external buffers?\n> \n> No, I think that comment is still valid as availableBuffers_ holds all\n> the buffers (internal or externally allocated) that the stream may\n> handle during its lifetime.\n> If any of that doesn't make sense, please shout!\n\nI think it's clearer now. The list doesn't distinguish between\ninternally/externally allocated buffers - it's a list of /all/ buffers\nused by the stream.\n\n--\nKieran\n\n> \n> Cheers,\n> Naush\n> \n> >\n> >\n> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> >\n> >\n> >\n> > >                 }\n> > >\n> > >                 /*\n> > > diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp\n> > > index 74b5abf447c7..e1858c731f57 100644\n> > > --- a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp\n> > > +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp\n> > > @@ -76,7 +76,7 @@ unsigned int Stream::getBufferId(FrameBuffer *buffer) const\n> > >         return it->first;\n> > >  }\n> > >\n> > > -void Stream::setExternalBuffer(FrameBuffer *buffer)\n> > > +void Stream::setExportedBuffer(FrameBuffer *buffer)\n> > >  {\n> > >         bufferMap_.emplace(id_.get(), buffer);\n> > >  }\n> > > diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.h b/src/libcamera/pipeline/rpi/common/rpi_stream.h\n> > > index ca591f99cc45..d1289c4679b9 100644\n> > > --- a/src/libcamera/pipeline/rpi/common/rpi_stream.h\n> > > +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.h\n> > > @@ -76,7 +76,7 @@ public:\n> > >         const BufferMap &getBuffers() const;\n> > >         unsigned int getBufferId(FrameBuffer *buffer) const;\n> > >\n> > > -       void setExternalBuffer(FrameBuffer *buffer);\n> > > +       void setExportedBuffer(FrameBuffer *buffer);\n> > >\n> > >         int prepareBuffers(unsigned int count);\n> > >         int queueBuffer(FrameBuffer *buffer);\n> > > --\n> > > 2.34.1\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 CEB59C0F1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  4 Sep 2023 10:09:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5EA11628E9;\n\tMon,  4 Sep 2023 12:09:55 +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 91ACD628D7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  4 Sep 2023 12:09:53 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8761FCCE;\n\tMon,  4 Sep 2023 12:08:27 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1693822195;\n\tbh=8zcm7hS6BuXzI8PkjGYQChm2R57kRrltPYWfIrxIcxo=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=pej8hgXscfLHQrl+6PzBCUdoBJl/mH8MtdZoZF54f0KHzIrQjzAu0tbtcNSqebnTo\n\tx1ZCgNhXaWY288RcBU2MDiSuojVfzpSJatof4LzZcBjSaO64GNFMMOlvgr4oXIwN95\n\tD9VNSlu/HOHwg6mb5zdrHAnqJaXXCNVyQF26A2GGx6cILvufXfGVqPCNhMWs1WfaxD\n\t3ITOUjaKQ3XvNutYE2B993i2wi7Lbp+xndzK/20qglW8UE78plADphCD6//qLYIa/M\n\thVFOzxeOmtVmFYhPAg7Bq11aldWqTMt/FK+024xdNlbnndNNzdzn+qzfneCZAvQQlq\n\tD+6D6OVMgOhbg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1693822107;\n\tbh=8zcm7hS6BuXzI8PkjGYQChm2R57kRrltPYWfIrxIcxo=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=ngb9XFjn9No/t8ia4jl3nWFYKRlhFuA0U+BMeHiKdFFD45slJ1xVaxZiqrUi2d9WQ\n\tx71lorNc7+TrDQhrSXShbs8qVymBXU91We1HDNloPokyuz4Q2kXGABtYAHVqKCu1eh\n\tqtULAhQif3NBfnS+rGSWyJskr9jpeMCkfpU1NI48="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"ngb9XFjn\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<CAEmqJPokGawKe0MmJdHyDD4eTs8gHMFCrFULAFytDR4MWOY1Pw@mail.gmail.com>","References":"<20230725085540.24863-1-naush@raspberrypi.com>\n\t<20230725085540.24863-4-naush@raspberrypi.com>\n\t<169381809181.277971.9881759822074537202@ping.linuxembedded.co.uk>\n\t<CAEmqJPokGawKe0MmJdHyDD4eTs8gHMFCrFULAFytDR4MWOY1Pw@mail.gmail.com>","To":"Naushir Patuck <naush@raspberrypi.com>","Date":"Mon, 04 Sep 2023 11:09:49 +0100","Message-ID":"<169382218999.277971.10417705259707488788@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 3/4] pipeline: rpi: Rename\n\tRPi::Stream::setExternalBuffer()","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>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]