[PATCH] cam: Implement %x handling

[Kieran Bingham]

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 at 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!