[libcamera-devel] [PATCH v2 2/3] cam: file_sink: Add support for DNG output

Laurent Pinchart laurent.pinchart at ideasonboard.com
Wed Oct 19 11:40:02 CEST 2022


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 at 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,

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list