[libcamera-devel] [PATCH 2/3] cam: Pass stream names to FileSink

[Kieran Bingham]

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 at ideasonboard.com>

> 
> Fixes: 02001fecb0f5 ("cam: Turn BufferWriter into a FrameSink")
> Signed-off-by: Laurent Pinchart <laurent.pinchart at 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
>