| Message ID | 20190403011235.12782-4-niklas.soderlund@ragnatech.se |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Niklas, On 03/04/2019 02:12, Niklas Söderlund wrote: > The completion handler needs to handle all buffers in the request. Solve > this by iterating over all buffers in the completed request and assign > each stream a name as we encounter them. The buffer writer needs to be > extended to learn about stream names so it can prefix the files it > writes with it. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/cam/buffer_writer.cpp | 5 +++-- > src/cam/buffer_writer.h | 2 +- > src/cam/main.cpp | 42 ++++++++++++++++++++++++++++----------- > 3 files changed, 34 insertions(+), 15 deletions(-) > > diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp > index 2d2258b4cd1cbbc2..b82ffaded688226a 100644 > --- a/src/cam/buffer_writer.cpp > +++ b/src/cam/buffer_writer.cpp > @@ -19,13 +19,14 @@ BufferWriter::BufferWriter(const std::string &pattern) > { > } > > -int BufferWriter::write(libcamera::Buffer *buffer) > +int BufferWriter::write(libcamera::Buffer *buffer, > + const std::string &streamName) > { > std::string filename; > size_t pos; > int fd, ret = 0; > > - filename = pattern_; > + filename = streamName + "-" + pattern_; > pos = filename.find_first_of('#'); > if (pos != std::string::npos) { > std::stringstream ss; > diff --git a/src/cam/buffer_writer.h b/src/cam/buffer_writer.h > index 9705773e0e397d45..7bf785d1e83235ff 100644 > --- a/src/cam/buffer_writer.h > +++ b/src/cam/buffer_writer.h > @@ -16,7 +16,7 @@ class BufferWriter > public: > BufferWriter(const std::string &pattern = "frame-#.bin"); > > - int write(libcamera::Buffer *buffer); > + int write(libcamera::Buffer *buffer, const std::string &streamName); > > private: > std::string pattern_; > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > index b47bda21cbb7f220..9af7907a3d937c28 100644 > --- a/src/cam/main.cpp > +++ b/src/cam/main.cpp > @@ -145,28 +145,46 @@ static int prepareCameraConfig(std::map<Stream *, StreamConfiguration> *config) > return 0; > } > > +static std::string streamToName(const Stream *stream) > +{ > + static std::map<const Stream *, std::string> names; > + > + if (names.find(stream) == names.end()) > + names[stream] = std::string("stream") + std::to_string(names.size()); > + > + return names[stream]; > +} > + > static void requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers) > { > static uint64_t last = 0; > + double fps = 0.0; > > if (request->status() == Request::RequestCancelled) > return; > > - Buffer *buffer = buffers.begin()->second; > + for (auto it = buffers.begin(); it != buffers.end(); ++it) { > + Stream *stream = it->first; > + Buffer *buffer = it->second; > + std::string name = streamToName(stream); > > - double fps = buffer->timestamp() - last; > - fps = last && fps ? 1000000000.0 / fps : 0.0; > - last = buffer->timestamp(); > + if (it == buffers.begin()) { > + fps = buffer->timestamp() - last; > + fps = last && fps ? 1000000000.0 / fps : 0.0; > + last = buffer->timestamp(); > + } Should we track FPS on a per stream basis? Here you are really calculating Requests (with at least one buffer) per second, which might be a perfectly reasonable thing to calculate, but what if (as a simple extreme) I queued up requests to alternate between two streams A and B. -------- R:1 A R:2 B R:3 A R:4 B -------- Is that 4 frames per interval? or two? It might be worth splitting the calculation out to a function and storing the result on a per stream basis without using static locals. > > - std::cout << "seq: " << std::setw(6) << std::setfill('0') << buffer->sequence() > - << " buf: " << buffer->index() > - << " bytesused: " << buffer->bytesused() > - << " timestamp: " << buffer->timestamp() > - << " fps: " << std::fixed << std::setprecision(2) << fps > - << std::endl; > + std::cout << name << " seq: " << std::setw(6) > + << std::setfill('0') << buffer->sequence() > + << " buf: " << buffer->index() > + << " bytesused: " << buffer->bytesused() > + << " timestamp: " << buffer->timestamp() > + << " fps: " << std::fixed << std::setprecision(2) << fps > + << std::endl; > > - if (writer) > - writer->write(buffer); > + if (writer) > + writer->write(buffer, name); > + } > > request = camera->createRequest(); > if (!request) { >
Hi Kieran, On Wed, Apr 03, 2019 at 11:24:27AM +0700, Kieran Bingham wrote: > On 03/04/2019 02:12, Niklas Söderlund wrote: > > The completion handler needs to handle all buffers in the request. Solve > > this by iterating over all buffers in the completed request and assign > > each stream a name as we encounter them. The buffer writer needs to be > > extended to learn about stream names so it can prefix the files it > > writes with it. > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > --- > > src/cam/buffer_writer.cpp | 5 +++-- > > src/cam/buffer_writer.h | 2 +- > > src/cam/main.cpp | 42 ++++++++++++++++++++++++++++----------- > > 3 files changed, 34 insertions(+), 15 deletions(-) > > > > diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp > > index 2d2258b4cd1cbbc2..b82ffaded688226a 100644 > > --- a/src/cam/buffer_writer.cpp > > +++ b/src/cam/buffer_writer.cpp > > @@ -19,13 +19,14 @@ BufferWriter::BufferWriter(const std::string &pattern) > > { > > } > > > > -int BufferWriter::write(libcamera::Buffer *buffer) > > +int BufferWriter::write(libcamera::Buffer *buffer, > > + const std::string &streamName) > > { > > std::string filename; > > size_t pos; > > int fd, ret = 0; > > > > - filename = pattern_; > > + filename = streamName + "-" + pattern_; > > pos = filename.find_first_of('#'); > > if (pos != std::string::npos) { > > std::stringstream ss; > > diff --git a/src/cam/buffer_writer.h b/src/cam/buffer_writer.h > > index 9705773e0e397d45..7bf785d1e83235ff 100644 > > --- a/src/cam/buffer_writer.h > > +++ b/src/cam/buffer_writer.h > > @@ -16,7 +16,7 @@ class BufferWriter > > public: > > BufferWriter(const std::string &pattern = "frame-#.bin"); > > > > - int write(libcamera::Buffer *buffer); > > + int write(libcamera::Buffer *buffer, const std::string &streamName); > > > > private: > > std::string pattern_; > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > > index b47bda21cbb7f220..9af7907a3d937c28 100644 > > --- a/src/cam/main.cpp > > +++ b/src/cam/main.cpp > > @@ -145,28 +145,46 @@ static int prepareCameraConfig(std::map<Stream *, StreamConfiguration> *config) > > return 0; > > } > > > > +static std::string streamToName(const Stream *stream) > > +{ > > + static std::map<const Stream *, std::string> names; > > + > > + if (names.find(stream) == names.end()) > > + names[stream] = std::string("stream") + std::to_string(names.size()); > > + > > + return names[stream]; > > +} > > + > > static void requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers) > > { > > static uint64_t last = 0; > > + double fps = 0.0; > > > > if (request->status() == Request::RequestCancelled) > > return; > > > > - Buffer *buffer = buffers.begin()->second; > > + for (auto it = buffers.begin(); it != buffers.end(); ++it) { > > + Stream *stream = it->first; > > + Buffer *buffer = it->second; > > + std::string name = streamToName(stream); > > > > - double fps = buffer->timestamp() - last; > > - fps = last && fps ? 1000000000.0 / fps : 0.0; > > - last = buffer->timestamp(); > > + if (it == buffers.begin()) { > > + fps = buffer->timestamp() - last; > > + fps = last && fps ? 1000000000.0 / fps : 0.0; > > + last = buffer->timestamp(); > > + } > > > Should we track FPS on a per stream basis? Here you are really > calculating Requests (with at least one buffer) per second, which might > be a perfectly reasonable thing to calculate, but what if (as a simple > extreme) I queued up requests to alternate between two streams A and B. > > -------- > R:1 A > R:2 B > R:3 A > R:4 B > -------- > > Is that 4 frames per interval? or two? I think the most interesting FPS value relates to the frame rate of the sensor, so that should be tracked at the request level, not the stream level. It could however make sense to count the number of frames captured for every stream and add the counts to the log message. > It might be worth splitting the calculation out to a function and > storing the result on a per stream basis without using static locals. We should create a top-level class for the cam application to store the existing global variables. > > > > - std::cout << "seq: " << std::setw(6) << std::setfill('0') << buffer->sequence() > > - << " buf: " << buffer->index() > > - << " bytesused: " << buffer->bytesused() > > - << " timestamp: " << buffer->timestamp() > > - << " fps: " << std::fixed << std::setprecision(2) << fps > > - << std::endl; > > + std::cout << name << " seq: " << std::setw(6) > > + << std::setfill('0') << buffer->sequence() > > + << " buf: " << buffer->index() > > + << " bytesused: " << buffer->bytesused() > > + << " timestamp: " << buffer->timestamp() > > + << " fps: " << std::fixed << std::setprecision(2) << fps > > + << std::endl; > > > > - if (writer) > > - writer->write(buffer); > > + if (writer) > > + writer->write(buffer, name); > > + } > > > > request = camera->createRequest(); > > if (!request) {
Hi Niklas, On Wed, Apr 03, 2019 at 03:12:34AM +0200, Niklas Söderlund wrote: > The completion handler needs to handle all buffers in the request. Solve > this by iterating over all buffers in the completed request and assign > each stream a name as we encounter them. The buffer writer needs to be > extended to learn about stream names so it can prefix the files it > writes with it. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/cam/buffer_writer.cpp | 5 +++-- > src/cam/buffer_writer.h | 2 +- > src/cam/main.cpp | 42 ++++++++++++++++++++++++++++----------- > 3 files changed, 34 insertions(+), 15 deletions(-) > > diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp > index 2d2258b4cd1cbbc2..b82ffaded688226a 100644 > --- a/src/cam/buffer_writer.cpp > +++ b/src/cam/buffer_writer.cpp > @@ -19,13 +19,14 @@ BufferWriter::BufferWriter(const std::string &pattern) > { > } > > -int BufferWriter::write(libcamera::Buffer *buffer) > +int BufferWriter::write(libcamera::Buffer *buffer, > + const std::string &streamName) > { > std::string filename; > size_t pos; > int fd, ret = 0; > > - filename = pattern_; > + filename = streamName + "-" + pattern_; > pos = filename.find_first_of('#'); > if (pos != std::string::npos) { > std::stringstream ss; > diff --git a/src/cam/buffer_writer.h b/src/cam/buffer_writer.h > index 9705773e0e397d45..7bf785d1e83235ff 100644 > --- a/src/cam/buffer_writer.h > +++ b/src/cam/buffer_writer.h > @@ -16,7 +16,7 @@ class BufferWriter > public: > BufferWriter(const std::string &pattern = "frame-#.bin"); > > - int write(libcamera::Buffer *buffer); > + int write(libcamera::Buffer *buffer, const std::string &streamName); > > private: > std::string pattern_; > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > index b47bda21cbb7f220..9af7907a3d937c28 100644 > --- a/src/cam/main.cpp > +++ b/src/cam/main.cpp > @@ -145,28 +145,46 @@ static int prepareCameraConfig(std::map<Stream *, StreamConfiguration> *config) > return 0; > } > > +static std::string streamToName(const Stream *stream) > +{ > + static std::map<const Stream *, std::string> names; > + > + if (names.find(stream) == names.end()) > + names[stream] = std::string("stream") + std::to_string(names.size()); > + > + return names[stream]; > +} > + > static void requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers) > { > static uint64_t last = 0; > + double fps = 0.0; > > if (request->status() == Request::RequestCancelled) > return; > > - Buffer *buffer = buffers.begin()->second; > + for (auto it = buffers.begin(); it != buffers.end(); ++it) { > + Stream *stream = it->first; > + Buffer *buffer = it->second; > + std::string name = streamToName(stream); As I've run into this issue before, and I solved it in a quite hackish way, how would you here deal with the following scenario: 1) Create a request 2) Add <Stream *viewfinder, Buffer *buffer> to the request 3) int x = 0; 4) Queue the request; 5) Handle the completed request; 6) Re-add the just completed <Stream *viewfinder, Buffer *buffer> pair 7) if (x < 10) then x++; goto 4); else goto 8) 8) Add a <Stream *stillcapture, Buffer *buffer> to the request 9) Handle the completed request, now with 2 streams 10) Requeue viewfinder only All this procedure just to say: how would an application recognize what stream is for which usage? Should they keep track of that or do we plan to provide a way to make Streams identifiable? (eg. by adding the StreamUsage to the Stream) Thanks j > > - double fps = buffer->timestamp() - last; > - fps = last && fps ? 1000000000.0 / fps : 0.0; > - last = buffer->timestamp(); > + if (it == buffers.begin()) { > + fps = buffer->timestamp() - last; > + fps = last && fps ? 1000000000.0 / fps : 0.0; > + last = buffer->timestamp(); > + } > > - std::cout << "seq: " << std::setw(6) << std::setfill('0') << buffer->sequence() > - << " buf: " << buffer->index() > - << " bytesused: " << buffer->bytesused() > - << " timestamp: " << buffer->timestamp() > - << " fps: " << std::fixed << std::setprecision(2) << fps > - << std::endl; > + std::cout << name << " seq: " << std::setw(6) > + << std::setfill('0') << buffer->sequence() > + << " buf: " << buffer->index() > + << " bytesused: " << buffer->bytesused() > + << " timestamp: " << buffer->timestamp() > + << " fps: " << std::fixed << std::setprecision(2) << fps > + << std::endl; > > - if (writer) > - writer->write(buffer); > + if (writer) > + writer->write(buffer, name); > + } > > request = camera->createRequest(); > if (!request) { > -- > 2.21.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Niklas, On Wed, Apr 03, 2019 at 03:12:34AM +0200, Niklas Söderlund wrote: > The completion handler needs to handle all buffers in the request. Solve > this by iterating over all buffers in the completed request and assign > each stream a name as we encounter them. The buffer writer needs to be > extended to learn about stream names so it can prefix the files it > writes with it. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/cam/buffer_writer.cpp | 5 +++-- > src/cam/buffer_writer.h | 2 +- > src/cam/main.cpp | 42 ++++++++++++++++++++++++++++----------- > 3 files changed, 34 insertions(+), 15 deletions(-) > > diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp > index 2d2258b4cd1cbbc2..b82ffaded688226a 100644 > --- a/src/cam/buffer_writer.cpp > +++ b/src/cam/buffer_writer.cpp > @@ -19,13 +19,14 @@ BufferWriter::BufferWriter(const std::string &pattern) > { > } > > -int BufferWriter::write(libcamera::Buffer *buffer) > +int BufferWriter::write(libcamera::Buffer *buffer, > + const std::string &streamName) > { > std::string filename; > size_t pos; > int fd, ret = 0; > > - filename = pattern_; > + filename = streamName + "-" + pattern_; this should be the other way around, otherwise I get: stream1-/tmp/frame-# FYI I sent this exact same patch in my v4 imgu+multi stream series: "[PATCH v4 28/31] src: cam: Add output name to frame writer" where I handle multiple stream requests too, as I assume you have not seen that series. thanks j > pos = filename.find_first_of('#'); > if (pos != std::string::npos) { > std::stringstream ss; > diff --git a/src/cam/buffer_writer.h b/src/cam/buffer_writer.h > index 9705773e0e397d45..7bf785d1e83235ff 100644 > --- a/src/cam/buffer_writer.h > +++ b/src/cam/buffer_writer.h > @@ -16,7 +16,7 @@ class BufferWriter > public: > BufferWriter(const std::string &pattern = "frame-#.bin"); > > - int write(libcamera::Buffer *buffer); > + int write(libcamera::Buffer *buffer, const std::string &streamName); > > private: > std::string pattern_; > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > index b47bda21cbb7f220..9af7907a3d937c28 100644 > --- a/src/cam/main.cpp > +++ b/src/cam/main.cpp > @@ -145,28 +145,46 @@ static int prepareCameraConfig(std::map<Stream *, StreamConfiguration> *config) > return 0; > } > > +static std::string streamToName(const Stream *stream) > +{ > + static std::map<const Stream *, std::string> names; > + > + if (names.find(stream) == names.end()) > + names[stream] = std::string("stream") + std::to_string(names.size()); > + > + return names[stream]; > +} > + > static void requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers) > { > static uint64_t last = 0; > + double fps = 0.0; > > if (request->status() == Request::RequestCancelled) > return; > > - Buffer *buffer = buffers.begin()->second; > + for (auto it = buffers.begin(); it != buffers.end(); ++it) { > + Stream *stream = it->first; > + Buffer *buffer = it->second; > + std::string name = streamToName(stream); > > - double fps = buffer->timestamp() - last; > - fps = last && fps ? 1000000000.0 / fps : 0.0; > - last = buffer->timestamp(); > + if (it == buffers.begin()) { > + fps = buffer->timestamp() - last; > + fps = last && fps ? 1000000000.0 / fps : 0.0; > + last = buffer->timestamp(); > + } > > - std::cout << "seq: " << std::setw(6) << std::setfill('0') << buffer->sequence() > - << " buf: " << buffer->index() > - << " bytesused: " << buffer->bytesused() > - << " timestamp: " << buffer->timestamp() > - << " fps: " << std::fixed << std::setprecision(2) << fps > - << std::endl; > + std::cout << name << " seq: " << std::setw(6) > + << std::setfill('0') << buffer->sequence() > + << " buf: " << buffer->index() > + << " bytesused: " << buffer->bytesused() > + << " timestamp: " << buffer->timestamp() > + << " fps: " << std::fixed << std::setprecision(2) << fps > + << std::endl; > > - if (writer) > - writer->write(buffer); > + if (writer) > + writer->write(buffer, name); > + } > > request = camera->createRequest(); > if (!request) { > -- > 2.21.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, Thanks for your feedback. On 2019-04-03 16:05:41 +0200, Jacopo Mondi wrote: > Hi Niklas, > > On Wed, Apr 03, 2019 at 03:12:34AM +0200, Niklas Söderlund wrote: > > The completion handler needs to handle all buffers in the request. Solve > > this by iterating over all buffers in the completed request and assign > > each stream a name as we encounter them. The buffer writer needs to be > > extended to learn about stream names so it can prefix the files it > > writes with it. > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > --- > > src/cam/buffer_writer.cpp | 5 +++-- > > src/cam/buffer_writer.h | 2 +- > > src/cam/main.cpp | 42 ++++++++++++++++++++++++++++----------- > > 3 files changed, 34 insertions(+), 15 deletions(-) > > > > diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp > > index 2d2258b4cd1cbbc2..b82ffaded688226a 100644 > > --- a/src/cam/buffer_writer.cpp > > +++ b/src/cam/buffer_writer.cpp > > @@ -19,13 +19,14 @@ BufferWriter::BufferWriter(const std::string &pattern) > > { > > } > > > > -int BufferWriter::write(libcamera::Buffer *buffer) > > +int BufferWriter::write(libcamera::Buffer *buffer, > > + const std::string &streamName) > > { > > std::string filename; > > size_t pos; > > int fd, ret = 0; > > > > - filename = pattern_; > > + filename = streamName + "-" + pattern_; > > pos = filename.find_first_of('#'); > > if (pos != std::string::npos) { > > std::stringstream ss; > > diff --git a/src/cam/buffer_writer.h b/src/cam/buffer_writer.h > > index 9705773e0e397d45..7bf785d1e83235ff 100644 > > --- a/src/cam/buffer_writer.h > > +++ b/src/cam/buffer_writer.h > > @@ -16,7 +16,7 @@ class BufferWriter > > public: > > BufferWriter(const std::string &pattern = "frame-#.bin"); > > > > - int write(libcamera::Buffer *buffer); > > + int write(libcamera::Buffer *buffer, const std::string &streamName); > > > > private: > > std::string pattern_; > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > > index b47bda21cbb7f220..9af7907a3d937c28 100644 > > --- a/src/cam/main.cpp > > +++ b/src/cam/main.cpp > > @@ -145,28 +145,46 @@ static int prepareCameraConfig(std::map<Stream *, StreamConfiguration> *config) > > return 0; > > } > > > > +static std::string streamToName(const Stream *stream) > > +{ > > + static std::map<const Stream *, std::string> names; > > + > > + if (names.find(stream) == names.end()) > > + names[stream] = std::string("stream") + std::to_string(names.size()); > > + > > + return names[stream]; > > +} > > + > > static void requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers) > > { > > static uint64_t last = 0; > > + double fps = 0.0; > > > > if (request->status() == Request::RequestCancelled) > > return; > > > > - Buffer *buffer = buffers.begin()->second; > > + for (auto it = buffers.begin(); it != buffers.end(); ++it) { > > + Stream *stream = it->first; > > + Buffer *buffer = it->second; > > + std::string name = streamToName(stream); > > As I've run into this issue before, and I solved it in a quite hackish > way, how would you here deal with the following scenario: > > 1) Create a request > 2) Add <Stream *viewfinder, Buffer *buffer> to the request > 3) int x = 0; > 4) Queue the request; > 5) Handle the completed request; > 6) Re-add the just completed <Stream *viewfinder, Buffer *buffer> pair > 7) if (x < 10) then x++; goto 4); else goto 8) > 8) Add a <Stream *stillcapture, Buffer *buffer> to the request > 9) Handle the completed request, now with 2 streams > 10) Requeue viewfinder only > > All this procedure just to say: how would an application recognize > what stream is for which usage? Should they keep track of that or do > we plan to provide a way to make Streams identifiable? (eg. by adding > the StreamUsage to the Stream) This is a good question and on outstanding issue we need to solve really soon. Currently there is no way to make a stream identifiable. > > Thanks > j > > > > > - double fps = buffer->timestamp() - last; > > - fps = last && fps ? 1000000000.0 / fps : 0.0; > > - last = buffer->timestamp(); > > + if (it == buffers.begin()) { > > + fps = buffer->timestamp() - last; > > + fps = last && fps ? 1000000000.0 / fps : 0.0; > > + last = buffer->timestamp(); > > + } > > > > - std::cout << "seq: " << std::setw(6) << std::setfill('0') << buffer->sequence() > > - << " buf: " << buffer->index() > > - << " bytesused: " << buffer->bytesused() > > - << " timestamp: " << buffer->timestamp() > > - << " fps: " << std::fixed << std::setprecision(2) << fps > > - << std::endl; > > + std::cout << name << " seq: " << std::setw(6) > > + << std::setfill('0') << buffer->sequence() > > + << " buf: " << buffer->index() > > + << " bytesused: " << buffer->bytesused() > > + << " timestamp: " << buffer->timestamp() > > + << " fps: " << std::fixed << std::setprecision(2) << fps > > + << std::endl; > > > > - if (writer) > > - writer->write(buffer); > > + if (writer) > > + writer->write(buffer, name); > > + } > > > > request = camera->createRequest(); > > if (!request) { > > -- > > 2.21.0 > > > > _______________________________________________ > > libcamera-devel mailing list > > libcamera-devel@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, Thanks for your feedback, On 2019-04-03 16:44:10 +0200, Jacopo Mondi wrote: > Hi Niklas, > > On Wed, Apr 03, 2019 at 03:12:34AM +0200, Niklas Söderlund wrote: > > The completion handler needs to handle all buffers in the request. Solve > > this by iterating over all buffers in the completed request and assign > > each stream a name as we encounter them. The buffer writer needs to be > > extended to learn about stream names so it can prefix the files it > > writes with it. > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > --- > > src/cam/buffer_writer.cpp | 5 +++-- > > src/cam/buffer_writer.h | 2 +- > > src/cam/main.cpp | 42 ++++++++++++++++++++++++++++----------- > > 3 files changed, 34 insertions(+), 15 deletions(-) > > > > diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp > > index 2d2258b4cd1cbbc2..b82ffaded688226a 100644 > > --- a/src/cam/buffer_writer.cpp > > +++ b/src/cam/buffer_writer.cpp > > @@ -19,13 +19,14 @@ BufferWriter::BufferWriter(const std::string &pattern) > > { > > } > > > > -int BufferWriter::write(libcamera::Buffer *buffer) > > +int BufferWriter::write(libcamera::Buffer *buffer, > > + const std::string &streamName) > > { > > std::string filename; > > size_t pos; > > int fd, ret = 0; > > > > - filename = pattern_; > > + filename = streamName + "-" + pattern_; > > this should be the other way around, otherwise I get: > stream1-/tmp/frame-# Thanks for reporting this usage, I had not thought about that. I have taken Laurent's suggestion and add the streamName together with the sequence number while expanding the #. > > FYI I sent this exact same patch in my v4 imgu+multi stream series: > "[PATCH v4 28/31] src: cam: Add output name to frame writer" > where I handle multiple stream requests too, as I assume you have not > seen that series. I had not noticed that patch no :-( > > thanks > j > > > > pos = filename.find_first_of('#'); > > if (pos != std::string::npos) { > > std::stringstream ss; > > diff --git a/src/cam/buffer_writer.h b/src/cam/buffer_writer.h > > index 9705773e0e397d45..7bf785d1e83235ff 100644 > > --- a/src/cam/buffer_writer.h > > +++ b/src/cam/buffer_writer.h > > @@ -16,7 +16,7 @@ class BufferWriter > > public: > > BufferWriter(const std::string &pattern = "frame-#.bin"); > > > > - int write(libcamera::Buffer *buffer); > > + int write(libcamera::Buffer *buffer, const std::string &streamName); > > > > private: > > std::string pattern_; > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > > index b47bda21cbb7f220..9af7907a3d937c28 100644 > > --- a/src/cam/main.cpp > > +++ b/src/cam/main.cpp > > @@ -145,28 +145,46 @@ static int prepareCameraConfig(std::map<Stream *, StreamConfiguration> *config) > > return 0; > > } > > > > +static std::string streamToName(const Stream *stream) > > +{ > > + static std::map<const Stream *, std::string> names; > > + > > + if (names.find(stream) == names.end()) > > + names[stream] = std::string("stream") + std::to_string(names.size()); > > + > > + return names[stream]; > > +} > > + > > static void requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers) > > { > > static uint64_t last = 0; > > + double fps = 0.0; > > > > if (request->status() == Request::RequestCancelled) > > return; > > > > - Buffer *buffer = buffers.begin()->second; > > + for (auto it = buffers.begin(); it != buffers.end(); ++it) { > > + Stream *stream = it->first; > > + Buffer *buffer = it->second; > > + std::string name = streamToName(stream); > > > > - double fps = buffer->timestamp() - last; > > - fps = last && fps ? 1000000000.0 / fps : 0.0; > > - last = buffer->timestamp(); > > + if (it == buffers.begin()) { > > + fps = buffer->timestamp() - last; > > + fps = last && fps ? 1000000000.0 / fps : 0.0; > > + last = buffer->timestamp(); > > + } > > > > - std::cout << "seq: " << std::setw(6) << std::setfill('0') << buffer->sequence() > > - << " buf: " << buffer->index() > > - << " bytesused: " << buffer->bytesused() > > - << " timestamp: " << buffer->timestamp() > > - << " fps: " << std::fixed << std::setprecision(2) << fps > > - << std::endl; > > + std::cout << name << " seq: " << std::setw(6) > > + << std::setfill('0') << buffer->sequence() > > + << " buf: " << buffer->index() > > + << " bytesused: " << buffer->bytesused() > > + << " timestamp: " << buffer->timestamp() > > + << " fps: " << std::fixed << std::setprecision(2) << fps > > + << std::endl; > > > > - if (writer) > > - writer->write(buffer); > > + if (writer) > > + writer->write(buffer, name); > > + } > > > > request = camera->createRequest(); > > if (!request) { > > -- > > 2.21.0 > > > > _______________________________________________ > > libcamera-devel mailing list > > libcamera-devel@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp index 2d2258b4cd1cbbc2..b82ffaded688226a 100644 --- a/src/cam/buffer_writer.cpp +++ b/src/cam/buffer_writer.cpp @@ -19,13 +19,14 @@ BufferWriter::BufferWriter(const std::string &pattern) { } -int BufferWriter::write(libcamera::Buffer *buffer) +int BufferWriter::write(libcamera::Buffer *buffer, + const std::string &streamName) { std::string filename; size_t pos; int fd, ret = 0; - filename = pattern_; + filename = streamName + "-" + pattern_; pos = filename.find_first_of('#'); if (pos != std::string::npos) { std::stringstream ss; diff --git a/src/cam/buffer_writer.h b/src/cam/buffer_writer.h index 9705773e0e397d45..7bf785d1e83235ff 100644 --- a/src/cam/buffer_writer.h +++ b/src/cam/buffer_writer.h @@ -16,7 +16,7 @@ class BufferWriter public: BufferWriter(const std::string &pattern = "frame-#.bin"); - int write(libcamera::Buffer *buffer); + int write(libcamera::Buffer *buffer, const std::string &streamName); private: std::string pattern_; diff --git a/src/cam/main.cpp b/src/cam/main.cpp index b47bda21cbb7f220..9af7907a3d937c28 100644 --- a/src/cam/main.cpp +++ b/src/cam/main.cpp @@ -145,28 +145,46 @@ static int prepareCameraConfig(std::map<Stream *, StreamConfiguration> *config) return 0; } +static std::string streamToName(const Stream *stream) +{ + static std::map<const Stream *, std::string> names; + + if (names.find(stream) == names.end()) + names[stream] = std::string("stream") + std::to_string(names.size()); + + return names[stream]; +} + static void requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers) { static uint64_t last = 0; + double fps = 0.0; if (request->status() == Request::RequestCancelled) return; - Buffer *buffer = buffers.begin()->second; + for (auto it = buffers.begin(); it != buffers.end(); ++it) { + Stream *stream = it->first; + Buffer *buffer = it->second; + std::string name = streamToName(stream); - double fps = buffer->timestamp() - last; - fps = last && fps ? 1000000000.0 / fps : 0.0; - last = buffer->timestamp(); + if (it == buffers.begin()) { + fps = buffer->timestamp() - last; + fps = last && fps ? 1000000000.0 / fps : 0.0; + last = buffer->timestamp(); + } - std::cout << "seq: " << std::setw(6) << std::setfill('0') << buffer->sequence() - << " buf: " << buffer->index() - << " bytesused: " << buffer->bytesused() - << " timestamp: " << buffer->timestamp() - << " fps: " << std::fixed << std::setprecision(2) << fps - << std::endl; + std::cout << name << " seq: " << std::setw(6) + << std::setfill('0') << buffer->sequence() + << " buf: " << buffer->index() + << " bytesused: " << buffer->bytesused() + << " timestamp: " << buffer->timestamp() + << " fps: " << std::fixed << std::setprecision(2) << fps + << std::endl; - if (writer) - writer->write(buffer); + if (writer) + writer->write(buffer, name); + } request = camera->createRequest(); if (!request) {
The completion handler needs to handle all buffers in the request. Solve this by iterating over all buffers in the completed request and assign each stream a name as we encounter them. The buffer writer needs to be extended to learn about stream names so it can prefix the files it writes with it. Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- src/cam/buffer_writer.cpp | 5 +++-- src/cam/buffer_writer.h | 2 +- src/cam/main.cpp | 42 ++++++++++++++++++++++++++++----------- 3 files changed, 34 insertions(+), 15 deletions(-)