[{"id":31383,"web_url":"https://patchwork.libcamera.org/comment/31383/","msgid":"<878qve3h4t.fsf@redhat.com>","date":"2024-09-26T09:21:06","subject":"Re: [PATCH 2/2] apps: cam: Print an error when outputting DNG and\n\tDNG support is missing","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Laurent,\n\nthank you for the patch, nice improvement.\n\nLaurent Pinchart writes:\n\n> When DNG support is missing, the cam application ignores the .dng suffix\n> of the file pattern and writes raw binary data instead, without\n> notifying the user. This leads to confusion. Fix it by printing an error\n> message.\n>\n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Milan Zamazal \n\n> ---\n> src/apps/cam/camera_session.cpp | 15 ++++++---\n> src/apps/cam/file_sink.cpp | 60 +++++++++++++++++++++++----------\n> src/apps/cam/file_sink.h | 18 ++++++++--\n> 3 files changed, 68 insertions(+), 25 deletions(-)\n>\n> diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp\n> index 097dc479241a..227df9e922ea 100644\n> --- a/src/apps/cam/camera_session.cpp\n> +++ b/src/apps/cam/camera_session.cpp\n> @@ -230,11 +230,16 @@ int CameraSession::start()\n> #endif\n> \n> \tif (options_.isSet(OptFile)) {\n> -\t\tif (!options_[OptFile].toString().empty())\n> -\t\t\tsink_ = std::make_unique(camera_.get(), streamNames_,\n> -\t\t\t\t\t\t\t options_[OptFile]);\n> -\t\telse\n> -\t\t\tsink_ = std::make_unique(camera_.get(), streamNames_);\n> +\t\tstd::unique_ptr sink =\n> +\t\t\tstd::make_unique(camera_.get(), streamNames_);\n> +\n> +\t\tif (!options_[OptFile].toString().empty()) {\n> +\t\t\tret = sink->setFilePattern(options_[OptFile]);\n> +\t\t\tif (ret)\n> +\t\t\t\treturn ret;\n> +\t\t}\n> +\n> +\t\tsink_ = std::move(sink);\n> \t}\n> \n> \tif (sink_) {\n> diff --git a/src/apps/cam/file_sink.cpp b/src/apps/cam/file_sink.cpp\n> index 3e000d2fd9c6..76e21db9bf9a 100644\n> --- a/src/apps/cam/file_sink.cpp\n> +++ b/src/apps/cam/file_sink.cpp\n> @@ -5,6 +5,7 @@\n> * File Sink\n> */\n> \n> +#include \n> #include \n> #include \n> #include \n> @@ -12,6 +13,7 @@\n> #include \n> #include \n> #include \n> +#include \n> \n> #include \n> \n> @@ -24,13 +26,13 @@\n> using namespace libcamera;\n> \n> FileSink::FileSink([[maybe_unused]] const libcamera::Camera *camera,\n> -\t\t const std::map &streamNames,\n> -\t\t const std::string &pattern)\n> +\t\t const std::map &streamNames)\n> \t:\n> #ifdef HAVE_TIFF\n> \t camera_(camera),\n> #endif\n> -\t streamNames_(streamNames), pattern_(pattern)\n> +\t pattern_(kDefaultFilePattern), fileType_(FileType::Binary),\n> +\t streamNames_(streamNames)\n> {\n> }\n> \n> @@ -38,6 +40,41 @@ FileSink::~FileSink()\n> {\n> }\n> \n> +int FileSink::setFilePattern(const std::string &pattern)\n> +{\n> +\tstatic const std::array, 2> types{{\n> +\t\t{ \".dng\", FileType::Dng },\n> +\t\t{ \".ppm\", FileType::Ppm },\n> +\t}};\n> +\n> +\tpattern_ = pattern;\n> +\n> +\tif (pattern_.empty() || pattern_.back() == '/')\n> +\t\tpattern_ += kDefaultFilePattern;\n> +\n> +\tfileType_ = FileType::Binary;\n> +\n> +\tfor (const auto &type : types) {\n> +\t\tif (pattern_.size() < type.first.size())\n> +\t\t\tcontinue;\n> +\n> +\t\tif (pattern_.find(type.first, pattern_.size() - type.first.size()) !=\n> +\t\t std::string::npos) {\n> +\t\t\tfileType_ = type.second;\n> +\t\t\tbreak;\n> +\t\t}\n> +\t}\n> +\n> +#ifndef HAVE_TIFF\n> +\tif (fileType_ == FileType::Dng) {\n> +\t\tstd::cerr << \"DNG support not available\" << std::endl;\n> +\t\treturn -EINVAL;\n> +\t}\n> +#endif /* HAVE_TIFF */\n> +\n> +\treturn 0;\n> +}\n> +\n> int FileSink::configure(const libcamera::CameraConfiguration &config)\n> {\n> \tint ret = FrameSink::configure(config);\n> @@ -67,21 +104,10 @@ bool FileSink::processRequest(Request *request)\n> void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer,\n> \t\t\t [[maybe_unused]] const ControlList &metadata)\n> {\n> -\tstd::string filename;\n> +\tstd::string filename = pattern_;\n> \tsize_t pos;\n> \tint fd, ret = 0;\n> \n> -\tif (!pattern_.empty())\n> -\t\tfilename = pattern_;\n> -\n> -#ifdef HAVE_TIFF\n> -\tbool dng = filename.find(\".dng\", filename.size() - 4) != std::string::npos;\n> -#endif /* HAVE_TIFF */\n> -\tbool ppm = filename.find(\".ppm\", filename.size() - 4) != std::string::npos;\n> -\n> -\tif (filename.empty() || filename.back() == '/')\n> -\t\tfilename += \"frame-#.bin\";\n> -\n> \tpos = filename.find_first_of('#');\n> \tif (pos != std::string::npos) {\n> \t\tstd::stringstream ss;\n> @@ -93,7 +119,7 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer,\n> \tImage *image = mappedBuffers_[buffer].get();\n> \n> #ifdef HAVE_TIFF\n> -\tif (dng) {\n> +\tif (fileType_ == FileType::Dng) {\n> \t\tret = DNGWriter::write(filename.c_str(), camera_,\n> \t\t\t\t stream->configuration(), metadata,\n> \t\t\t\t buffer, image->data(0).data());\n> @@ -104,7 +130,7 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer,\n> \t\treturn;\n> \t}\n> #endif /* HAVE_TIFF */\n> -\tif (ppm) {\n> +\tif (fileType_ == FileType::Ppm) {\n> \t\tret = PPMWriter::write(filename.c_str(), stream->configuration(),\n> \t\t\t\t image->data(0));\n> \t\tif (ret < 0)\n> diff --git a/src/apps/cam/file_sink.h b/src/apps/cam/file_sink.h\n> index 9d560783af09..71b7fe0feab5 100644\n> --- a/src/apps/cam/file_sink.h\n> +++ b/src/apps/cam/file_sink.h\n> @@ -21,10 +21,11 @@ class FileSink : public FrameSink\n> {\n> public:\n> \tFileSink(const libcamera::Camera *camera,\n> -\t\t const std::map &streamNames,\n> -\t\t const std::string &pattern = \"\");\n> +\t\t const std::map &streamNames);\n> \t~FileSink();\n> \n> +\tint setFilePattern(const std::string &pattern);\n> +\n> \tint configure(const libcamera::CameraConfiguration &config) override;\n> \n> \tvoid mapBuffer(libcamera::FrameBuffer *buffer) override;\n> @@ -32,6 +33,14 @@ public:\n> \tbool processRequest(libcamera::Request *request) override;\n> \n> private:\n> +\tstatic constexpr const char *kDefaultFilePattern = \"frame-#.bin\";\n> +\n> +\tenum class FileType {\n> +\t\tBinary,\n> +\t\tDng,\n> +\t\tPpm,\n> +\t};\n> +\n> \tvoid writeBuffer(const libcamera::Stream *stream,\n> \t\t\t libcamera::FrameBuffer *buffer,\n> \t\t\t const libcamera::ControlList &metadata);\n> @@ -39,7 +48,10 @@ private:\n> #ifdef HAVE_TIFF\n> \tconst libcamera::Camera *camera_;\n> #endif\n> -\tstd::map streamNames_;\n> +\n> \tstd::string pattern_;\n> +\tFileType fileType_;\n> +\n> +\tstd::map streamNames_;\n> \tstd::map> mappedBuffers_;\n> };","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 2F0F9C0F1B\n\tfor ;\n\tThu, 26 Sep 2024 09:21:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AD5DE63512;\n\tThu, 26 Sep 2024 11:21:17 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E6B85634FB\n\tfor ;\n\tThu, 26 Sep 2024 11:21:12 +0200 (CEST)","from mail-lf1-f69.google.com (mail-lf1-f69.google.com\n\t[209.85.167.69]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-127-rAtZ7znPPYCky4qQOjzdJA-1; Thu, 26 Sep 2024 05:21:10 -0400","by mail-lf1-f69.google.com with SMTP id\n\t2adb3069b0e04-536589936d3so619378e87.0\n\tfor ;\n\tThu, 26 Sep 2024 02:21:10 -0700 (PDT)","from nuthatch (ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\ta640c23a62f3a-a9392f34809sm328004966b.37.2024.09.26.02.21.07\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 26 Sep 2024 02:21:07 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"C8+djEtx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1727342471;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=Xyc3kdePAKJBvgRA5Np5YG+IrG+OeObJCgtuLnqEHuM=;\n\tb=C8+djEtx+eXBcQtWFM26RSEyqgi3pNrYIIvH9pIvV5GfVt2tAH9z4rvYpv8UObn9w/PBZq\n\tsxzcknYWp4xxs4FR5cb+SXhFyGo//3a3Y5pT6IXqmJn/w5PUWo4Cmu4G4NsLkMsaPbhDBm\n\tRQeV9qdienXhsE3dB83si3yS5q3X3N0=","X-MC-Unique":"rAtZ7znPPYCky4qQOjzdJA-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1727342469; x=1727947269;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=Xyc3kdePAKJBvgRA5Np5YG+IrG+OeObJCgtuLnqEHuM=;\n\tb=Du+eFPgHlLoDWGehRLoKGH3AqrnM2ZWqU7yN/TehZO8SbFyYhe7yC6F/pzDPFW0JM2\n\tbv03lBQFlJoigL3qVlSd6PbCp0bCqOdCPZxEkjHLbpgir3cY/j9Ibk6A6YL45aBhb62w\n\tVqbJi4kJKSHCcw0AIOvH9AoidKR7dIH7tS1BzZ6gN4mjV/oH5a42TuWa79bFmyJ9H4Fa\n\t0wSGtWjUfCAZQYccY8xYQRfNb9CnwAZ0ctagppGvUOW2DVB/8SVw4Xlgyn3ovM+VIfAw\n\tCmTsjei4xM/DQdVZxcOp+5C475/fDtaJMN1qojk/dwutffNZ3bMesyAa1BhgCTOWXhvp\n\troVA==","X-Gm-Message-State":"AOJu0YygYiE+JKraP0KkUeadAZI0KQbBlofYhitZhmfz5VKOSgBTehvB\n\t8f15T3B+jMtiP5Ox+kjF7FUwDxah24KxQAF8ANyQ7FvltoQTDBINTBgDeA9N3WginoWsu7hGhe+\n\twosMQuQF8V+G5WFONx69MgXDcTRsLPGnoXy+7NQXjUvUcwIOuI468vLg6yWAeqACxk7XSLFc=","X-Received":["by 2002:a05:6512:1188:b0:52b:bd90:29c8 with SMTP id\n\t2adb3069b0e04-538801abfd6mr2805312e87.60.1727342468745; \n\tThu, 26 Sep 2024 02:21:08 -0700 (PDT)","by 2002:a05:6512:1188:b0:52b:bd90:29c8 with SMTP id\n\t2adb3069b0e04-538801abfd6mr2805294e87.60.1727342468219; \n\tThu, 26 Sep 2024 02:21:08 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IFuWs5l1Fgyk3Qujx9wFTV+v8dwWJ76dvz5ZtIsaxOachg88g001lTFkVk5G3/pcrHR+Lqo2g==","From":"Milan Zamazal ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org, Arne Caspari\n\t","Subject":"Re: [PATCH 2/2] apps: cam: Print an error when outputting DNG and\n\tDNG support is missing","In-Reply-To":"<20240925152134.20284-3-laurent.pinchart@ideasonboard.com>\n\t(Laurent Pinchart's message of \"Wed, 25 Sep 2024 18:21:34 +0300\")","References":"<20240925152134.20284-1-laurent.pinchart@ideasonboard.com>\n\t<20240925152134.20284-3-laurent.pinchart@ideasonboard.com>","Date":"Thu, 26 Sep 2024 11:21:06 +0200","Message-ID":"<878qve3h4t.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31404,"web_url":"https://patchwork.libcamera.org/comment/31404/","msgid":"","date":"2024-09-26T13:59:42","subject":"Re: [PATCH 2/2] apps: cam: Print an error when outputting DNG and\n\tDNG support is missing","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Laurent\n\nOn Wed, Sep 25, 2024 at 06:21:34PM GMT, Laurent Pinchart wrote:\n> When DNG support is missing, the cam application ignores the .dng suffix\n> of the file pattern and writes raw binary data instead, without\n> notifying the user. This leads to confusion. Fix it by printing an error\n> message.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/apps/cam/camera_session.cpp | 15 ++++++---\n> src/apps/cam/file_sink.cpp | 60 +++++++++++++++++++++++----------\n> src/apps/cam/file_sink.h | 18 ++++++++--\n> 3 files changed, 68 insertions(+), 25 deletions(-)\n>\n> diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp\n> index 097dc479241a..227df9e922ea 100644\n> --- a/src/apps/cam/camera_session.cpp\n> +++ b/src/apps/cam/camera_session.cpp\n> @@ -230,11 +230,16 @@ int CameraSession::start()\n> #endif\n>\n> \tif (options_.isSet(OptFile)) {\n> -\t\tif (!options_[OptFile].toString().empty())\n> -\t\t\tsink_ = std::make_unique(camera_.get(), streamNames_,\n> -\t\t\t\t\t\t\t options_[OptFile]);\n> -\t\telse\n> -\t\t\tsink_ = std::make_unique(camera_.get(), streamNames_);\n> +\t\tstd::unique_ptr sink =\n> +\t\t\tstd::make_unique(camera_.get(), streamNames_);\n> +\n> +\t\tif (!options_[OptFile].toString().empty()) {\n> +\t\t\tret = sink->setFilePattern(options_[OptFile]);\n> +\t\t\tif (ret)\n> +\t\t\t\treturn ret;\n> +\t\t}\n> +\n> +\t\tsink_ = std::move(sink);\n> \t}\n>\n> \tif (sink_) {\n> diff --git a/src/apps/cam/file_sink.cpp b/src/apps/cam/file_sink.cpp\n> index 3e000d2fd9c6..76e21db9bf9a 100644\n> --- a/src/apps/cam/file_sink.cpp\n> +++ b/src/apps/cam/file_sink.cpp\n> @@ -5,6 +5,7 @@\n> * File Sink\n> */\n>\n> +#include \n> #include \n> #include \n> #include \n> @@ -12,6 +13,7 @@\n> #include \n> #include \n> #include \n> +#include \n>\n> #include \n>\n> @@ -24,13 +26,13 @@\n> using namespace libcamera;\n>\n> FileSink::FileSink([[maybe_unused]] const libcamera::Camera *camera,\n> -\t\t const std::map &streamNames,\n> -\t\t const std::string &pattern)\n> +\t\t const std::map &streamNames)\n> \t:\n> #ifdef HAVE_TIFF\n> \t camera_(camera),\n> #endif\n\nI wonder if we actually need this or we could initialize the camera_\nmember unconditionally (and drop the above [[maybe_unused]])\n\n> -\t streamNames_(streamNames), pattern_(pattern)\n> +\t pattern_(kDefaultFilePattern), fileType_(FileType::Binary),\n> +\t streamNames_(streamNames)\n> {\n> }\n>\n> @@ -38,6 +40,41 @@ FileSink::~FileSink()\n> {\n> }\n>\n> +int FileSink::setFilePattern(const std::string &pattern)\n> +{\n> +\tstatic const std::array, 2> types{{\n> +\t\t{ \".dng\", FileType::Dng },\n> +\t\t{ \".ppm\", FileType::Ppm },\n> +\t}};\n> +\n> +\tpattern_ = pattern;\n> +\n> +\tif (pattern_.empty() || pattern_.back() == '/')\n> +\t\tpattern_ += kDefaultFilePattern;\n> +\n> +\tfileType_ = FileType::Binary;\n> +\n> +\tfor (const auto &type : types) {\n> +\t\tif (pattern_.size() < type.first.size())\n> +\t\t\tcontinue;\n> +\n> +\t\tif (pattern_.find(type.first, pattern_.size() - type.first.size()) !=\n\nOr\n if (pattern_.rfind(type.first) != std::string::npos)\n\nThe rest looks good to me\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> +\t\t std::string::npos) {\n> +\t\t\tfileType_ = type.second;\n> +\t\t\tbreak;\n> +\t\t}\n> +\t}\n> +\n> +#ifndef HAVE_TIFF\n> +\tif (fileType_ == FileType::Dng) {\n> +\t\tstd::cerr << \"DNG support not available\" << std::endl;\n> +\t\treturn -EINVAL;\n> +\t}\n> +#endif /* HAVE_TIFF */\n> +\n> +\treturn 0;\n> +}\n> +\n> int FileSink::configure(const libcamera::CameraConfiguration &config)\n> {\n> \tint ret = FrameSink::configure(config);\n> @@ -67,21 +104,10 @@ bool FileSink::processRequest(Request *request)\n> void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer,\n> \t\t\t [[maybe_unused]] const ControlList &metadata)\n> {\n> -\tstd::string filename;\n> +\tstd::string filename = pattern_;\n> \tsize_t pos;\n> \tint fd, ret = 0;\n>\n> -\tif (!pattern_.empty())\n> -\t\tfilename = pattern_;\n> -\n> -#ifdef HAVE_TIFF\n> -\tbool dng = filename.find(\".dng\", filename.size() - 4) != std::string::npos;\n> -#endif /* HAVE_TIFF */\n> -\tbool ppm = filename.find(\".ppm\", filename.size() - 4) != std::string::npos;\n> -\n> -\tif (filename.empty() || filename.back() == '/')\n> -\t\tfilename += \"frame-#.bin\";\n> -\n> \tpos = filename.find_first_of('#');\n> \tif (pos != std::string::npos) {\n> \t\tstd::stringstream ss;\n> @@ -93,7 +119,7 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer,\n> \tImage *image = mappedBuffers_[buffer].get();\n>\n> #ifdef HAVE_TIFF\n> -\tif (dng) {\n> +\tif (fileType_ == FileType::Dng) {\n> \t\tret = DNGWriter::write(filename.c_str(), camera_,\n> \t\t\t\t stream->configuration(), metadata,\n> \t\t\t\t buffer, image->data(0).data());\n> @@ -104,7 +130,7 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer,\n> \t\treturn;\n> \t}\n> #endif /* HAVE_TIFF */\n> -\tif (ppm) {\n> +\tif (fileType_ == FileType::Ppm) {\n> \t\tret = PPMWriter::write(filename.c_str(), stream->configuration(),\n> \t\t\t\t image->data(0));\n> \t\tif (ret < 0)\n> diff --git a/src/apps/cam/file_sink.h b/src/apps/cam/file_sink.h\n> index 9d560783af09..71b7fe0feab5 100644\n> --- a/src/apps/cam/file_sink.h\n> +++ b/src/apps/cam/file_sink.h\n> @@ -21,10 +21,11 @@ class FileSink : public FrameSink\n> {\n> public:\n> \tFileSink(const libcamera::Camera *camera,\n> -\t\t const std::map &streamNames,\n> -\t\t const std::string &pattern = \"\");\n> +\t\t const std::map &streamNames);\n> \t~FileSink();\n>\n> +\tint setFilePattern(const std::string &pattern);\n> +\n> \tint configure(const libcamera::CameraConfiguration &config) override;\n>\n> \tvoid mapBuffer(libcamera::FrameBuffer *buffer) override;\n> @@ -32,6 +33,14 @@ public:\n> \tbool processRequest(libcamera::Request *request) override;\n>\n> private:\n> +\tstatic constexpr const char *kDefaultFilePattern = \"frame-#.bin\";\n> +\n> +\tenum class FileType {\n> +\t\tBinary,\n> +\t\tDng,\n> +\t\tPpm,\n> +\t};\n> +\n> \tvoid writeBuffer(const libcamera::Stream *stream,\n> \t\t\t libcamera::FrameBuffer *buffer,\n> \t\t\t const libcamera::ControlList &metadata);\n> @@ -39,7 +48,10 @@ private:\n> #ifdef HAVE_TIFF\n> \tconst libcamera::Camera *camera_;\n> #endif\n> -\tstd::map streamNames_;\n> +\n> \tstd::string pattern_;\n> +\tFileType fileType_;\n> +\n> +\tstd::map streamNames_;\n> \tstd::map> mappedBuffers_;\n> };\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","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 D41F0C3257\n\tfor ;\n\tThu, 26 Sep 2024 13:59:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 013A06350F;\n\tThu, 26 Sep 2024 15:59:48 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8C820634F9\n\tfor ;\n\tThu, 26 Sep 2024 15:59:46 +0200 (CEST)","from ideasonboard.com (mob-5-90-51-229.net.vodafone.it\n\t[5.90.51.229])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 10DD5169;\n\tThu, 26 Sep 2024 15:58:18 +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=\"LWjSwEMv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727359098;\n\tbh=kt2cXiJch0C7gK2ajBz1zOtnOIeOJTXr5x4y/6lkKZI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=LWjSwEMvdCZjKSu9PV8TsjVvYlMMWYuRCtlrCJ/j/Z4MEPN4QJWBagMnoCtj/Dm9D\n\tTmEvPybdpw+M1iXgeXucEbCAasST2IANfNJ7h5ClGoTVA36QPNrs9JUUJBook1hbyn\n\tjzDz1JvdGckMgo4xcDFU7iJbgIkqvbFHcH7mByho=","Date":"Thu, 26 Sep 2024 15:59:42 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org, \n\tArne Caspari ","Subject":"Re: [PATCH 2/2] apps: cam: Print an error when outputting DNG and\n\tDNG support is missing","Message-ID":"","References":"<20240925152134.20284-1-laurent.pinchart@ideasonboard.com>\n\t<20240925152134.20284-3-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240925152134.20284-3-laurent.pinchart@ideasonboard.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31408,"web_url":"https://patchwork.libcamera.org/comment/31408/","msgid":"<20240926142507.GH21788@pendragon.ideasonboard.com>","date":"2024-09-26T14:25:07","subject":"Re: [PATCH 2/2] apps: cam: Print an error when outputting DNG and\n\tDNG support is missing","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Sep 26, 2024 at 03:59:42PM +0200, Jacopo Mondi wrote:\n> Hi Laurent\n> \n> On Wed, Sep 25, 2024 at 06:21:34PM GMT, Laurent Pinchart wrote:\n> > When DNG support is missing, the cam application ignores the .dng suffix\n> > of the file pattern and writes raw binary data instead, without\n> > notifying the user. This leads to confusion. Fix it by printing an error\n> > message.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/apps/cam/camera_session.cpp | 15 ++++++---\n> > src/apps/cam/file_sink.cpp | 60 +++++++++++++++++++++++----------\n> > src/apps/cam/file_sink.h | 18 ++++++++--\n> > 3 files changed, 68 insertions(+), 25 deletions(-)\n> >\n> > diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp\n> > index 097dc479241a..227df9e922ea 100644\n> > --- a/src/apps/cam/camera_session.cpp\n> > +++ b/src/apps/cam/camera_session.cpp\n> > @@ -230,11 +230,16 @@ int CameraSession::start()\n> > #endif\n> >\n> > \tif (options_.isSet(OptFile)) {\n> > -\t\tif (!options_[OptFile].toString().empty())\n> > -\t\t\tsink_ = std::make_unique(camera_.get(), streamNames_,\n> > -\t\t\t\t\t\t\t options_[OptFile]);\n> > -\t\telse\n> > -\t\t\tsink_ = std::make_unique(camera_.get(), streamNames_);\n> > +\t\tstd::unique_ptr sink =\n> > +\t\t\tstd::make_unique(camera_.get(), streamNames_);\n> > +\n> > +\t\tif (!options_[OptFile].toString().empty()) {\n> > +\t\t\tret = sink->setFilePattern(options_[OptFile]);\n> > +\t\t\tif (ret)\n> > +\t\t\t\treturn ret;\n> > +\t\t}\n> > +\n> > +\t\tsink_ = std::move(sink);\n> > \t}\n> >\n> > \tif (sink_) {\n> > diff --git a/src/apps/cam/file_sink.cpp b/src/apps/cam/file_sink.cpp\n> > index 3e000d2fd9c6..76e21db9bf9a 100644\n> > --- a/src/apps/cam/file_sink.cpp\n> > +++ b/src/apps/cam/file_sink.cpp\n> > @@ -5,6 +5,7 @@\n> > * File Sink\n> > */\n> >\n> > +#include \n> > #include \n> > #include \n> > #include \n> > @@ -12,6 +13,7 @@\n> > #include \n> > #include \n> > #include \n> > +#include \n> >\n> > #include \n> >\n> > @@ -24,13 +26,13 @@\n> > using namespace libcamera;\n> >\n> > FileSink::FileSink([[maybe_unused]] const libcamera::Camera *camera,\n> > -\t\t const std::map &streamNames,\n> > -\t\t const std::string &pattern)\n> > +\t\t const std::map &streamNames)\n> > \t:\n> > #ifdef HAVE_TIFF\n> > \t camera_(camera),\n> > #endif\n> \n> I wonder if we actually need this or we could initialize the camera_\n> member unconditionally (and drop the above [[maybe_unused]])\n\nI thought about that too but decided not to change it as it's a separate\nissue.\n\n> > -\t streamNames_(streamNames), pattern_(pattern)\n> > +\t pattern_(kDefaultFilePattern), fileType_(FileType::Binary),\n> > +\t streamNames_(streamNames)\n> > {\n> > }\n> >\n> > @@ -38,6 +40,41 @@ FileSink::~FileSink()\n> > {\n> > }\n> >\n> > +int FileSink::setFilePattern(const std::string &pattern)\n> > +{\n> > +\tstatic const std::array, 2> types{{\n> > +\t\t{ \".dng\", FileType::Dng },\n> > +\t\t{ \".ppm\", FileType::Ppm },\n> > +\t}};\n> > +\n> > +\tpattern_ = pattern;\n> > +\n> > +\tif (pattern_.empty() || pattern_.back() == '/')\n> > +\t\tpattern_ += kDefaultFilePattern;\n> > +\n> > +\tfileType_ = FileType::Binary;\n> > +\n> > +\tfor (const auto &type : types) {\n> > +\t\tif (pattern_.size() < type.first.size())\n> > +\t\t\tcontinue;\n> > +\n> > +\t\tif (pattern_.find(type.first, pattern_.size() - type.first.size()) !=\n> \n> Or\n> if (pattern_.rfind(type.first) != std::string::npos)\n\nWe want to check if pattern_ ends with type.first, not if type.first is\nfound somewhere inside pattern_. C++20 has a nice\nstd::string::ends_with()\n(https://en.cppreference.com/w/cpp/string/basic_string/ends_with), but\nC++17 doesn't :-(\n\n> The rest looks good to me\n> Reviewed-by: Jacopo Mondi \n> \n> > +\t\t std::string::npos) {\n> > +\t\t\tfileType_ = type.second;\n> > +\t\t\tbreak;\n> > +\t\t}\n> > +\t}\n> > +\n> > +#ifndef HAVE_TIFF\n> > +\tif (fileType_ == FileType::Dng) {\n> > +\t\tstd::cerr << \"DNG support not available\" << std::endl;\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +#endif /* HAVE_TIFF */\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > int FileSink::configure(const libcamera::CameraConfiguration &config)\n> > {\n> > \tint ret = FrameSink::configure(config);\n> > @@ -67,21 +104,10 @@ bool FileSink::processRequest(Request *request)\n> > void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer,\n> > \t\t\t [[maybe_unused]] const ControlList &metadata)\n> > {\n> > -\tstd::string filename;\n> > +\tstd::string filename = pattern_;\n> > \tsize_t pos;\n> > \tint fd, ret = 0;\n> >\n> > -\tif (!pattern_.empty())\n> > -\t\tfilename = pattern_;\n> > -\n> > -#ifdef HAVE_TIFF\n> > -\tbool dng = filename.find(\".dng\", filename.size() - 4) != std::string::npos;\n> > -#endif /* HAVE_TIFF */\n> > -\tbool ppm = filename.find(\".ppm\", filename.size() - 4) != std::string::npos;\n> > -\n> > -\tif (filename.empty() || filename.back() == '/')\n> > -\t\tfilename += \"frame-#.bin\";\n> > -\n> > \tpos = filename.find_first_of('#');\n> > \tif (pos != std::string::npos) {\n> > \t\tstd::stringstream ss;\n> > @@ -93,7 +119,7 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer,\n> > \tImage *image = mappedBuffers_[buffer].get();\n> >\n> > #ifdef HAVE_TIFF\n> > -\tif (dng) {\n> > +\tif (fileType_ == FileType::Dng) {\n> > \t\tret = DNGWriter::write(filename.c_str(), camera_,\n> > \t\t\t\t stream->configuration(), metadata,\n> > \t\t\t\t buffer, image->data(0).data());\n> > @@ -104,7 +130,7 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer,\n> > \t\treturn;\n> > \t}\n> > #endif /* HAVE_TIFF */\n> > -\tif (ppm) {\n> > +\tif (fileType_ == FileType::Ppm) {\n> > \t\tret = PPMWriter::write(filename.c_str(), stream->configuration(),\n> > \t\t\t\t image->data(0));\n> > \t\tif (ret < 0)\n> > diff --git a/src/apps/cam/file_sink.h b/src/apps/cam/file_sink.h\n> > index 9d560783af09..71b7fe0feab5 100644\n> > --- a/src/apps/cam/file_sink.h\n> > +++ b/src/apps/cam/file_sink.h\n> > @@ -21,10 +21,11 @@ class FileSink : public FrameSink\n> > {\n> > public:\n> > \tFileSink(const libcamera::Camera *camera,\n> > -\t\t const std::map &streamNames,\n> > -\t\t const std::string &pattern = \"\");\n> > +\t\t const std::map &streamNames);\n> > \t~FileSink();\n> >\n> > +\tint setFilePattern(const std::string &pattern);\n> > +\n> > \tint configure(const libcamera::CameraConfiguration &config) override;\n> >\n> > \tvoid mapBuffer(libcamera::FrameBuffer *buffer) override;\n> > @@ -32,6 +33,14 @@ public:\n> > \tbool processRequest(libcamera::Request *request) override;\n> >\n> > private:\n> > +\tstatic constexpr const char *kDefaultFilePattern = \"frame-#.bin\";\n> > +\n> > +\tenum class FileType {\n> > +\t\tBinary,\n> > +\t\tDng,\n> > +\t\tPpm,\n> > +\t};\n> > +\n> > \tvoid writeBuffer(const libcamera::Stream *stream,\n> > \t\t\t libcamera::FrameBuffer *buffer,\n> > \t\t\t const libcamera::ControlList &metadata);\n> > @@ -39,7 +48,10 @@ private:\n> > #ifdef HAVE_TIFF\n> > \tconst libcamera::Camera *camera_;\n> > #endif\n> > -\tstd::map streamNames_;\n> > +\n> > \tstd::string pattern_;\n> > +\tFileType fileType_;\n> > +\n> > +\tstd::map streamNames_;\n> > \tstd::map> mappedBuffers_;\n> > };","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 53EDCC0F1B\n\tfor ;\n\tThu, 26 Sep 2024 14:25:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 123AD6350F;\n\tThu, 26 Sep 2024 16:25:11 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A66AF634F9\n\tfor ;\n\tThu, 26 Sep 2024 16:25:09 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 336A98D4;\n\tThu, 26 Sep 2024 16:23:41 +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=\"UBgjctHY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727360621;\n\tbh=vZvVy5sCSU4esNLR1Oev0svrPtQ9k6GNvfXs4HUPjFI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UBgjctHYgsN8nz210V7RJ268NLC4sM4KEXk9SjJCJpGNbt+TFI5Q1mYVyoh/F2q+V\n\tMibfMRV3efD+X9xwtxu4/Ky7oji7J7SGTNcOhmHRpdzHYKAlNFw5W/dy5aTp2gFHnB\n\tlZhGNVwfpf8pHoaROLCCLrii3n/ymInsu4Hfp0MM=","Date":"Thu, 26 Sep 2024 17:25:07 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org,\n\tArne Caspari ","Subject":"Re: [PATCH 2/2] apps: cam: Print an error when outputting DNG and\n\tDNG support is missing","Message-ID":"<20240926142507.GH21788@pendragon.ideasonboard.com>","References":"<20240925152134.20284-1-laurent.pinchart@ideasonboard.com>\n\t<20240925152134.20284-3-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]