| Message ID | 20211012022321.27034-3-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Quoting Laurent Pinchart (2021-10-12 03:23:20) > The FileSink class constructs stream names internally the same way that > the CameraSession does, except that it fails to add the camera name. > This results in files being written without the camera name. > > This could be fixed in FileSink, but we would still duplicate code to > construct stream names. Pass the stream names map from CameraSession to > FileSink instead, and store it internally. Ayee ... this seems like a single FileSink sinks multiple streams. I guess that's ok, and perhaps it simplifies the connections - but in my mind a sink is a single sink. So each sink should be constructed with it's appropriate name rather than passing in an array. But if that's how it's already implemented, then this is a shorter fix for getting the names in so I'm going to go along with it. Perhaps I can see some benefits to a single sink receiving all streams (i.e. a fully completed request) so it can pick and choose what it wants to represent. A view finder sink might show all streams perhaps for instance, or just the smallest ;-) Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Fixes: 02001fecb0f5 ("cam: Turn BufferWriter into a FrameSink") > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/cam/camera_session.cpp | 5 +++-- > src/cam/file_sink.cpp | 11 +++-------- > src/cam/file_sink.h | 3 ++- > 3 files changed, 8 insertions(+), 11 deletions(-) > > diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp > index 5a25baab03f5..605018278c5a 100644 > --- a/src/cam/camera_session.cpp > +++ b/src/cam/camera_session.cpp > @@ -193,9 +193,10 @@ int CameraSession::start() > > if (options_.isSet(OptFile)) { > if (!options_[OptFile].toString().empty()) > - sink_ = std::make_unique<FileSink>(options_[OptFile]); > + sink_ = std::make_unique<FileSink>(streamNames_, > + options_[OptFile]); > else > - sink_ = std::make_unique<FileSink>(); > + sink_ = std::make_unique<FileSink>(streamNames_); > } > > if (sink_) { > diff --git a/src/cam/file_sink.cpp b/src/cam/file_sink.cpp > index 3c2e565b27a2..45213d4a54fe 100644 > --- a/src/cam/file_sink.cpp > +++ b/src/cam/file_sink.cpp > @@ -20,8 +20,9 @@ > > using namespace libcamera; > > -FileSink::FileSink(const std::string &pattern) > - : pattern_(pattern) > +FileSink::FileSink(const std::map<const libcamera::Stream *, std::string> &streamNames, > + const std::string &pattern) > + : streamNames_(streamNames), pattern_(pattern) > { > } > > @@ -35,12 +36,6 @@ int FileSink::configure(const libcamera::CameraConfiguration &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; > } > > diff --git a/src/cam/file_sink.h b/src/cam/file_sink.h > index 335be93b8732..8de93a01a1e8 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::string &pattern = ""); > + FileSink(const std::map<const libcamera::Stream *, std::string> &streamNames, > + const std::string &pattern = ""); > ~FileSink(); > > int configure(const libcamera::CameraConfiguration &config) override; > -- > Regards, > > Laurent Pinchart >
diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp index 5a25baab03f5..605018278c5a 100644 --- a/src/cam/camera_session.cpp +++ b/src/cam/camera_session.cpp @@ -193,9 +193,10 @@ int CameraSession::start() if (options_.isSet(OptFile)) { if (!options_[OptFile].toString().empty()) - sink_ = std::make_unique<FileSink>(options_[OptFile]); + sink_ = std::make_unique<FileSink>(streamNames_, + options_[OptFile]); else - sink_ = std::make_unique<FileSink>(); + sink_ = std::make_unique<FileSink>(streamNames_); } if (sink_) { diff --git a/src/cam/file_sink.cpp b/src/cam/file_sink.cpp index 3c2e565b27a2..45213d4a54fe 100644 --- a/src/cam/file_sink.cpp +++ b/src/cam/file_sink.cpp @@ -20,8 +20,9 @@ using namespace libcamera; -FileSink::FileSink(const std::string &pattern) - : pattern_(pattern) +FileSink::FileSink(const std::map<const libcamera::Stream *, std::string> &streamNames, + const std::string &pattern) + : streamNames_(streamNames), pattern_(pattern) { } @@ -35,12 +36,6 @@ int FileSink::configure(const libcamera::CameraConfiguration &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; } diff --git a/src/cam/file_sink.h b/src/cam/file_sink.h index 335be93b8732..8de93a01a1e8 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::string &pattern = ""); + FileSink(const std::map<const libcamera::Stream *, std::string> &streamNames, + const std::string &pattern = ""); ~FileSink(); int configure(const libcamera::CameraConfiguration &config) override;
The FileSink class constructs stream names internally the same way that the CameraSession does, except that it fails to add the camera name. This results in files being written without the camera name. This could be fixed in FileSink, but we would still duplicate code to construct stream names. Pass the stream names map from CameraSession to FileSink instead, and store it internally. Fixes: 02001fecb0f5 ("cam: Turn BufferWriter into a FrameSink") Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/cam/camera_session.cpp | 5 +++-- src/cam/file_sink.cpp | 11 +++-------- src/cam/file_sink.h | 3 ++- 3 files changed, 8 insertions(+), 11 deletions(-)