| Message ID | 20240925152134.20284-3-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | b2538c80b96df937ff6fa823a489bf4710cee0e2 |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, thank you for the patch, nice improvement. Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes: > When DNG support is missing, the cam application ignores the .dng suffix > of the file pattern and writes raw binary data instead, without > notifying the user. This leads to confusion. Fix it by printing an error > message. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Milan Zamazal <mzamazal@redhat.com> > --- > src/apps/cam/camera_session.cpp | 15 ++++++--- > src/apps/cam/file_sink.cpp | 60 +++++++++++++++++++++++---------- > src/apps/cam/file_sink.h | 18 ++++++++-- > 3 files changed, 68 insertions(+), 25 deletions(-) > > diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp > index 097dc479241a..227df9e922ea 100644 > --- a/src/apps/cam/camera_session.cpp > +++ b/src/apps/cam/camera_session.cpp > @@ -230,11 +230,16 @@ int CameraSession::start() > #endif > > if (options_.isSet(OptFile)) { > - if (!options_[OptFile].toString().empty()) > - sink_ = std::make_unique<FileSink>(camera_.get(), streamNames_, > - options_[OptFile]); > - else > - sink_ = std::make_unique<FileSink>(camera_.get(), streamNames_); > + std::unique_ptr<FileSink> sink = > + std::make_unique<FileSink>(camera_.get(), streamNames_); > + > + if (!options_[OptFile].toString().empty()) { > + ret = sink->setFilePattern(options_[OptFile]); > + if (ret) > + return ret; > + } > + > + sink_ = std::move(sink); > } > > if (sink_) { > diff --git a/src/apps/cam/file_sink.cpp b/src/apps/cam/file_sink.cpp > index 3e000d2fd9c6..76e21db9bf9a 100644 > --- a/src/apps/cam/file_sink.cpp > +++ b/src/apps/cam/file_sink.cpp > @@ -5,6 +5,7 @@ > * File Sink > */ > > +#include <array> > #include <assert.h> > #include <fcntl.h> > #include <iomanip> > @@ -12,6 +13,7 @@ > #include <sstream> > #include <string.h> > #include <unistd.h> > +#include <utility> > > #include <libcamera/camera.h> > > @@ -24,13 +26,13 @@ > using namespace libcamera; > > FileSink::FileSink([[maybe_unused]] const libcamera::Camera *camera, > - const std::map<const libcamera::Stream *, std::string> &streamNames, > - const std::string &pattern) > + const std::map<const libcamera::Stream *, std::string> &streamNames) > : > #ifdef HAVE_TIFF > camera_(camera), > #endif > - streamNames_(streamNames), pattern_(pattern) > + pattern_(kDefaultFilePattern), fileType_(FileType::Binary), > + streamNames_(streamNames) > { > } > > @@ -38,6 +40,41 @@ FileSink::~FileSink() > { > } > > +int FileSink::setFilePattern(const std::string &pattern) > +{ > + static const std::array<std::pair<std::string, FileType>, 2> types{{ > + { ".dng", FileType::Dng }, > + { ".ppm", FileType::Ppm }, > + }}; > + > + pattern_ = pattern; > + > + if (pattern_.empty() || pattern_.back() == '/') > + pattern_ += kDefaultFilePattern; > + > + fileType_ = FileType::Binary; > + > + for (const auto &type : types) { > + if (pattern_.size() < type.first.size()) > + continue; > + > + if (pattern_.find(type.first, pattern_.size() - type.first.size()) != > + std::string::npos) { > + fileType_ = type.second; > + break; > + } > + } > + > +#ifndef HAVE_TIFF > + if (fileType_ == FileType::Dng) { > + std::cerr << "DNG support not available" << std::endl; > + return -EINVAL; > + } > +#endif /* HAVE_TIFF */ > + > + return 0; > +} > + > int FileSink::configure(const libcamera::CameraConfiguration &config) > { > int ret = FrameSink::configure(config); > @@ -67,21 +104,10 @@ bool FileSink::processRequest(Request *request) > void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer, > [[maybe_unused]] const ControlList &metadata) > { > - std::string filename; > + std::string filename = pattern_; > size_t pos; > int fd, ret = 0; > > - if (!pattern_.empty()) > - filename = pattern_; > - > -#ifdef HAVE_TIFF > - bool dng = filename.find(".dng", filename.size() - 4) != std::string::npos; > -#endif /* HAVE_TIFF */ > - bool ppm = filename.find(".ppm", filename.size() - 4) != std::string::npos; > - > - if (filename.empty() || filename.back() == '/') > - filename += "frame-#.bin"; > - > pos = filename.find_first_of('#'); > if (pos != std::string::npos) { > std::stringstream ss; > @@ -93,7 +119,7 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer, > Image *image = mappedBuffers_[buffer].get(); > > #ifdef HAVE_TIFF > - if (dng) { > + if (fileType_ == FileType::Dng) { > ret = DNGWriter::write(filename.c_str(), camera_, > stream->configuration(), metadata, > buffer, image->data(0).data()); > @@ -104,7 +130,7 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer, > return; > } > #endif /* HAVE_TIFF */ > - if (ppm) { > + if (fileType_ == FileType::Ppm) { > ret = PPMWriter::write(filename.c_str(), stream->configuration(), > image->data(0)); > if (ret < 0) > diff --git a/src/apps/cam/file_sink.h b/src/apps/cam/file_sink.h > index 9d560783af09..71b7fe0feab5 100644 > --- a/src/apps/cam/file_sink.h > +++ b/src/apps/cam/file_sink.h > @@ -21,10 +21,11 @@ class FileSink : public FrameSink > { > public: > FileSink(const libcamera::Camera *camera, > - const std::map<const libcamera::Stream *, std::string> &streamNames, > - const std::string &pattern = ""); > + const std::map<const libcamera::Stream *, std::string> &streamNames); > ~FileSink(); > > + int setFilePattern(const std::string &pattern); > + > int configure(const libcamera::CameraConfiguration &config) override; > > void mapBuffer(libcamera::FrameBuffer *buffer) override; > @@ -32,6 +33,14 @@ public: > bool processRequest(libcamera::Request *request) override; > > private: > + static constexpr const char *kDefaultFilePattern = "frame-#.bin"; > + > + enum class FileType { > + Binary, > + Dng, > + Ppm, > + }; > + > void writeBuffer(const libcamera::Stream *stream, > libcamera::FrameBuffer *buffer, > const libcamera::ControlList &metadata); > @@ -39,7 +48,10 @@ private: > #ifdef HAVE_TIFF > const libcamera::Camera *camera_; > #endif > - std::map<const libcamera::Stream *, std::string> streamNames_; > + > std::string pattern_; > + FileType fileType_; > + > + std::map<const libcamera::Stream *, std::string> streamNames_; > std::map<libcamera::FrameBuffer *, std::unique_ptr<Image>> mappedBuffers_; > };
Hi Laurent On Wed, Sep 25, 2024 at 06:21:34PM GMT, Laurent Pinchart wrote: > When DNG support is missing, the cam application ignores the .dng suffix > of the file pattern and writes raw binary data instead, without > notifying the user. This leads to confusion. Fix it by printing an error > message. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/apps/cam/camera_session.cpp | 15 ++++++--- > src/apps/cam/file_sink.cpp | 60 +++++++++++++++++++++++---------- > src/apps/cam/file_sink.h | 18 ++++++++-- > 3 files changed, 68 insertions(+), 25 deletions(-) > > diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp > index 097dc479241a..227df9e922ea 100644 > --- a/src/apps/cam/camera_session.cpp > +++ b/src/apps/cam/camera_session.cpp > @@ -230,11 +230,16 @@ int CameraSession::start() > #endif > > if (options_.isSet(OptFile)) { > - if (!options_[OptFile].toString().empty()) > - sink_ = std::make_unique<FileSink>(camera_.get(), streamNames_, > - options_[OptFile]); > - else > - sink_ = std::make_unique<FileSink>(camera_.get(), streamNames_); > + std::unique_ptr<FileSink> sink = > + std::make_unique<FileSink>(camera_.get(), streamNames_); > + > + if (!options_[OptFile].toString().empty()) { > + ret = sink->setFilePattern(options_[OptFile]); > + if (ret) > + return ret; > + } > + > + sink_ = std::move(sink); > } > > if (sink_) { > diff --git a/src/apps/cam/file_sink.cpp b/src/apps/cam/file_sink.cpp > index 3e000d2fd9c6..76e21db9bf9a 100644 > --- a/src/apps/cam/file_sink.cpp > +++ b/src/apps/cam/file_sink.cpp > @@ -5,6 +5,7 @@ > * File Sink > */ > > +#include <array> > #include <assert.h> > #include <fcntl.h> > #include <iomanip> > @@ -12,6 +13,7 @@ > #include <sstream> > #include <string.h> > #include <unistd.h> > +#include <utility> > > #include <libcamera/camera.h> > > @@ -24,13 +26,13 @@ > using namespace libcamera; > > FileSink::FileSink([[maybe_unused]] const libcamera::Camera *camera, > - const std::map<const libcamera::Stream *, std::string> &streamNames, > - const std::string &pattern) > + const std::map<const libcamera::Stream *, std::string> &streamNames) > : > #ifdef HAVE_TIFF > camera_(camera), > #endif I wonder if we actually need this or we could initialize the camera_ member unconditionally (and drop the above [[maybe_unused]]) > - streamNames_(streamNames), pattern_(pattern) > + pattern_(kDefaultFilePattern), fileType_(FileType::Binary), > + streamNames_(streamNames) > { > } > > @@ -38,6 +40,41 @@ FileSink::~FileSink() > { > } > > +int FileSink::setFilePattern(const std::string &pattern) > +{ > + static const std::array<std::pair<std::string, FileType>, 2> types{{ > + { ".dng", FileType::Dng }, > + { ".ppm", FileType::Ppm }, > + }}; > + > + pattern_ = pattern; > + > + if (pattern_.empty() || pattern_.back() == '/') > + pattern_ += kDefaultFilePattern; > + > + fileType_ = FileType::Binary; > + > + for (const auto &type : types) { > + if (pattern_.size() < type.first.size()) > + continue; > + > + if (pattern_.find(type.first, pattern_.size() - type.first.size()) != Or if (pattern_.rfind(type.first) != std::string::npos) The rest looks good to me Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Thanks j > + std::string::npos) { > + fileType_ = type.second; > + break; > + } > + } > + > +#ifndef HAVE_TIFF > + if (fileType_ == FileType::Dng) { > + std::cerr << "DNG support not available" << std::endl; > + return -EINVAL; > + } > +#endif /* HAVE_TIFF */ > + > + return 0; > +} > + > int FileSink::configure(const libcamera::CameraConfiguration &config) > { > int ret = FrameSink::configure(config); > @@ -67,21 +104,10 @@ bool FileSink::processRequest(Request *request) > void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer, > [[maybe_unused]] const ControlList &metadata) > { > - std::string filename; > + std::string filename = pattern_; > size_t pos; > int fd, ret = 0; > > - if (!pattern_.empty()) > - filename = pattern_; > - > -#ifdef HAVE_TIFF > - bool dng = filename.find(".dng", filename.size() - 4) != std::string::npos; > -#endif /* HAVE_TIFF */ > - bool ppm = filename.find(".ppm", filename.size() - 4) != std::string::npos; > - > - if (filename.empty() || filename.back() == '/') > - filename += "frame-#.bin"; > - > pos = filename.find_first_of('#'); > if (pos != std::string::npos) { > std::stringstream ss; > @@ -93,7 +119,7 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer, > Image *image = mappedBuffers_[buffer].get(); > > #ifdef HAVE_TIFF > - if (dng) { > + if (fileType_ == FileType::Dng) { > ret = DNGWriter::write(filename.c_str(), camera_, > stream->configuration(), metadata, > buffer, image->data(0).data()); > @@ -104,7 +130,7 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer, > return; > } > #endif /* HAVE_TIFF */ > - if (ppm) { > + if (fileType_ == FileType::Ppm) { > ret = PPMWriter::write(filename.c_str(), stream->configuration(), > image->data(0)); > if (ret < 0) > diff --git a/src/apps/cam/file_sink.h b/src/apps/cam/file_sink.h > index 9d560783af09..71b7fe0feab5 100644 > --- a/src/apps/cam/file_sink.h > +++ b/src/apps/cam/file_sink.h > @@ -21,10 +21,11 @@ class FileSink : public FrameSink > { > public: > FileSink(const libcamera::Camera *camera, > - const std::map<const libcamera::Stream *, std::string> &streamNames, > - const std::string &pattern = ""); > + const std::map<const libcamera::Stream *, std::string> &streamNames); > ~FileSink(); > > + int setFilePattern(const std::string &pattern); > + > int configure(const libcamera::CameraConfiguration &config) override; > > void mapBuffer(libcamera::FrameBuffer *buffer) override; > @@ -32,6 +33,14 @@ public: > bool processRequest(libcamera::Request *request) override; > > private: > + static constexpr const char *kDefaultFilePattern = "frame-#.bin"; > + > + enum class FileType { > + Binary, > + Dng, > + Ppm, > + }; > + > void writeBuffer(const libcamera::Stream *stream, > libcamera::FrameBuffer *buffer, > const libcamera::ControlList &metadata); > @@ -39,7 +48,10 @@ private: > #ifdef HAVE_TIFF > const libcamera::Camera *camera_; > #endif > - std::map<const libcamera::Stream *, std::string> streamNames_; > + > std::string pattern_; > + FileType fileType_; > + > + std::map<const libcamera::Stream *, std::string> streamNames_; > std::map<libcamera::FrameBuffer *, std::unique_ptr<Image>> mappedBuffers_; > }; > -- > Regards, > > Laurent Pinchart >
On Thu, Sep 26, 2024 at 03:59:42PM +0200, Jacopo Mondi wrote: > Hi Laurent > > On Wed, Sep 25, 2024 at 06:21:34PM GMT, Laurent Pinchart wrote: > > When DNG support is missing, the cam application ignores the .dng suffix > > of the file pattern and writes raw binary data instead, without > > notifying the user. This leads to confusion. Fix it by printing an error > > message. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > src/apps/cam/camera_session.cpp | 15 ++++++--- > > src/apps/cam/file_sink.cpp | 60 +++++++++++++++++++++++---------- > > src/apps/cam/file_sink.h | 18 ++++++++-- > > 3 files changed, 68 insertions(+), 25 deletions(-) > > > > diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp > > index 097dc479241a..227df9e922ea 100644 > > --- a/src/apps/cam/camera_session.cpp > > +++ b/src/apps/cam/camera_session.cpp > > @@ -230,11 +230,16 @@ int CameraSession::start() > > #endif > > > > if (options_.isSet(OptFile)) { > > - if (!options_[OptFile].toString().empty()) > > - sink_ = std::make_unique<FileSink>(camera_.get(), streamNames_, > > - options_[OptFile]); > > - else > > - sink_ = std::make_unique<FileSink>(camera_.get(), streamNames_); > > + std::unique_ptr<FileSink> sink = > > + std::make_unique<FileSink>(camera_.get(), streamNames_); > > + > > + if (!options_[OptFile].toString().empty()) { > > + ret = sink->setFilePattern(options_[OptFile]); > > + if (ret) > > + return ret; > > + } > > + > > + sink_ = std::move(sink); > > } > > > > if (sink_) { > > diff --git a/src/apps/cam/file_sink.cpp b/src/apps/cam/file_sink.cpp > > index 3e000d2fd9c6..76e21db9bf9a 100644 > > --- a/src/apps/cam/file_sink.cpp > > +++ b/src/apps/cam/file_sink.cpp > > @@ -5,6 +5,7 @@ > > * File Sink > > */ > > > > +#include <array> > > #include <assert.h> > > #include <fcntl.h> > > #include <iomanip> > > @@ -12,6 +13,7 @@ > > #include <sstream> > > #include <string.h> > > #include <unistd.h> > > +#include <utility> > > > > #include <libcamera/camera.h> > > > > @@ -24,13 +26,13 @@ > > using namespace libcamera; > > > > FileSink::FileSink([[maybe_unused]] const libcamera::Camera *camera, > > - const std::map<const libcamera::Stream *, std::string> &streamNames, > > - const std::string &pattern) > > + const std::map<const libcamera::Stream *, std::string> &streamNames) > > : > > #ifdef HAVE_TIFF > > camera_(camera), > > #endif > > I wonder if we actually need this or we could initialize the camera_ > member unconditionally (and drop the above [[maybe_unused]]) I thought about that too but decided not to change it as it's a separate issue. > > - streamNames_(streamNames), pattern_(pattern) > > + pattern_(kDefaultFilePattern), fileType_(FileType::Binary), > > + streamNames_(streamNames) > > { > > } > > > > @@ -38,6 +40,41 @@ FileSink::~FileSink() > > { > > } > > > > +int FileSink::setFilePattern(const std::string &pattern) > > +{ > > + static const std::array<std::pair<std::string, FileType>, 2> types{{ > > + { ".dng", FileType::Dng }, > > + { ".ppm", FileType::Ppm }, > > + }}; > > + > > + pattern_ = pattern; > > + > > + if (pattern_.empty() || pattern_.back() == '/') > > + pattern_ += kDefaultFilePattern; > > + > > + fileType_ = FileType::Binary; > > + > > + for (const auto &type : types) { > > + if (pattern_.size() < type.first.size()) > > + continue; > > + > > + if (pattern_.find(type.first, pattern_.size() - type.first.size()) != > > Or > if (pattern_.rfind(type.first) != std::string::npos) We want to check if pattern_ ends with type.first, not if type.first is found somewhere inside pattern_. C++20 has a nice std::string::ends_with() (https://en.cppreference.com/w/cpp/string/basic_string/ends_with), but C++17 doesn't :-( > The rest looks good to me > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > + std::string::npos) { > > + fileType_ = type.second; > > + break; > > + } > > + } > > + > > +#ifndef HAVE_TIFF > > + if (fileType_ == FileType::Dng) { > > + std::cerr << "DNG support not available" << std::endl; > > + return -EINVAL; > > + } > > +#endif /* HAVE_TIFF */ > > + > > + return 0; > > +} > > + > > int FileSink::configure(const libcamera::CameraConfiguration &config) > > { > > int ret = FrameSink::configure(config); > > @@ -67,21 +104,10 @@ bool FileSink::processRequest(Request *request) > > void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer, > > [[maybe_unused]] const ControlList &metadata) > > { > > - std::string filename; > > + std::string filename = pattern_; > > size_t pos; > > int fd, ret = 0; > > > > - if (!pattern_.empty()) > > - filename = pattern_; > > - > > -#ifdef HAVE_TIFF > > - bool dng = filename.find(".dng", filename.size() - 4) != std::string::npos; > > -#endif /* HAVE_TIFF */ > > - bool ppm = filename.find(".ppm", filename.size() - 4) != std::string::npos; > > - > > - if (filename.empty() || filename.back() == '/') > > - filename += "frame-#.bin"; > > - > > pos = filename.find_first_of('#'); > > if (pos != std::string::npos) { > > std::stringstream ss; > > @@ -93,7 +119,7 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer, > > Image *image = mappedBuffers_[buffer].get(); > > > > #ifdef HAVE_TIFF > > - if (dng) { > > + if (fileType_ == FileType::Dng) { > > ret = DNGWriter::write(filename.c_str(), camera_, > > stream->configuration(), metadata, > > buffer, image->data(0).data()); > > @@ -104,7 +130,7 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer, > > return; > > } > > #endif /* HAVE_TIFF */ > > - if (ppm) { > > + if (fileType_ == FileType::Ppm) { > > ret = PPMWriter::write(filename.c_str(), stream->configuration(), > > image->data(0)); > > if (ret < 0) > > diff --git a/src/apps/cam/file_sink.h b/src/apps/cam/file_sink.h > > index 9d560783af09..71b7fe0feab5 100644 > > --- a/src/apps/cam/file_sink.h > > +++ b/src/apps/cam/file_sink.h > > @@ -21,10 +21,11 @@ class FileSink : public FrameSink > > { > > public: > > FileSink(const libcamera::Camera *camera, > > - const std::map<const libcamera::Stream *, std::string> &streamNames, > > - const std::string &pattern = ""); > > + const std::map<const libcamera::Stream *, std::string> &streamNames); > > ~FileSink(); > > > > + int setFilePattern(const std::string &pattern); > > + > > int configure(const libcamera::CameraConfiguration &config) override; > > > > void mapBuffer(libcamera::FrameBuffer *buffer) override; > > @@ -32,6 +33,14 @@ public: > > bool processRequest(libcamera::Request *request) override; > > > > private: > > + static constexpr const char *kDefaultFilePattern = "frame-#.bin"; > > + > > + enum class FileType { > > + Binary, > > + Dng, > > + Ppm, > > + }; > > + > > void writeBuffer(const libcamera::Stream *stream, > > libcamera::FrameBuffer *buffer, > > const libcamera::ControlList &metadata); > > @@ -39,7 +48,10 @@ private: > > #ifdef HAVE_TIFF > > const libcamera::Camera *camera_; > > #endif > > - std::map<const libcamera::Stream *, std::string> streamNames_; > > + > > std::string pattern_; > > + FileType fileType_; > > + > > + std::map<const libcamera::Stream *, std::string> streamNames_; > > std::map<libcamera::FrameBuffer *, std::unique_ptr<Image>> mappedBuffers_; > > };
diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp index 097dc479241a..227df9e922ea 100644 --- a/src/apps/cam/camera_session.cpp +++ b/src/apps/cam/camera_session.cpp @@ -230,11 +230,16 @@ int CameraSession::start() #endif if (options_.isSet(OptFile)) { - if (!options_[OptFile].toString().empty()) - sink_ = std::make_unique<FileSink>(camera_.get(), streamNames_, - options_[OptFile]); - else - sink_ = std::make_unique<FileSink>(camera_.get(), streamNames_); + std::unique_ptr<FileSink> sink = + std::make_unique<FileSink>(camera_.get(), streamNames_); + + if (!options_[OptFile].toString().empty()) { + ret = sink->setFilePattern(options_[OptFile]); + if (ret) + return ret; + } + + sink_ = std::move(sink); } if (sink_) { diff --git a/src/apps/cam/file_sink.cpp b/src/apps/cam/file_sink.cpp index 3e000d2fd9c6..76e21db9bf9a 100644 --- a/src/apps/cam/file_sink.cpp +++ b/src/apps/cam/file_sink.cpp @@ -5,6 +5,7 @@ * File Sink */ +#include <array> #include <assert.h> #include <fcntl.h> #include <iomanip> @@ -12,6 +13,7 @@ #include <sstream> #include <string.h> #include <unistd.h> +#include <utility> #include <libcamera/camera.h> @@ -24,13 +26,13 @@ using namespace libcamera; FileSink::FileSink([[maybe_unused]] const libcamera::Camera *camera, - const std::map<const libcamera::Stream *, std::string> &streamNames, - const std::string &pattern) + const std::map<const libcamera::Stream *, std::string> &streamNames) : #ifdef HAVE_TIFF camera_(camera), #endif - streamNames_(streamNames), pattern_(pattern) + pattern_(kDefaultFilePattern), fileType_(FileType::Binary), + streamNames_(streamNames) { } @@ -38,6 +40,41 @@ FileSink::~FileSink() { } +int FileSink::setFilePattern(const std::string &pattern) +{ + static const std::array<std::pair<std::string, FileType>, 2> types{{ + { ".dng", FileType::Dng }, + { ".ppm", FileType::Ppm }, + }}; + + pattern_ = pattern; + + if (pattern_.empty() || pattern_.back() == '/') + pattern_ += kDefaultFilePattern; + + fileType_ = FileType::Binary; + + for (const auto &type : types) { + if (pattern_.size() < type.first.size()) + continue; + + if (pattern_.find(type.first, pattern_.size() - type.first.size()) != + std::string::npos) { + fileType_ = type.second; + break; + } + } + +#ifndef HAVE_TIFF + if (fileType_ == FileType::Dng) { + std::cerr << "DNG support not available" << std::endl; + return -EINVAL; + } +#endif /* HAVE_TIFF */ + + return 0; +} + int FileSink::configure(const libcamera::CameraConfiguration &config) { int ret = FrameSink::configure(config); @@ -67,21 +104,10 @@ bool FileSink::processRequest(Request *request) void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer, [[maybe_unused]] const ControlList &metadata) { - std::string filename; + std::string filename = pattern_; size_t pos; int fd, ret = 0; - if (!pattern_.empty()) - filename = pattern_; - -#ifdef HAVE_TIFF - bool dng = filename.find(".dng", filename.size() - 4) != std::string::npos; -#endif /* HAVE_TIFF */ - bool ppm = filename.find(".ppm", filename.size() - 4) != std::string::npos; - - if (filename.empty() || filename.back() == '/') - filename += "frame-#.bin"; - pos = filename.find_first_of('#'); if (pos != std::string::npos) { std::stringstream ss; @@ -93,7 +119,7 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer, Image *image = mappedBuffers_[buffer].get(); #ifdef HAVE_TIFF - if (dng) { + if (fileType_ == FileType::Dng) { ret = DNGWriter::write(filename.c_str(), camera_, stream->configuration(), metadata, buffer, image->data(0).data()); @@ -104,7 +130,7 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer, return; } #endif /* HAVE_TIFF */ - if (ppm) { + if (fileType_ == FileType::Ppm) { ret = PPMWriter::write(filename.c_str(), stream->configuration(), image->data(0)); if (ret < 0) diff --git a/src/apps/cam/file_sink.h b/src/apps/cam/file_sink.h index 9d560783af09..71b7fe0feab5 100644 --- a/src/apps/cam/file_sink.h +++ b/src/apps/cam/file_sink.h @@ -21,10 +21,11 @@ class FileSink : public FrameSink { public: FileSink(const libcamera::Camera *camera, - const std::map<const libcamera::Stream *, std::string> &streamNames, - const std::string &pattern = ""); + const std::map<const libcamera::Stream *, std::string> &streamNames); ~FileSink(); + int setFilePattern(const std::string &pattern); + int configure(const libcamera::CameraConfiguration &config) override; void mapBuffer(libcamera::FrameBuffer *buffer) override; @@ -32,6 +33,14 @@ public: bool processRequest(libcamera::Request *request) override; private: + static constexpr const char *kDefaultFilePattern = "frame-#.bin"; + + enum class FileType { + Binary, + Dng, + Ppm, + }; + void writeBuffer(const libcamera::Stream *stream, libcamera::FrameBuffer *buffer, const libcamera::ControlList &metadata); @@ -39,7 +48,10 @@ private: #ifdef HAVE_TIFF const libcamera::Camera *camera_; #endif - std::map<const libcamera::Stream *, std::string> streamNames_; + std::string pattern_; + FileType fileType_; + + std::map<const libcamera::Stream *, std::string> streamNames_; std::map<libcamera::FrameBuffer *, std::unique_ptr<Image>> mappedBuffers_; };
When DNG support is missing, the cam application ignores the .dng suffix of the file pattern and writes raw binary data instead, without notifying the user. This leads to confusion. Fix it by printing an error message. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/apps/cam/camera_session.cpp | 15 ++++++--- src/apps/cam/file_sink.cpp | 60 +++++++++++++++++++++++---------- src/apps/cam/file_sink.h | 18 ++++++++-- 3 files changed, 68 insertions(+), 25 deletions(-)