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

[Milan Zamazal]

Laurent Pinchart <laurent.pinchart at ideasonboard.com> writes:

> 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.

Eventually, I decided to add new options to act at the stream level and
keeping the original ones to act at the camera level.  Hopefully to get
the best of both.

Unfortunately, it's more complicated (and more work) than I initially
expected.  I posted RFC patches, let's see.  One bug fix and some
cleanup included.

>> 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.

I rejected the idea because it doesn't match well with what happens with
the stream keywords.

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