[{"id":20136,"web_url":"https://patchwork.libcamera.org/comment/20136/","msgid":"<163403702390.2943484.1161715016450327751@Monstersaurus>","date":"2021-10-12T11:10:23","subject":"Re: [libcamera-devel] [PATCH 2/3] cam: Pass stream names to FileSink","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2021-10-12 03:23:20)\n> The FileSink class constructs stream names internally the same way that\n> the CameraSession does, except that it fails to add the camera name.\n> This results in files being written without the camera name.\n> \n> This could be fixed in FileSink, but we would still duplicate code to\n> construct stream names. Pass the stream names map from CameraSession to\n> FileSink instead, and store it internally.\n\nAyee ... this seems like a single FileSink sinks multiple streams. I\nguess that's ok, and perhaps it simplifies the connections - but in my\nmind a sink is a single sink.\n\nSo each sink should be  constructed with it's appropriate name rather\nthan passing in an array.\n\nBut if that's how it's already implemented, then this is a shorter fix\nfor getting the names in so I'm going to go along with it.\n\nPerhaps I can see some benefits to a single sink receiving all streams\n(i.e. a fully completed request) so it can pick and choose what it wants\nto represent. A view finder sink might show all streams perhaps for\ninstance, or just the smallest ;-)\n\nAcked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> \n> Fixes: 02001fecb0f5 (\"cam: Turn BufferWriter into a FrameSink\")\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  src/cam/camera_session.cpp |  5 +++--\n>  src/cam/file_sink.cpp      | 11 +++--------\n>  src/cam/file_sink.h        |  3 ++-\n>  3 files changed, 8 insertions(+), 11 deletions(-)\n> \n> diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp\n> index 5a25baab03f5..605018278c5a 100644\n> --- a/src/cam/camera_session.cpp\n> +++ b/src/cam/camera_session.cpp\n> @@ -193,9 +193,10 @@ int CameraSession::start()\n>  \n>         if (options_.isSet(OptFile)) {\n>                 if (!options_[OptFile].toString().empty())\n> -                       sink_ = std::make_unique<FileSink>(options_[OptFile]);\n> +                       sink_ = std::make_unique<FileSink>(streamNames_,\n> +                                                          options_[OptFile]);\n>                 else\n> -                       sink_ = std::make_unique<FileSink>();\n> +                       sink_ = std::make_unique<FileSink>(streamNames_);\n>         }\n>  \n>         if (sink_) {\n> diff --git a/src/cam/file_sink.cpp b/src/cam/file_sink.cpp\n> index 3c2e565b27a2..45213d4a54fe 100644\n> --- a/src/cam/file_sink.cpp\n> +++ b/src/cam/file_sink.cpp\n> @@ -20,8 +20,9 @@\n>  \n>  using namespace libcamera;\n>  \n> -FileSink::FileSink(const std::string &pattern)\n> -       : pattern_(pattern)\n> +FileSink::FileSink(const std::map<const libcamera::Stream *, std::string> &streamNames,\n> +                  const std::string &pattern)\n> +       : streamNames_(streamNames), pattern_(pattern)\n>  {\n>  }\n>  \n> @@ -35,12 +36,6 @@ int FileSink::configure(const libcamera::CameraConfiguration &config)\n>         if (ret < 0)\n>                 return ret;\n>  \n> -       streamNames_.clear();\n> -       for (unsigned int index = 0; index < config.size(); ++index) {\n> -               const StreamConfiguration &cfg = config.at(index);\n> -               streamNames_[cfg.stream()] = \"stream\" + std::to_string(index);\n> -       }\n> -\n>         return 0;\n>  }\n>  \n> diff --git a/src/cam/file_sink.h b/src/cam/file_sink.h\n> index 335be93b8732..8de93a01a1e8 100644\n> --- a/src/cam/file_sink.h\n> +++ b/src/cam/file_sink.h\n> @@ -20,7 +20,8 @@ class Image;\n>  class FileSink : public FrameSink\n>  {\n>  public:\n> -       FileSink(const std::string &pattern = \"\");\n> +       FileSink(const std::map<const libcamera::Stream *, std::string> &streamNames,\n> +                const std::string &pattern = \"\");\n>         ~FileSink();\n>  \n>         int configure(const libcamera::CameraConfiguration &config) override;\n> -- \n> Regards,\n> \n> Laurent Pinchart\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 B2AF3C323E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 12 Oct 2021 11:10:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4561C68F4D;\n\tTue, 12 Oct 2021 13:10:28 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A6CEB68F4C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 12 Oct 2021 13:10:26 +0200 (CEST)","from pendragon.ideasonboard.com\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 45C66F1;\n\tTue, 12 Oct 2021 13:10:26 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"f8IOD2u7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634037026;\n\tbh=pQ8FA5JBjBHrc7fR0gbYCCC8SwR8AzOgPA6QzqEeSxs=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=f8IOD2u7mNfZYSXO2vPh8eEVQ4cTGhxNZU+mIFh3YroWjwSVjlvyMdx1srfvGLhUL\n\tzpZGKAP1JmuxyEhY/pz9kSUXk1RjwMOSX8u933x+VjU36zP9Ikv/vECQu545+Y1RWF\n\tmXfTDn/NUXPXiSazr54ZJmilzuoaCeNRUA2IYBiw=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211012022321.27034-3-laurent.pinchart@ideasonboard.com>","References":"<20211012022321.27034-1-laurent.pinchart@ideasonboard.com>\n\t<20211012022321.27034-3-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 12 Oct 2021 12:10:23 +0100","Message-ID":"<163403702390.2943484.1161715016450327751@Monstersaurus>","User-Agent":"alot/0.9.1","Subject":"Re: [libcamera-devel] [PATCH 2/3] cam: Pass stream names to FileSink","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]