| Message ID | 20210804124314.8044-4-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On Wed, Aug 04, 2021 at 03:43:09PM +0300, Laurent Pinchart wrote: > Make the BufferWriter class inherit from FrameSink, and use the > FrameSink API to manage it. This makes the code more generic, and will > allow usage of other sinks. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > Changes since v2: > > - Write all buffers > - Fix errno printing in writeBuffer() > > Changes since v1: > > - Print message if the file can't be opened > --- > src/cam/buffer_writer.cpp | 39 +++++++++++++++++++---- > src/cam/buffer_writer.h | 17 +++++++--- > src/cam/camera_session.cpp | 64 ++++++++++++++++++++++++++++++++------ > src/cam/camera_session.h | 6 ++-- > 4 files changed, 104 insertions(+), 22 deletions(-) > > diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp > index a7648a92fc92..2f4b2b02d3cb 100644 > --- a/src/cam/buffer_writer.cpp > +++ b/src/cam/buffer_writer.cpp > @@ -13,6 +13,8 @@ > #include <sys/mman.h> > #include <unistd.h> > > +#include <libcamera/camera.h> > + > #include "buffer_writer.h" > > using namespace libcamera; > @@ -32,6 +34,21 @@ BufferWriter::~BufferWriter() > mappedBuffers_.clear(); > } > > +int BufferWriter::configure(const libcamera::CameraConfiguration &config) > +{ > + int ret = FrameSink::configure(config); > + if (ret < 0) > + return ret; > + > + streamNames_.clear(); > + for (unsigned int index = 0; index < config.size(); ++index) { > + const StreamConfiguration &cfg = config.at(index); > + streamNames_[cfg.stream()] = "stream" + std::to_string(index); > + } > + > + return 0; > +} > + > void BufferWriter::mapBuffer(FrameBuffer *buffer) > { > for (const FrameBuffer::Plane &plane : buffer->planes()) { > @@ -43,7 +60,15 @@ void BufferWriter::mapBuffer(FrameBuffer *buffer) > } > } > > -int BufferWriter::write(FrameBuffer *buffer, const std::string &streamName) > +bool BufferWriter::processRequest(Request *request) > +{ > + for (auto [stream, buffer] : request->buffers()) > + writeBuffer(stream, buffer); > + > + return true; > +} > + > +void BufferWriter::writeBuffer(const Stream *stream, FrameBuffer *buffer) > { > std::string filename; > size_t pos; > @@ -58,7 +83,7 @@ int BufferWriter::write(FrameBuffer *buffer, const std::string &streamName) > pos = filename.find_first_of('#'); > if (pos != std::string::npos) { > std::stringstream ss; > - ss << streamName << "-" << std::setw(6) > + ss << streamNames_[stream] << "-" << std::setw(6) > << std::setfill('0') << buffer->metadata().sequence; > filename.replace(pos, 1, ss.str()); > } > @@ -66,8 +91,12 @@ int BufferWriter::write(FrameBuffer *buffer, const std::string &streamName) > 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); > - if (fd == -1) > - return -errno; > + if (fd == -1) { > + ret = -errno; > + std::cerr << "failed to open file " << filename << ": " > + << strerror(-ret) << std::endl; > + return; > + } > > for (unsigned int i = 0; i < buffer->planes().size(); ++i) { > const FrameBuffer::Plane &plane = buffer->planes()[i]; > @@ -96,6 +125,4 @@ int BufferWriter::write(FrameBuffer *buffer, const std::string &streamName) > } > > close(fd); > - > - return ret; > } > diff --git a/src/cam/buffer_writer.h b/src/cam/buffer_writer.h > index 7626de42d369..32bb6ed5e045 100644 > --- a/src/cam/buffer_writer.h > +++ b/src/cam/buffer_writer.h > @@ -10,20 +10,27 @@ > #include <map> > #include <string> > > -#include <libcamera/framebuffer.h> > +#include <libcamera/stream.h> > > -class BufferWriter > +#include "frame_sink.h" > + > +class BufferWriter : public FrameSink > { > public: > BufferWriter(const std::string &pattern = ""); > ~BufferWriter(); > > - void mapBuffer(libcamera::FrameBuffer *buffer); > + int configure(const libcamera::CameraConfiguration &config) override; > > - int write(libcamera::FrameBuffer *buffer, > - const std::string &streamName); > + void mapBuffer(libcamera::FrameBuffer *buffer) override; > + > + bool processRequest(libcamera::Request *request) override; > > private: > + void writeBuffer(const libcamera::Stream *stream, > + libcamera::FrameBuffer *buffer); > + > + std::map<const libcamera::Stream *, std::string> streamNames_; > std::string pattern_; > std::map<int, std::pair<void *, unsigned int>> mappedBuffers_; > }; > diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp > index 0d49fc1ade83..f91b5234a082 100644 > --- a/src/cam/camera_session.cpp > +++ b/src/cam/camera_session.cpp > @@ -13,6 +13,7 @@ > #include <libcamera/control_ids.h> > #include <libcamera/property_ids.h> > > +#include "buffer_writer.h" > #include "camera_session.h" > #include "event_loop.h" > #include "main.h" > @@ -162,9 +163,20 @@ int CameraSession::start() > > if (options_.isSet(OptFile)) { > if (!options_[OptFile].toString().empty()) > - writer_ = std::make_unique<BufferWriter>(options_[OptFile]); > + sink_ = std::make_unique<BufferWriter>(options_[OptFile]); > else > - writer_ = std::make_unique<BufferWriter>(); > + sink_ = std::make_unique<BufferWriter>(); > + } > + > + if (sink_) { > + ret = sink_->configure(*config_); > + if (ret < 0) { > + std::cout << "Failed to configure frame sink" > + << std::endl; > + return ret; > + } > + > + sink_->requestProcessed.connect(this, &CameraSession::sinkRelease); > } > > allocator_ = std::make_unique<FrameBufferAllocator>(camera_); > @@ -178,7 +190,13 @@ void CameraSession::stop() > if (ret) > std::cout << "Failed to stop capture" << std::endl; > > - writer_.reset(); > + if (sink_) { > + ret = sink_->stop(); > + if (ret) > + std::cout << "Failed to stop frame sink" << std::endl; > + } > + > + sink_.reset(); > > requests_.clear(); > > @@ -227,16 +245,26 @@ int CameraSession::startCapture() > return ret; > } > > - if (writer_) > - writer_->mapBuffer(buffer.get()); > + if (sink_) > + sink_->mapBuffer(buffer.get()); > } > > requests_.push_back(std::move(request)); > } > > + if (sink_) { > + ret = sink_->start(); > + if (ret) { > + std::cout << "Failed to start frame sink" << std::endl; > + return ret; > + } > + } > + > ret = camera_->start(); > if (ret) { > std::cout << "Failed to start capture" << std::endl; > + if (sink_) > + sink_->stop(); > return ret; > } > > @@ -245,6 +273,8 @@ int CameraSession::startCapture() > if (ret < 0) { > std::cerr << "Can't queue request" << std::endl; > camera_->stop(); > + if (sink_) > + sink_->stop(); > return ret; > } > } > @@ -296,6 +326,8 @@ void CameraSession::processRequest(Request *request) > fps = last_ != 0 && fps ? 1000000000.0 / fps : 0.0; > last_ = ts; > > + bool requeue = true; > + > std::stringstream info; > info << ts / 1000000000 << "." > << std::setw(6) << std::setfill('0') << ts / 1000 % 1000000 > @@ -304,11 +336,10 @@ void CameraSession::processRequest(Request *request) > for (auto it = buffers.begin(); it != buffers.end(); ++it) { > const Stream *stream = it->first; > FrameBuffer *buffer = it->second; > - const std::string &name = streamName_[stream]; > > const FrameMetadata &metadata = buffer->metadata(); > > - info << " " << name > + info << " " << streamName_[stream] > << " seq: " << std::setw(6) << std::setfill('0') << metadata.sequence > << " bytesused: "; > > @@ -318,9 +349,11 @@ void CameraSession::processRequest(Request *request) > if (++nplane < metadata.planes.size()) > info << "/"; > } > + } > > - if (writer_) > - writer_->write(buffer, name); > + if (sink_) { > + if (!sink_->processRequest(request)) > + requeue = false; > } > > std::cout << info.str() << std::endl; > @@ -340,6 +373,19 @@ void CameraSession::processRequest(Request *request) > return; > } > > + /* > + * If the frame sink holds on the request, we'll requeue it later in the > + * complete handler. > + */ > + if (!requeue) > + return; > + > + request->reuse(Request::ReuseBuffers); > + camera_->queueRequest(request); > +} > + > +void CameraSession::sinkRelease(Request *request) > +{ > request->reuse(Request::ReuseBuffers); > queueRequest(request); > } > diff --git a/src/cam/camera_session.h b/src/cam/camera_session.h > index b0f50e7f998e..2ccc71977a99 100644 > --- a/src/cam/camera_session.h > +++ b/src/cam/camera_session.h > @@ -21,9 +21,10 @@ > #include <libcamera/request.h> > #include <libcamera/stream.h> > > -#include "buffer_writer.h" > #include "options.h" > > +class FrameSink; > + > class CameraSession > { > public: > @@ -53,13 +54,14 @@ private: > int queueRequest(libcamera::Request *request); > void requestComplete(libcamera::Request *request); > void processRequest(libcamera::Request *request); > + void sinkRelease(libcamera::Request *request); > > const OptionsParser::Options &options_; > std::shared_ptr<libcamera::Camera> camera_; > std::unique_ptr<libcamera::CameraConfiguration> config_; > > std::map<const libcamera::Stream *, std::string> streamName_; > - std::unique_ptr<BufferWriter> writer_; > + std::unique_ptr<FrameSink> sink_; > unsigned int cameraIndex_; > > uint64_t last_; > -- > Regards, > > Laurent Pinchart >
diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp index a7648a92fc92..2f4b2b02d3cb 100644 --- a/src/cam/buffer_writer.cpp +++ b/src/cam/buffer_writer.cpp @@ -13,6 +13,8 @@ #include <sys/mman.h> #include <unistd.h> +#include <libcamera/camera.h> + #include "buffer_writer.h" using namespace libcamera; @@ -32,6 +34,21 @@ BufferWriter::~BufferWriter() mappedBuffers_.clear(); } +int BufferWriter::configure(const libcamera::CameraConfiguration &config) +{ + int ret = FrameSink::configure(config); + if (ret < 0) + return ret; + + streamNames_.clear(); + for (unsigned int index = 0; index < config.size(); ++index) { + const StreamConfiguration &cfg = config.at(index); + streamNames_[cfg.stream()] = "stream" + std::to_string(index); + } + + return 0; +} + void BufferWriter::mapBuffer(FrameBuffer *buffer) { for (const FrameBuffer::Plane &plane : buffer->planes()) { @@ -43,7 +60,15 @@ void BufferWriter::mapBuffer(FrameBuffer *buffer) } } -int BufferWriter::write(FrameBuffer *buffer, const std::string &streamName) +bool BufferWriter::processRequest(Request *request) +{ + for (auto [stream, buffer] : request->buffers()) + writeBuffer(stream, buffer); + + return true; +} + +void BufferWriter::writeBuffer(const Stream *stream, FrameBuffer *buffer) { std::string filename; size_t pos; @@ -58,7 +83,7 @@ int BufferWriter::write(FrameBuffer *buffer, const std::string &streamName) pos = filename.find_first_of('#'); if (pos != std::string::npos) { std::stringstream ss; - ss << streamName << "-" << std::setw(6) + ss << streamNames_[stream] << "-" << std::setw(6) << std::setfill('0') << buffer->metadata().sequence; filename.replace(pos, 1, ss.str()); } @@ -66,8 +91,12 @@ int BufferWriter::write(FrameBuffer *buffer, const std::string &streamName) 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); - if (fd == -1) - return -errno; + if (fd == -1) { + ret = -errno; + std::cerr << "failed to open file " << filename << ": " + << strerror(-ret) << std::endl; + return; + } for (unsigned int i = 0; i < buffer->planes().size(); ++i) { const FrameBuffer::Plane &plane = buffer->planes()[i]; @@ -96,6 +125,4 @@ int BufferWriter::write(FrameBuffer *buffer, const std::string &streamName) } close(fd); - - return ret; } diff --git a/src/cam/buffer_writer.h b/src/cam/buffer_writer.h index 7626de42d369..32bb6ed5e045 100644 --- a/src/cam/buffer_writer.h +++ b/src/cam/buffer_writer.h @@ -10,20 +10,27 @@ #include <map> #include <string> -#include <libcamera/framebuffer.h> +#include <libcamera/stream.h> -class BufferWriter +#include "frame_sink.h" + +class BufferWriter : public FrameSink { public: BufferWriter(const std::string &pattern = ""); ~BufferWriter(); - void mapBuffer(libcamera::FrameBuffer *buffer); + int configure(const libcamera::CameraConfiguration &config) override; - int write(libcamera::FrameBuffer *buffer, - const std::string &streamName); + void mapBuffer(libcamera::FrameBuffer *buffer) override; + + bool processRequest(libcamera::Request *request) override; private: + void writeBuffer(const libcamera::Stream *stream, + libcamera::FrameBuffer *buffer); + + std::map<const libcamera::Stream *, std::string> streamNames_; std::string pattern_; std::map<int, std::pair<void *, unsigned int>> mappedBuffers_; }; diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp index 0d49fc1ade83..f91b5234a082 100644 --- a/src/cam/camera_session.cpp +++ b/src/cam/camera_session.cpp @@ -13,6 +13,7 @@ #include <libcamera/control_ids.h> #include <libcamera/property_ids.h> +#include "buffer_writer.h" #include "camera_session.h" #include "event_loop.h" #include "main.h" @@ -162,9 +163,20 @@ int CameraSession::start() if (options_.isSet(OptFile)) { if (!options_[OptFile].toString().empty()) - writer_ = std::make_unique<BufferWriter>(options_[OptFile]); + sink_ = std::make_unique<BufferWriter>(options_[OptFile]); else - writer_ = std::make_unique<BufferWriter>(); + sink_ = std::make_unique<BufferWriter>(); + } + + if (sink_) { + ret = sink_->configure(*config_); + if (ret < 0) { + std::cout << "Failed to configure frame sink" + << std::endl; + return ret; + } + + sink_->requestProcessed.connect(this, &CameraSession::sinkRelease); } allocator_ = std::make_unique<FrameBufferAllocator>(camera_); @@ -178,7 +190,13 @@ void CameraSession::stop() if (ret) std::cout << "Failed to stop capture" << std::endl; - writer_.reset(); + if (sink_) { + ret = sink_->stop(); + if (ret) + std::cout << "Failed to stop frame sink" << std::endl; + } + + sink_.reset(); requests_.clear(); @@ -227,16 +245,26 @@ int CameraSession::startCapture() return ret; } - if (writer_) - writer_->mapBuffer(buffer.get()); + if (sink_) + sink_->mapBuffer(buffer.get()); } requests_.push_back(std::move(request)); } + if (sink_) { + ret = sink_->start(); + if (ret) { + std::cout << "Failed to start frame sink" << std::endl; + return ret; + } + } + ret = camera_->start(); if (ret) { std::cout << "Failed to start capture" << std::endl; + if (sink_) + sink_->stop(); return ret; } @@ -245,6 +273,8 @@ int CameraSession::startCapture() if (ret < 0) { std::cerr << "Can't queue request" << std::endl; camera_->stop(); + if (sink_) + sink_->stop(); return ret; } } @@ -296,6 +326,8 @@ void CameraSession::processRequest(Request *request) fps = last_ != 0 && fps ? 1000000000.0 / fps : 0.0; last_ = ts; + bool requeue = true; + std::stringstream info; info << ts / 1000000000 << "." << std::setw(6) << std::setfill('0') << ts / 1000 % 1000000 @@ -304,11 +336,10 @@ void CameraSession::processRequest(Request *request) for (auto it = buffers.begin(); it != buffers.end(); ++it) { const Stream *stream = it->first; FrameBuffer *buffer = it->second; - const std::string &name = streamName_[stream]; const FrameMetadata &metadata = buffer->metadata(); - info << " " << name + info << " " << streamName_[stream] << " seq: " << std::setw(6) << std::setfill('0') << metadata.sequence << " bytesused: "; @@ -318,9 +349,11 @@ void CameraSession::processRequest(Request *request) if (++nplane < metadata.planes.size()) info << "/"; } + } - if (writer_) - writer_->write(buffer, name); + if (sink_) { + if (!sink_->processRequest(request)) + requeue = false; } std::cout << info.str() << std::endl; @@ -340,6 +373,19 @@ void CameraSession::processRequest(Request *request) return; } + /* + * If the frame sink holds on the request, we'll requeue it later in the + * complete handler. + */ + if (!requeue) + return; + + request->reuse(Request::ReuseBuffers); + camera_->queueRequest(request); +} + +void CameraSession::sinkRelease(Request *request) +{ request->reuse(Request::ReuseBuffers); queueRequest(request); } diff --git a/src/cam/camera_session.h b/src/cam/camera_session.h index b0f50e7f998e..2ccc71977a99 100644 --- a/src/cam/camera_session.h +++ b/src/cam/camera_session.h @@ -21,9 +21,10 @@ #include <libcamera/request.h> #include <libcamera/stream.h> -#include "buffer_writer.h" #include "options.h" +class FrameSink; + class CameraSession { public: @@ -53,13 +54,14 @@ private: int queueRequest(libcamera::Request *request); void requestComplete(libcamera::Request *request); void processRequest(libcamera::Request *request); + void sinkRelease(libcamera::Request *request); const OptionsParser::Options &options_; std::shared_ptr<libcamera::Camera> camera_; std::unique_ptr<libcamera::CameraConfiguration> config_; std::map<const libcamera::Stream *, std::string> streamName_; - std::unique_ptr<BufferWriter> writer_; + std::unique_ptr<FrameSink> sink_; unsigned int cameraIndex_; uint64_t last_;