[RFC PATCH v2 13/13] apps: cam: Write raw file if PPM cannot be written

[Laurent Pinchart]

On Mon, Jan 27, 2025 at 01:13:40PM +0100, Milan Zamazal wrote:
> Laurent Pinchart writes:
> > On Fri, Jan 24, 2025 at 10:58:04PM +0100, Milan Zamazal wrote:
> >> `cam' application can be requested to write its output as a PPM file.
> >> However, this is supported only for certain formats (only BGR888
> >> currently).  If the output format cannot be written, `cam' reports an
> >> error and doesn't write anything.
> >> 
> >> This is all right with a single stream input but impractical with
> >> multiple streams of different output formats (e.g. a processed stream +
> >> a raw stream) where some of them can be written in the supported format
> >> while the other not.  In such a case, it's better to write the supported
> >> formats as PPM files and the unsupported formats as raw files, with the
> >> corresponding warning on stderr.  `.raw' extension is added to the file
> >> name in such a case to make clear it's not a PPM file.
> >> 
> >> This is a sort of hack but serves the purpose for the moment.
> >
> > Should we instead change the -F option (and the -D and -S options too I
> > suppose) to act at the stream level instead of the camera level ?
> 
> Something like this would be useful.  But maybe rather than changing the
> given options (how?),

The OptionParser class supports hierarchical options, the addOption()
function takes a parent as its last argument.

> it would be simpler to add a keyword to -s,
> e.g. output=file/sdl/kms.  What do you think?

That's an interesting idea too. Should we then deprecate the -F, -D and
-S options ? I'm a bit concerned about how disturbing that would be.

> >> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> >> ---
> >>  src/apps/cam/file_sink.cpp | 16 +++++++++++-----
> >>  1 file changed, 11 insertions(+), 5 deletions(-)
> >> 
> >> diff --git a/src/apps/cam/file_sink.cpp b/src/apps/cam/file_sink.cpp
> >> index 76e21db9..1a866137 100644
> >> --- a/src/apps/cam/file_sink.cpp
> >> +++ b/src/apps/cam/file_sink.cpp
> >> @@ -7,6 +7,7 @@
> >>  
> >>  #include <array>
> >>  #include <assert.h>
> >> +#include <errno.h>
> >>  #include <fcntl.h>
> >>  #include <iomanip>
> >>  #include <iostream>
> >> @@ -133,11 +134,16 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer,
> >>  	if (fileType_ == FileType::Ppm) {
> >>  		ret = PPMWriter::write(filename.c_str(), stream->configuration(),
> >>  				       image->data(0));
> >> -		if (ret < 0)
> >> -			std::cerr << "failed to write PPM file `" << filename
> >> -				  << "'" << std::endl;
> >> -
> >> -		return;
> >> +		if (ret == -EINVAL) {
> >> +			filename += ".raw";
> >> +			std::cerr << "cannot write file in PPM format, writing `"
> >> +				  << filename << "' instead" << std::endl;
> >> +		} else {
> >> +			if (ret < 0)
> >> +				std::cerr << "failed to write PPM file `" << filename
> >> +					  << "'" << std::endl;
> >> +			return;
> >> +		}
> >>  	}
> >>  
> >>  	fd = open(filename.c_str(), O_CREAT | O_WRONLY |

-- 
Regards,

Laurent Pinchart