| Message ID | Z/t0OSQynbJ0BhEX@duo.ucw.cz |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi! > With raw files, we have no good place to store the metadata, yet > timestamp, width / height and format is useful for further > processing. Implement %x so that timestamped frames etc are possible. > > Signed-off-by: Pavel Machek <pavel@ucw.cz> Any feedback on this one? > diff --git a/src/apps/cam/file_sink.cpp b/src/apps/cam/file_sink.cpp > index 65794a2f..22cdaaa2 100644 > --- a/src/apps/cam/file_sink.cpp > +++ b/src/apps/cam/file_sink.cpp > @@ -16,6 +16,8 @@ > #include <string.h> > #include <unistd.h> > #include <utility> > +#include <chrono> > +#include <ctime> > > #include <libcamera/camera.h> > > @@ -116,6 +118,44 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer, > filename.replace(pos, 1, ss.str()); > } > > + std::ostringstream result; > + for (size_t i = 0; i < filename.size(); ++i) { > + if (filename[i] == '%' && i + 1 < filename.size()) { > + char specifier = filename[i + 1]; > + i++; // Skip specifier character > + switch (specifier) { > + case 's': > + result << std::setw(6) << std::setfill('0') << buffer->metadata().sequence; > + break; > + case 'w': > + result << stream->configuration().size.width; > + break; > + case 'h': > + result << stream->configuration().size.height; > + break; > + case 'f': > + result << stream->configuration().pixelFormat; > + break; > + case 't': { > + auto now = std::chrono::system_clock::now(); > + auto micros = std::chrono::duration_cast<std::chrono::microseconds>( > + now.time_since_epoch()).count(); > + result << micros; > + break; > + } > + case '%': > + result << '%' << specifier; > + break; > + default: > + result << "%BAD%"; > + } > + } else { > + result << filename[i]; > + } > + } > + > + filename = result.str(); > + > Image *image = mappedBuffers_[buffer].get(); > > #ifdef HAVE_TIFF >
Quoting Pavel Machek (2025-05-03 14:21:29) > Hi! > > > With raw files, we have no good place to store the metadata, yet > > timestamp, width / height and format is useful for further > > processing. Implement %x so that timestamped frames etc are possible. > > > > Signed-off-by: Pavel Machek <pavel@ucw.cz> > > Any feedback on this one? I think this is helpful. I'd probably think using some defined combination as a default would be helpful too, and some how using the same definitions across cam, qcam and camshark to make sure the output filename formats are consistent across the toolings would help. Perhaps we should make some header only helpers in libcamera-base to have something optionally/commonly re-usable? > > diff --git a/src/apps/cam/file_sink.cpp b/src/apps/cam/file_sink.cpp > > index 65794a2f..22cdaaa2 100644 > > --- a/src/apps/cam/file_sink.cpp > > +++ b/src/apps/cam/file_sink.cpp > > @@ -16,6 +16,8 @@ > > #include <string.h> > > #include <unistd.h> > > #include <utility> > > +#include <chrono> > > +#include <ctime> > > > > #include <libcamera/camera.h> > > > > @@ -116,6 +118,44 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer, > > filename.replace(pos, 1, ss.str()); > > } > > > > + std::ostringstream result; > > + for (size_t i = 0; i < filename.size(); ++i) { > > + if (filename[i] == '%' && i + 1 < filename.size()) { > > + char specifier = filename[i + 1]; > > + i++; // Skip specifier character > > + switch (specifier) { > > + case 's': > > + result << std::setw(6) << std::setfill('0') << buffer->metadata().sequence; > > + break; > > + case 'w': > > + result << stream->configuration().size.width; > > + break; > > + case 'h': > > + result << stream->configuration().size.height; > > + break; What about a %s for size to automatically output %wx%h (which is already possible with just result << stream->configuration().size; I think ... But you've already proposed s for sequence ;-s Are there any use cases where someone would want to split the width and height separately in the output file name ? (Perhaps customisability would make sense sometime there) > > + case 'f': > > + result << stream->configuration().pixelFormat; > > + break; seems reasonable. I do wonder if we extended this to have multiple chars we could copy the kernel and use %4cc .. (The kernel has %p4cc) > > + case 't': { > > + auto now = std::chrono::system_clock::now(); > > + auto micros = std::chrono::duration_cast<std::chrono::microseconds>( > > + now.time_since_epoch()).count(); Shouldn't this come from the request completion/sensor timestamp information instead ? > > + result << micros; > > + break; > > + } > > + case '%': > > + result << '%' << specifier; > > + break; > > + default: > > + result << "%BAD%"; > > + } > > + } else { > > + result << filename[i]; > > + } > > + } > > + > > + filename = result.str(); > > + > > Image *image = mappedBuffers_[buffer].get(); > > > > #ifdef HAVE_TIFF > > > > > > -- > I don't work for Nazis and criminals, and neither should you. > Boycott Putin, Trump, and Musk!
Hi 2025. 04. 13. 10:22 keltezéssel, Pavel Machek írta: > With raw files, we have no good place to store the metadata, yet > timestamp, width / height and format is useful for further > processing. Implement %x so that timestamped frames etc are possible. > > Signed-off-by: Pavel Machek <pavel@ucw.cz> > > diff --git a/src/apps/cam/file_sink.cpp b/src/apps/cam/file_sink.cpp > index 65794a2f..22cdaaa2 100644 > --- a/src/apps/cam/file_sink.cpp > +++ b/src/apps/cam/file_sink.cpp > @@ -16,6 +16,8 @@ > #include <string.h> > #include <unistd.h> > #include <utility> > +#include <chrono> > +#include <ctime> > > #include <libcamera/camera.h> > > @@ -116,6 +118,44 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer, > filename.replace(pos, 1, ss.str()); > } > > + std::ostringstream result; > + for (size_t i = 0; i < filename.size(); ++i) { > + if (filename[i] == '%' && i + 1 < filename.size()) { > + char specifier = filename[i + 1]; > + i++; // Skip specifier character > + switch (specifier) { > + case 's': > + result << std::setw(6) << std::setfill('0') << buffer->metadata().sequence; > + break; > + case 'w': > + result << stream->configuration().size.width; > + break; > + case 'h': > + result << stream->configuration().size.height; > + break; > + case 'f': > + result << stream->configuration().pixelFormat; > + break; > + case 't': { > + auto now = std::chrono::system_clock::now(); > + auto micros = std::chrono::duration_cast<std::chrono::microseconds>( > + now.time_since_epoch()).count(); > + result << micros; > + break; > + } > + case '%': > + result << '%' << specifier; > + break; > + default: > + result << "%BAD%"; I believe when an unknown specifier is encountered, it should be left as is; or if `setFilePattern()` has been extended to check the format string, then it could potentially abort. Regards, Barnabás Pőcze > + } > + } else { > + result << filename[i]; > + } > + } > + > + filename = result.str(); > + > Image *image = mappedBuffers_[buffer].get(); > > #ifdef HAVE_TIFF >
diff --git a/src/apps/cam/file_sink.cpp b/src/apps/cam/file_sink.cpp index 65794a2f..22cdaaa2 100644 --- a/src/apps/cam/file_sink.cpp +++ b/src/apps/cam/file_sink.cpp @@ -16,6 +16,8 @@ #include <string.h> #include <unistd.h> #include <utility> +#include <chrono> +#include <ctime> #include <libcamera/camera.h> @@ -116,6 +118,44 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer, filename.replace(pos, 1, ss.str()); } + std::ostringstream result; + for (size_t i = 0; i < filename.size(); ++i) { + if (filename[i] == '%' && i + 1 < filename.size()) { + char specifier = filename[i + 1]; + i++; // Skip specifier character + switch (specifier) { + case 's': + result << std::setw(6) << std::setfill('0') << buffer->metadata().sequence; + break; + case 'w': + result << stream->configuration().size.width; + break; + case 'h': + result << stream->configuration().size.height; + break; + case 'f': + result << stream->configuration().pixelFormat; + break; + case 't': { + auto now = std::chrono::system_clock::now(); + auto micros = std::chrono::duration_cast<std::chrono::microseconds>( + now.time_since_epoch()).count(); + result << micros; + break; + } + case '%': + result << '%' << specifier; + break; + default: + result << "%BAD%"; + } + } else { + result << filename[i]; + } + } + + filename = result.str(); + Image *image = mappedBuffers_[buffer].get(); #ifdef HAVE_TIFF
With raw files, we have no good place to store the metadata, yet timestamp, width / height and format is useful for further processing. Implement %x so that timestamped frames etc are possible. Signed-off-by: Pavel Machek <pavel@ucw.cz>