[RFC PATCH v2 00/13] Enable raw streams with software ISP

[Milan Zamazal]

Hi Laurent,

thank you for the reviews.

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

> Hi Milan,
>
> On Fri, Jan 24, 2025 at 10:57:51PM +0100, Milan Zamazal wrote:
>> This makes raw streams working again in ‘simple’ pipeline when software
>> ISP is enabled for the given device.  At most one raw stream and one
>> processed stream (possibly both at once) are supported.
>> 
>> An example ‘cam’ invocation requesting a raw stream rather than a debayered stream:
>> 
>>   cam -c1 -C8 -s role=raw,width=1920,height=1080 -Ffile#.raw
>> 
>> Or for both raw and processed streams:
>> 
>>   cam -c1 -C8 -s role=raw,width=1920,height=1080,pixelformat=SRGGB8 -s
>> role=viewfinder,width=1920,height=1080,pixelformat=RGB888 -Ffile#
>> 
>> When only a raw stream is requested, there are no exposure/gain
>> adjustments applied.  This could be improved in future, once software
>> ISP gets a mechanism to gather image statistics without processing and
>> using them to make the adjustments, or once manual exposure controls are
>> added to software ISP.  In the meantime, exposure must be changed
>> externally.
>
> This is fine.
>
>> A part of the patches is a small change to ‘cam’ PPM file handling, to
>> be able to produce PPM files together with raw files when both processed
>> and raw streams are requested.
>> 
>> The patches are RFC because I’m not sure whether they are implemented
>> properly.  It’s also necessary to determine the right balance between
>> the requested functionality (single raw + processed streams should be
>> enough for current needs), cleanness of the implementation and the
>> complexity of the patches.
>
> The famous yak shaving balance.
>
> As you'll see in the review of individual patches, I think there's some
> cleanup to be done. 

I agree completely.  I think v3 is a much better version and a good step
in the iterative process of progressing from a proof of concept and
guesswork to a better understanding and making a usable code.

> The good news is that we don't need a full yak shaving, just trimming
> some edges :-)

I tried to save the poor yak and although the v3 patches are
significantly reworked, I tried to make them relatively modest.  Let's
see how much it works for you; I admit I have been struggling with all
the details when working on v3 and may have missed the bigger picture.

>> Changes in v2:
>> - Completely reworked.
>> - Extended to be able to produce a raw stream together with a processed
>>   stream.
>> 
>> Milan Zamazal (13):
>>   libcamera: software_isp: Move a non-loop condition out of the loop
>>   libcamera: simple: Increase the default number of streams to 2
>>   libcamera: simple: Don't use raw output formats with conversions
>>   libcamera: simple: Add plain output configurations to software ISP
>>   libcamera: simple: Identify requested stream roles
>>   libcamera: simple: Protect against null maxPipeConfig
>>   libcamera: simple: Consider raw output configurations
>>   libcamera: simple: Handle adjusted and raw configurations separately
>>   libcamera: simple: Don't use conversion with an added raw stream
>>   libcamera: simple: Make raw streams working
>>   apps: ppm_writer: Add a missing include
>>   apps: ppm_writer: Return EIO on I/O errors
>>   apps: cam: Write raw file if PPM cannot be written
>> 
>>  src/apps/cam/file_sink.cpp                  |  16 +-
>>  src/apps/common/ppm_writer.cpp              |   7 +-
>>  src/libcamera/pipeline/simple/simple.cpp    | 188 ++++++++++++++++----
>>  src/libcamera/software_isp/software_isp.cpp |   7 +-
>>  4 files changed, 168 insertions(+), 50 deletions(-)