| Message ID | 20221018080908.2841339-3-paul.elder@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Paul, Thank you for the patch. On Tue, Oct 18, 2022 at 05:09:07PM +0900, Paul Elder via libcamera-devel wrote: > Add support for outputting buffers in DNG format. It reuses the DNG > writer that we had previously in qcam. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > Changes in v2: > - update help test to mention the dng output feature > - pass camera as first argument to FileSink constructor > --- > src/cam/camera_session.cpp | 4 ++-- > src/cam/file_sink.cpp | 32 ++++++++++++++++++++++++++------ > src/cam/file_sink.h | 7 +++++-- > src/cam/main.cpp | 2 ++ > src/cam/meson.build | 8 ++++++++ > 5 files changed, 43 insertions(+), 10 deletions(-) > > diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp > index 238186a3..6b409c98 100644 > --- a/src/cam/camera_session.cpp > +++ b/src/cam/camera_session.cpp > @@ -207,10 +207,10 @@ int CameraSession::start() > > if (options_.isSet(OptFile)) { > if (!options_[OptFile].toString().empty()) > - sink_ = std::make_unique<FileSink>(streamNames_, > + sink_ = std::make_unique<FileSink>(camera_.get(), streamNames_, > options_[OptFile]); > else > - sink_ = std::make_unique<FileSink>(streamNames_); > + sink_ = std::make_unique<FileSink>(camera_.get(), streamNames_); > } > > if (sink_) { > diff --git a/src/cam/file_sink.cpp b/src/cam/file_sink.cpp > index 45213d4a..8bba4e7f 100644 > --- a/src/cam/file_sink.cpp > +++ b/src/cam/file_sink.cpp > @@ -15,14 +15,16 @@ > > #include <libcamera/camera.h> > > +#include "dng_writer.h" > #include "file_sink.h" > #include "image.h" > > using namespace libcamera; > > -FileSink::FileSink(const std::map<const libcamera::Stream *, std::string> &streamNames, > +FileSink::FileSink(const libcamera::Camera *camera, > + const std::map<const libcamera::Stream *, std::string> &streamNames, > const std::string &pattern) > - : streamNames_(streamNames), pattern_(pattern) > + : camera_(camera), streamNames_(streamNames), pattern_(pattern) > { > } > > @@ -51,12 +53,13 @@ void FileSink::mapBuffer(FrameBuffer *buffer) > bool FileSink::processRequest(Request *request) > { > for (auto [stream, buffer] : request->buffers()) > - writeBuffer(stream, buffer); > + writeBuffer(stream, buffer, request->metadata()); > > return true; > } > > -void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) > +void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer, > + [[maybe_unused]] const ControlList &metadata) > { > std::string filename; > size_t pos; > @@ -65,6 +68,10 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) > if (!pattern_.empty()) > filename = pattern_; > > +#ifdef HAVE_TIFF > + bool dng = filename.find(".dng", filename.size() - 4) != std::string::npos; > +#endif /* HAVE_TIFF */ > + > if (filename.empty() || filename.back() == '/') > filename += "frame-#.bin"; > > @@ -76,6 +83,21 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) > filename.replace(pos, 1, ss.str()); > } > > + Image *image = mappedBuffers_[buffer].get(); > + > +#ifdef HAVE_TIFF > + if (dng) { > + ret = DNGWriter::write(filename.c_str(), camera_, > + stream->configuration(), metadata, > + buffer, image->data(0).data()); > + if (ret < 0) > + std::cerr << "failed to write DNG " << filename > + << std::endl; It's nice to quote file names to avoid ambiguities: std::cerr << "failed to write DNG file `" << filename << "'" << std::endl; Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + > + return; > + } > +#endif /* HAVE_TIFF */ > + > fd = open(filename.c_str(), O_CREAT | O_WRONLY | > (pos == std::string::npos ? O_APPEND : O_TRUNC), > S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH); > @@ -86,8 +108,6 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) > return; > } > > - Image *image = mappedBuffers_[buffer].get(); > - > for (unsigned int i = 0; i < buffer->planes().size(); ++i) { > const FrameMetadata::Plane &meta = buffer->metadata().planes()[i]; > > diff --git a/src/cam/file_sink.h b/src/cam/file_sink.h > index 067736f5..9ce8b619 100644 > --- a/src/cam/file_sink.h > +++ b/src/cam/file_sink.h > @@ -20,7 +20,8 @@ class Image; > class FileSink : public FrameSink > { > public: > - FileSink(const std::map<const libcamera::Stream *, std::string> &streamNames, > + FileSink(const libcamera::Camera *camera, > + const std::map<const libcamera::Stream *, std::string> &streamNames, > const std::string &pattern = ""); > ~FileSink(); > > @@ -32,8 +33,10 @@ public: > > private: > void writeBuffer(const libcamera::Stream *stream, > - libcamera::FrameBuffer *buffer); > + libcamera::FrameBuffer *buffer, > + const libcamera::ControlList &metadata); > > + const libcamera::Camera *camera_; > std::map<const libcamera::Stream *, std::string> streamNames_; > std::string pattern_; > std::map<libcamera::FrameBuffer *, std::unique_ptr<Image>> mappedBuffers_; > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > index 53c2ffde..c4e18a13 100644 > --- a/src/cam/main.cpp > +++ b/src/cam/main.cpp > @@ -144,6 +144,8 @@ int CamApp::parseOptions(int argc, char *argv[]) > "to write files, using the default file name. Otherwise it sets the\n" > "full file path and name. The first '#' character in the file name\n" > "is expanded to the camera index, stream name and frame sequence number.\n" > + "If the file name ends with '.dng', then the frame will be written to\n" > + "the output file(s) in DNG format.\n" > "The default file name is 'frame-#.bin'.", > "file", ArgumentOptional, "filename", false, > OptCamera); > diff --git a/src/cam/meson.build b/src/cam/meson.build > index 9c766221..06dbea06 100644 > --- a/src/cam/meson.build > +++ b/src/cam/meson.build > @@ -52,6 +52,13 @@ if libsdl2.found() > endif > endif > > +if libtiff.found() > + cam_cpp_args += ['-DHAVE_TIFF'] > + cam_sources += files([ > + 'dng_writer.cpp', > + ]) > +endif > + > cam = executable('cam', cam_sources, > dependencies : [ > libatomic, > @@ -60,6 +67,7 @@ cam = executable('cam', cam_sources, > libevent, > libjpeg, > libsdl2, > + libtiff, > libyaml, > ], > cpp_args : cam_cpp_args,
Hi Paul On Tue, Oct 18, 2022 at 05:09:07PM +0900, Paul Elder via libcamera-devel wrote: > Add support for outputting buffers in DNG format. It reuses the DNG > writer that we had previously in qcam. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > Changes in v2: > - update help test to mention the dng output feature > - pass camera as first argument to FileSink constructor > --- > src/cam/camera_session.cpp | 4 ++-- > src/cam/file_sink.cpp | 32 ++++++++++++++++++++++++++------ > src/cam/file_sink.h | 7 +++++-- > src/cam/main.cpp | 2 ++ > src/cam/meson.build | 8 ++++++++ > 5 files changed, 43 insertions(+), 10 deletions(-) > > diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp > index 238186a3..6b409c98 100644 > --- a/src/cam/camera_session.cpp > +++ b/src/cam/camera_session.cpp > @@ -207,10 +207,10 @@ int CameraSession::start() > > if (options_.isSet(OptFile)) { > if (!options_[OptFile].toString().empty()) > - sink_ = std::make_unique<FileSink>(streamNames_, > + sink_ = std::make_unique<FileSink>(camera_.get(), streamNames_, > options_[OptFile]); > else > - sink_ = std::make_unique<FileSink>(streamNames_); > + sink_ = std::make_unique<FileSink>(camera_.get(), streamNames_); > } > > if (sink_) { > diff --git a/src/cam/file_sink.cpp b/src/cam/file_sink.cpp > index 45213d4a..8bba4e7f 100644 > --- a/src/cam/file_sink.cpp > +++ b/src/cam/file_sink.cpp > @@ -15,14 +15,16 @@ > > #include <libcamera/camera.h> > > +#include "dng_writer.h" > #include "file_sink.h" > #include "image.h" > > using namespace libcamera; > > -FileSink::FileSink(const std::map<const libcamera::Stream *, std::string> &streamNames, > +FileSink::FileSink(const libcamera::Camera *camera, > + const std::map<const libcamera::Stream *, std::string> &streamNames, > const std::string &pattern) > - : streamNames_(streamNames), pattern_(pattern) > + : camera_(camera), streamNames_(streamNames), pattern_(pattern) > { > } > > @@ -51,12 +53,13 @@ void FileSink::mapBuffer(FrameBuffer *buffer) > bool FileSink::processRequest(Request *request) > { > for (auto [stream, buffer] : request->buffers()) > - writeBuffer(stream, buffer); > + writeBuffer(stream, buffer, request->metadata()); > > return true; > } > > -void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) > +void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer, > + [[maybe_unused]] const ControlList &metadata) > { > std::string filename; bool dng = false; > size_t pos; > @@ -65,6 +68,10 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) > if (!pattern_.empty()) > filename = pattern_; > > +#ifdef HAVE_TIFF > + bool dng = filename.find(".dng", filename.size() - 4) != std::string::npos; dng = filename.find(".dng", filename.size() - 4) != std::string::npos; > +#endif /* HAVE_TIFF */ > + > if (filename.empty() || filename.back() == '/') > filename += "frame-#.bin"; > > @@ -76,6 +83,21 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) > filename.replace(pos, 1, ss.str()); > } > > + Image *image = mappedBuffers_[buffer].get(); > + > +#ifdef HAVE_TIFF And you can here remove the #ifdef and the [[maybe_unused]] attribute from the metadata argument > + if (dng) { > + ret = DNGWriter::write(filename.c_str(), camera_, > + stream->configuration(), metadata, > + buffer, image->data(0).data()); > + if (ret < 0) > + std::cerr << "failed to write DNG " << filename > + << std::endl; > + > + return; > + } > +#endif /* HAVE_TIFF */ > + > fd = open(filename.c_str(), O_CREAT | O_WRONLY | > (pos == std::string::npos ? O_APPEND : O_TRUNC), > S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH); > @@ -86,8 +108,6 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) > return; > } > > - Image *image = mappedBuffers_[buffer].get(); > - > for (unsigned int i = 0; i < buffer->planes().size(); ++i) { > const FrameMetadata::Plane &meta = buffer->metadata().planes()[i]; > > diff --git a/src/cam/file_sink.h b/src/cam/file_sink.h > index 067736f5..9ce8b619 100644 > --- a/src/cam/file_sink.h > +++ b/src/cam/file_sink.h > @@ -20,7 +20,8 @@ class Image; > class FileSink : public FrameSink > { > public: > - FileSink(const std::map<const libcamera::Stream *, std::string> &streamNames, > + FileSink(const libcamera::Camera *camera, > + const std::map<const libcamera::Stream *, std::string> &streamNames, > const std::string &pattern = ""); > ~FileSink(); > > @@ -32,8 +33,10 @@ public: > > private: > void writeBuffer(const libcamera::Stream *stream, If only we could get the Camera * back from the request, you could avoid storing camera_ as class member as you could do bool FileSink::processRequest(Request *request) { for (auto [stream, buffer] : request->buffers()) writeBuffer(request->camera(), stream, buffer, request->metadata()); return true; } I recall we discussed that in the past and if it's not there there might be reasons (likely the fact that a Request can outlive the Camera it has been created ?) > - libcamera::FrameBuffer *buffer); > + libcamera::FrameBuffer *buffer, > + const libcamera::ControlList &metadata); > > + const libcamera::Camera *camera_; > std::map<const libcamera::Stream *, std::string> streamNames_; > std::string pattern_; > std::map<libcamera::FrameBuffer *, std::unique_ptr<Image>> mappedBuffers_; > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > index 53c2ffde..c4e18a13 100644 > --- a/src/cam/main.cpp > +++ b/src/cam/main.cpp > @@ -144,6 +144,8 @@ int CamApp::parseOptions(int argc, char *argv[]) > "to write files, using the default file name. Otherwise it sets the\n" > "full file path and name. The first '#' character in the file name\n" > "is expanded to the camera index, stream name and frame sequence number.\n" > + "If the file name ends with '.dng', then the frame will be written to\n" > + "the output file(s) in DNG format.\n" > "The default file name is 'frame-#.bin'.", > "file", ArgumentOptional, "filename", false, > OptCamera); > diff --git a/src/cam/meson.build b/src/cam/meson.build > index 9c766221..06dbea06 100644 > --- a/src/cam/meson.build > +++ b/src/cam/meson.build > @@ -52,6 +52,13 @@ if libsdl2.found() > endif > endif > > +if libtiff.found() > + cam_cpp_args += ['-DHAVE_TIFF'] > + cam_sources += files([ > + 'dng_writer.cpp', > + ]) > +endif > + > cam = executable('cam', cam_sources, > dependencies : [ > libatomic, > @@ -60,6 +67,7 @@ cam = executable('cam', cam_sources, > libevent, > libjpeg, > libsdl2, > + libtiff, > libyaml, > ], > cpp_args : cam_cpp_args, > -- > 2.30.2 >
On Wed, Oct 19, 2022 at 10:34:32AM +0200, Jacopo Mondi via libcamera-devel wrote: > On Tue, Oct 18, 2022 at 05:09:07PM +0900, Paul Elder via libcamera-devel wrote: > > Add support for outputting buffers in DNG format. It reuses the DNG > > writer that we had previously in qcam. > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > > > --- > > Changes in v2: > > - update help test to mention the dng output feature > > - pass camera as first argument to FileSink constructor > > --- > > src/cam/camera_session.cpp | 4 ++-- > > src/cam/file_sink.cpp | 32 ++++++++++++++++++++++++++------ > > src/cam/file_sink.h | 7 +++++-- > > src/cam/main.cpp | 2 ++ > > src/cam/meson.build | 8 ++++++++ > > 5 files changed, 43 insertions(+), 10 deletions(-) > > > > diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp > > index 238186a3..6b409c98 100644 > > --- a/src/cam/camera_session.cpp > > +++ b/src/cam/camera_session.cpp > > @@ -207,10 +207,10 @@ int CameraSession::start() > > > > if (options_.isSet(OptFile)) { > > if (!options_[OptFile].toString().empty()) > > - sink_ = std::make_unique<FileSink>(streamNames_, > > + sink_ = std::make_unique<FileSink>(camera_.get(), streamNames_, > > options_[OptFile]); > > else > > - sink_ = std::make_unique<FileSink>(streamNames_); > > + sink_ = std::make_unique<FileSink>(camera_.get(), streamNames_); > > } > > > > if (sink_) { > > diff --git a/src/cam/file_sink.cpp b/src/cam/file_sink.cpp > > index 45213d4a..8bba4e7f 100644 > > --- a/src/cam/file_sink.cpp > > +++ b/src/cam/file_sink.cpp > > @@ -15,14 +15,16 @@ > > > > #include <libcamera/camera.h> > > > > +#include "dng_writer.h" > > #include "file_sink.h" > > #include "image.h" > > > > using namespace libcamera; > > > > -FileSink::FileSink(const std::map<const libcamera::Stream *, std::string> &streamNames, > > +FileSink::FileSink(const libcamera::Camera *camera, > > + const std::map<const libcamera::Stream *, std::string> &streamNames, > > const std::string &pattern) > > - : streamNames_(streamNames), pattern_(pattern) > > + : camera_(camera), streamNames_(streamNames), pattern_(pattern) > > { > > } > > > > @@ -51,12 +53,13 @@ void FileSink::mapBuffer(FrameBuffer *buffer) > > bool FileSink::processRequest(Request *request) > > { > > for (auto [stream, buffer] : request->buffers()) > > - writeBuffer(stream, buffer); > > + writeBuffer(stream, buffer, request->metadata()); > > > > return true; > > } > > > > -void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) > > +void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer, > > + [[maybe_unused]] const ControlList &metadata) > > { > > std::string filename; > > bool dng = false; > > > size_t pos; > > @@ -65,6 +68,10 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) > > if (!pattern_.empty()) > > filename = pattern_; > > > > +#ifdef HAVE_TIFF > > + bool dng = filename.find(".dng", filename.size() - 4) != std::string::npos; > > dng = filename.find(".dng", filename.size() - 4) != std::string::npos; > > > +#endif /* HAVE_TIFF */ > > + > > if (filename.empty() || filename.back() == '/') > > filename += "frame-#.bin"; > > > > @@ -76,6 +83,21 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) > > filename.replace(pos, 1, ss.str()); > > } > > > > + Image *image = mappedBuffers_[buffer].get(); > > + > > +#ifdef HAVE_TIFF > > And you can here remove the #ifdef and the [[maybe_unused]] attribute from the > metadata argument When !HAVE_TIFF, the DNGWriter class isn't declared by dng_writer.h. The code below would thus not compile. > > + if (dng) { > > + ret = DNGWriter::write(filename.c_str(), camera_, > > + stream->configuration(), metadata, > > + buffer, image->data(0).data()); > > + if (ret < 0) > > + std::cerr << "failed to write DNG " << filename > > + << std::endl; > > + > > + return; > > + } > > +#endif /* HAVE_TIFF */ > > + > > fd = open(filename.c_str(), O_CREAT | O_WRONLY | > > (pos == std::string::npos ? O_APPEND : O_TRUNC), > > S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH); > > @@ -86,8 +108,6 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) > > return; > > } > > > > - Image *image = mappedBuffers_[buffer].get(); > > - > > for (unsigned int i = 0; i < buffer->planes().size(); ++i) { > > const FrameMetadata::Plane &meta = buffer->metadata().planes()[i]; > > > > diff --git a/src/cam/file_sink.h b/src/cam/file_sink.h > > index 067736f5..9ce8b619 100644 > > --- a/src/cam/file_sink.h > > +++ b/src/cam/file_sink.h > > @@ -20,7 +20,8 @@ class Image; > > class FileSink : public FrameSink > > { > > public: > > - FileSink(const std::map<const libcamera::Stream *, std::string> &streamNames, > > + FileSink(const libcamera::Camera *camera, > > + const std::map<const libcamera::Stream *, std::string> &streamNames, > > const std::string &pattern = ""); > > ~FileSink(); > > > > @@ -32,8 +33,10 @@ public: > > > > private: > > void writeBuffer(const libcamera::Stream *stream, > > If only we could get the Camera * back from the request, you could > avoid storing camera_ as class member as you could do > > bool FileSink::processRequest(Request *request) > { > for (auto [stream, buffer] : request->buffers()) > writeBuffer(request->camera(), stream, buffer, request->metadata()); > > return true; > } > > I recall we discussed that in the past and if it's not there there > might be reasons (likely the fact that a Request can outlive the > Camera it has been created ?) > > > - libcamera::FrameBuffer *buffer); > > + libcamera::FrameBuffer *buffer, > > + const libcamera::ControlList &metadata); > > > > + const libcamera::Camera *camera_; > > std::map<const libcamera::Stream *, std::string> streamNames_; > > std::string pattern_; > > std::map<libcamera::FrameBuffer *, std::unique_ptr<Image>> mappedBuffers_; > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > > index 53c2ffde..c4e18a13 100644 > > --- a/src/cam/main.cpp > > +++ b/src/cam/main.cpp > > @@ -144,6 +144,8 @@ int CamApp::parseOptions(int argc, char *argv[]) > > "to write files, using the default file name. Otherwise it sets the\n" > > "full file path and name. The first '#' character in the file name\n" > > "is expanded to the camera index, stream name and frame sequence number.\n" > > + "If the file name ends with '.dng', then the frame will be written to\n" > > + "the output file(s) in DNG format.\n" > > "The default file name is 'frame-#.bin'.", > > "file", ArgumentOptional, "filename", false, > > OptCamera); > > diff --git a/src/cam/meson.build b/src/cam/meson.build > > index 9c766221..06dbea06 100644 > > --- a/src/cam/meson.build > > +++ b/src/cam/meson.build > > @@ -52,6 +52,13 @@ if libsdl2.found() > > endif > > endif > > > > +if libtiff.found() > > + cam_cpp_args += ['-DHAVE_TIFF'] > > + cam_sources += files([ > > + 'dng_writer.cpp', > > + ]) > > +endif > > + > > cam = executable('cam', cam_sources, > > dependencies : [ > > libatomic, > > @@ -60,6 +67,7 @@ cam = executable('cam', cam_sources, > > libevent, > > libjpeg, > > libsdl2, > > + libtiff, > > libyaml, > > ], > > cpp_args : cam_cpp_args,
Hi Laurent On Wed, Oct 19, 2022 at 12:40:02PM +0300, Laurent Pinchart wrote: > On Wed, Oct 19, 2022 at 10:34:32AM +0200, Jacopo Mondi via libcamera-devel wrote: > > On Tue, Oct 18, 2022 at 05:09:07PM +0900, Paul Elder via libcamera-devel wrote: > > > Add support for outputting buffers in DNG format. It reuses the DNG > > > writer that we had previously in qcam. > > > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > > > > > --- > > > Changes in v2: > > > - update help test to mention the dng output feature > > > - pass camera as first argument to FileSink constructor > > > --- > > > src/cam/camera_session.cpp | 4 ++-- > > > src/cam/file_sink.cpp | 32 ++++++++++++++++++++++++++------ > > > src/cam/file_sink.h | 7 +++++-- > > > src/cam/main.cpp | 2 ++ > > > src/cam/meson.build | 8 ++++++++ > > > 5 files changed, 43 insertions(+), 10 deletions(-) > > > > > > diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp > > > index 238186a3..6b409c98 100644 > > > --- a/src/cam/camera_session.cpp > > > +++ b/src/cam/camera_session.cpp > > > @@ -207,10 +207,10 @@ int CameraSession::start() > > > > > > if (options_.isSet(OptFile)) { > > > if (!options_[OptFile].toString().empty()) > > > - sink_ = std::make_unique<FileSink>(streamNames_, > > > + sink_ = std::make_unique<FileSink>(camera_.get(), streamNames_, > > > options_[OptFile]); > > > else > > > - sink_ = std::make_unique<FileSink>(streamNames_); > > > + sink_ = std::make_unique<FileSink>(camera_.get(), streamNames_); > > > } > > > > > > if (sink_) { > > > diff --git a/src/cam/file_sink.cpp b/src/cam/file_sink.cpp > > > index 45213d4a..8bba4e7f 100644 > > > --- a/src/cam/file_sink.cpp > > > +++ b/src/cam/file_sink.cpp > > > @@ -15,14 +15,16 @@ > > > > > > #include <libcamera/camera.h> > > > > > > +#include "dng_writer.h" > > > #include "file_sink.h" > > > #include "image.h" > > > > > > using namespace libcamera; > > > > > > -FileSink::FileSink(const std::map<const libcamera::Stream *, std::string> &streamNames, > > > +FileSink::FileSink(const libcamera::Camera *camera, > > > + const std::map<const libcamera::Stream *, std::string> &streamNames, > > > const std::string &pattern) > > > - : streamNames_(streamNames), pattern_(pattern) > > > + : camera_(camera), streamNames_(streamNames), pattern_(pattern) > > > { > > > } > > > > > > @@ -51,12 +53,13 @@ void FileSink::mapBuffer(FrameBuffer *buffer) > > > bool FileSink::processRequest(Request *request) > > > { > > > for (auto [stream, buffer] : request->buffers()) > > > - writeBuffer(stream, buffer); > > > + writeBuffer(stream, buffer, request->metadata()); > > > > > > return true; > > > } > > > > > > -void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) > > > +void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer, > > > + [[maybe_unused]] const ControlList &metadata) > > > { > > > std::string filename; > > > > bool dng = false; > > > > > size_t pos; > > > @@ -65,6 +68,10 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) > > > if (!pattern_.empty()) > > > filename = pattern_; > > > > > > +#ifdef HAVE_TIFF > > > + bool dng = filename.find(".dng", filename.size() - 4) != std::string::npos; > > > > dng = filename.find(".dng", filename.size() - 4) != std::string::npos; > > > > > +#endif /* HAVE_TIFF */ > > > + > > > if (filename.empty() || filename.back() == '/') > > > filename += "frame-#.bin"; > > > > > > @@ -76,6 +83,21 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) > > > filename.replace(pos, 1, ss.str()); > > > } > > > > > > + Image *image = mappedBuffers_[buffer].get(); > > > + > > > +#ifdef HAVE_TIFF > > > > And you can here remove the #ifdef and the [[maybe_unused]] attribute from the > > metadata argument > > When !HAVE_TIFF, the DNGWriter class isn't declared by dng_writer.h. The > code below would thus not compile. > At a first look I don't seen anything that depends on libtiff in dng_writer.h But as it would require an additional change on top which is clearly not for this series Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > > > + if (dng) { > > > + ret = DNGWriter::write(filename.c_str(), camera_, > > > + stream->configuration(), metadata, > > > + buffer, image->data(0).data()); > > > + if (ret < 0) > > > + std::cerr << "failed to write DNG " << filename > > > + << std::endl; > > > + > > > + return; > > > + } > > > +#endif /* HAVE_TIFF */ > > > + > > > fd = open(filename.c_str(), O_CREAT | O_WRONLY | > > > (pos == std::string::npos ? O_APPEND : O_TRUNC), > > > S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH); > > > @@ -86,8 +108,6 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) > > > return; > > > } > > > > > > - Image *image = mappedBuffers_[buffer].get(); > > > - > > > for (unsigned int i = 0; i < buffer->planes().size(); ++i) { > > > const FrameMetadata::Plane &meta = buffer->metadata().planes()[i]; > > > > > > diff --git a/src/cam/file_sink.h b/src/cam/file_sink.h > > > index 067736f5..9ce8b619 100644 > > > --- a/src/cam/file_sink.h > > > +++ b/src/cam/file_sink.h > > > @@ -20,7 +20,8 @@ class Image; > > > class FileSink : public FrameSink > > > { > > > public: > > > - FileSink(const std::map<const libcamera::Stream *, std::string> &streamNames, > > > + FileSink(const libcamera::Camera *camera, > > > + const std::map<const libcamera::Stream *, std::string> &streamNames, > > > const std::string &pattern = ""); > > > ~FileSink(); > > > > > > @@ -32,8 +33,10 @@ public: > > > > > > private: > > > void writeBuffer(const libcamera::Stream *stream, > > > > If only we could get the Camera * back from the request, you could > > avoid storing camera_ as class member as you could do > > > > bool FileSink::processRequest(Request *request) > > { > > for (auto [stream, buffer] : request->buffers()) > > writeBuffer(request->camera(), stream, buffer, request->metadata()); > > > > return true; > > } > > > > I recall we discussed that in the past and if it's not there there > > might be reasons (likely the fact that a Request can outlive the > > Camera it has been created ?) > > > > > - libcamera::FrameBuffer *buffer); > > > + libcamera::FrameBuffer *buffer, > > > + const libcamera::ControlList &metadata); > > > > > > + const libcamera::Camera *camera_; > > > std::map<const libcamera::Stream *, std::string> streamNames_; > > > std::string pattern_; > > > std::map<libcamera::FrameBuffer *, std::unique_ptr<Image>> mappedBuffers_; > > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > > > index 53c2ffde..c4e18a13 100644 > > > --- a/src/cam/main.cpp > > > +++ b/src/cam/main.cpp > > > @@ -144,6 +144,8 @@ int CamApp::parseOptions(int argc, char *argv[]) > > > "to write files, using the default file name. Otherwise it sets the\n" > > > "full file path and name. The first '#' character in the file name\n" > > > "is expanded to the camera index, stream name and frame sequence number.\n" > > > + "If the file name ends with '.dng', then the frame will be written to\n" > > > + "the output file(s) in DNG format.\n" > > > "The default file name is 'frame-#.bin'.", > > > "file", ArgumentOptional, "filename", false, > > > OptCamera); > > > diff --git a/src/cam/meson.build b/src/cam/meson.build > > > index 9c766221..06dbea06 100644 > > > --- a/src/cam/meson.build > > > +++ b/src/cam/meson.build > > > @@ -52,6 +52,13 @@ if libsdl2.found() > > > endif > > > endif > > > > > > +if libtiff.found() > > > + cam_cpp_args += ['-DHAVE_TIFF'] > > > + cam_sources += files([ > > > + 'dng_writer.cpp', > > > + ]) > > > +endif > > > + > > > cam = executable('cam', cam_sources, > > > dependencies : [ > > > libatomic, > > > @@ -60,6 +67,7 @@ cam = executable('cam', cam_sources, > > > libevent, > > > libjpeg, > > > libsdl2, > > > + libtiff, > > > libyaml, > > > ], > > > cpp_args : cam_cpp_args, > > -- > Regards, > > Laurent Pinchart
diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp index 238186a3..6b409c98 100644 --- a/src/cam/camera_session.cpp +++ b/src/cam/camera_session.cpp @@ -207,10 +207,10 @@ int CameraSession::start() if (options_.isSet(OptFile)) { if (!options_[OptFile].toString().empty()) - sink_ = std::make_unique<FileSink>(streamNames_, + sink_ = std::make_unique<FileSink>(camera_.get(), streamNames_, options_[OptFile]); else - sink_ = std::make_unique<FileSink>(streamNames_); + sink_ = std::make_unique<FileSink>(camera_.get(), streamNames_); } if (sink_) { diff --git a/src/cam/file_sink.cpp b/src/cam/file_sink.cpp index 45213d4a..8bba4e7f 100644 --- a/src/cam/file_sink.cpp +++ b/src/cam/file_sink.cpp @@ -15,14 +15,16 @@ #include <libcamera/camera.h> +#include "dng_writer.h" #include "file_sink.h" #include "image.h" using namespace libcamera; -FileSink::FileSink(const std::map<const libcamera::Stream *, std::string> &streamNames, +FileSink::FileSink(const libcamera::Camera *camera, + const std::map<const libcamera::Stream *, std::string> &streamNames, const std::string &pattern) - : streamNames_(streamNames), pattern_(pattern) + : camera_(camera), streamNames_(streamNames), pattern_(pattern) { } @@ -51,12 +53,13 @@ void FileSink::mapBuffer(FrameBuffer *buffer) bool FileSink::processRequest(Request *request) { for (auto [stream, buffer] : request->buffers()) - writeBuffer(stream, buffer); + writeBuffer(stream, buffer, request->metadata()); return true; } -void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) +void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer, + [[maybe_unused]] const ControlList &metadata) { std::string filename; size_t pos; @@ -65,6 +68,10 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) if (!pattern_.empty()) filename = pattern_; +#ifdef HAVE_TIFF + bool dng = filename.find(".dng", filename.size() - 4) != std::string::npos; +#endif /* HAVE_TIFF */ + if (filename.empty() || filename.back() == '/') filename += "frame-#.bin"; @@ -76,6 +83,21 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) filename.replace(pos, 1, ss.str()); } + Image *image = mappedBuffers_[buffer].get(); + +#ifdef HAVE_TIFF + if (dng) { + ret = DNGWriter::write(filename.c_str(), camera_, + stream->configuration(), metadata, + buffer, image->data(0).data()); + if (ret < 0) + std::cerr << "failed to write DNG " << filename + << std::endl; + + return; + } +#endif /* HAVE_TIFF */ + fd = open(filename.c_str(), O_CREAT | O_WRONLY | (pos == std::string::npos ? O_APPEND : O_TRUNC), S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH); @@ -86,8 +108,6 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) return; } - Image *image = mappedBuffers_[buffer].get(); - for (unsigned int i = 0; i < buffer->planes().size(); ++i) { const FrameMetadata::Plane &meta = buffer->metadata().planes()[i]; diff --git a/src/cam/file_sink.h b/src/cam/file_sink.h index 067736f5..9ce8b619 100644 --- a/src/cam/file_sink.h +++ b/src/cam/file_sink.h @@ -20,7 +20,8 @@ class Image; class FileSink : public FrameSink { public: - FileSink(const std::map<const libcamera::Stream *, std::string> &streamNames, + FileSink(const libcamera::Camera *camera, + const std::map<const libcamera::Stream *, std::string> &streamNames, const std::string &pattern = ""); ~FileSink(); @@ -32,8 +33,10 @@ public: private: void writeBuffer(const libcamera::Stream *stream, - libcamera::FrameBuffer *buffer); + libcamera::FrameBuffer *buffer, + const libcamera::ControlList &metadata); + const libcamera::Camera *camera_; std::map<const libcamera::Stream *, std::string> streamNames_; std::string pattern_; std::map<libcamera::FrameBuffer *, std::unique_ptr<Image>> mappedBuffers_; diff --git a/src/cam/main.cpp b/src/cam/main.cpp index 53c2ffde..c4e18a13 100644 --- a/src/cam/main.cpp +++ b/src/cam/main.cpp @@ -144,6 +144,8 @@ int CamApp::parseOptions(int argc, char *argv[]) "to write files, using the default file name. Otherwise it sets the\n" "full file path and name. The first '#' character in the file name\n" "is expanded to the camera index, stream name and frame sequence number.\n" + "If the file name ends with '.dng', then the frame will be written to\n" + "the output file(s) in DNG format.\n" "The default file name is 'frame-#.bin'.", "file", ArgumentOptional, "filename", false, OptCamera); diff --git a/src/cam/meson.build b/src/cam/meson.build index 9c766221..06dbea06 100644 --- a/src/cam/meson.build +++ b/src/cam/meson.build @@ -52,6 +52,13 @@ if libsdl2.found() endif endif +if libtiff.found() + cam_cpp_args += ['-DHAVE_TIFF'] + cam_sources += files([ + 'dng_writer.cpp', + ]) +endif + cam = executable('cam', cam_sources, dependencies : [ libatomic, @@ -60,6 +67,7 @@ cam = executable('cam', cam_sources, libevent, libjpeg, libsdl2, + libtiff, libyaml, ], cpp_args : cam_cpp_args,
Add support for outputting buffers in DNG format. It reuses the DNG writer that we had previously in qcam. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- Changes in v2: - update help test to mention the dng output feature - pass camera as first argument to FileSink constructor --- src/cam/camera_session.cpp | 4 ++-- src/cam/file_sink.cpp | 32 ++++++++++++++++++++++++++------ src/cam/file_sink.h | 7 +++++-- src/cam/main.cpp | 2 ++ src/cam/meson.build | 8 ++++++++ 5 files changed, 43 insertions(+), 10 deletions(-)