[libcamera-devel] A proposal about implementation rules and return values for CameraConfiguration::validate()

[Jacopo Mondi]

Hi Hiro,

On Thu, Oct 29, 2020 at 06:37:44PM +0900, Hirokazu Honda wrote:
> Hi,
>
> I look at validate() implementations of CameraConfigurations (e.g.
> IPU3CameraConfiguration) thinking of mapping the accepted
> configurations to ones requested by Android HAL client.
> CameraConfiguration::validate() returns Valid, Adjusted and Invalid.
> From the implementations, each of them is returned
> [Valid]
> when all configurations are accepted by a native camera. In other
> words, the native camera will produce the same streams as requested.
> [Adjusted]
> when some configurations can be accepted by native camera, while some
> of them are modified like a dimension is aligned and a format is
> changed.
> [Invalid]
> when there is a configuration that cannot be accepted by a native camera.
>
> Although these may not match the definitions, the implementations look
> to follow definitions like this.

I concur we need to establish stricter rules and enrich documentation
to make different CameraConfiguration implementations consistent.

Have you find discrepancies between different pipeline handler
implementations that you want to report ?

> However, the difference between Adjusted and Invalid is very ambiguous.
> I find two problems.
>
> First, which value is returned Adjusted or Invalid, is dependent on
> the order of configurations and CameraConfiguration implementation,
> because present CameraConfiguration classes
> 1.) drop given configurations and regard as Adjusted e.g. [1], and
> 2.) returns Invalid if there is a configuration that cannot be
> accepted by a native camera. e.g. [2, 3]
> For example, if the configuration is {acceptable, non-acceptable,
> non-acceptable}, a CameraConfiguration returns Adjusted if it drops
> the last two ones thanks to 1.).
> On the other hand, if the configuration is {non-acceptable,
> acceptable, no-acceptable}, a CameraConfiguration returns Invalid
> because the first one is configured to a native camera thanks to 2.).
>

Not to be picky, but I read [1]. [2] and [3] as follow

[1] The number of requested streams is not supported ( > 3 for IPU3).
    Adjusting it is trivial as the last one can be dropped and
    proceed to validate the actual request content

[2] The stream combination is not supported (ie {raw raw nv12})
    Deciding which stream to drop is not trivial. Even if we can
    establish a priority ordering and decide, in example, the first
    has higher priority like in 1.) in this example it would not work
    as {raw, raw} won't be supported anyway.

[3] One stream configuration is not supported and cannot be trivially
    adjusted by, in example, rounding or changing its format. In this
    case the IPU3 pipe config calculation fails, and it's not easy to
    correct it as it would require:
    a) finding out which StreamConfiguration makes the calculation
    fail and why
    b) adjust it to a size that satisfies the pipeline configuration
    which is slightly obscure and not completely documented.
    As you can see, it's very platform specific.

All in all I think the criteria is "if it can be trivially adjusted,
adjust it, otherwise consider the configuration invalid".

Unfortunately, what can be "trivially adjusted" depends on the
platform constraints, and we should do better in documenting the most
trivial cases and ensure they are treated uniformly in all the
different pipelines.

> Let's consider the second problem with another example, Android HAL
> requests NV12 360p and MJPEG 360p, and a native camera can output only
> a single stream, not both of them, at the same time.
> Libcamera Android HAL adaptation layer executes
> Configuration::validate() with them.
> The validate() returns Invalid because both of them cannot be produced
> simultaneously.
> However, then Android HAL adaptation layer may want to reproduce MJPEG
> 360p from NV12 360p.
> We would like to know "best-effort" accepted configurations and manage
> to produce all the requested streams from the accepted configurations.
>

I think this 'optimizations' (use the less possible streams from the
camera and rely on software conversion as much as possible, as in this
example) should be kept within the android HAL. It is not easy to
guarantee a mapping that works on one platform works on all others,
and in the example you have provided it's not trivial for the camera
to decide which stream to drop (assuming dropping one stream is enough
to make the configuration Valid).

I know I will be shot down quickly, but I'm afraid mappings between
Android requests and what has to be requested to the libcamera::Camera
are better recorded in a platform-specific configuration file. At
least for 'optimized' devices that have high quality requirements.

If no configuration file is provided, mapping works 'best effort' and
we will try to do our best, without guarantees of consistent behaviour
among different platforms.

> My proposal is to
> 1.) Change the Invalid definition to "no configuration is accepted or
> other critical fatals like config is empty".

I would be very happy to see a more detailed documentation of the
return codes as you proposed doing for 'Invalid'

> 2.) [Filter-rule] When CameraConfiguration drops given configurations,
> it drops ones given later. In other words, earlier the configuration
> is put in cong lists, higher priority the config has.
> 3.) [Best-effort config] When CameraConfiguration find a native camera
> cannot accept all the configurations, it tries to try the given
> configurations from begin to end increasing one by one.

I think this could be made standard behaviour, but they're less
trivial than what it looks like.

In example, in 2.) how does a pipeline handler know that it has to
drop one stream configuration to make the camera configuration valid ?
The trivial case is when the requested number of streams is larger
than the maximum number of streams, in this case yes, dropping the
last one is enough to make sure the validate() process can continue to
actually test what's inside each StreamConfiguration.

But in other cases it might be less trivial to find out that just
dropping one stream makes the rest of the combination valid. Using the
fictional example of a platform with a single scaler that cannot be
shared among different streams, if you're asked for 2 scaled stream
what is the right behavior ? Drop one of the two or adjust one of the
two to the non-scaled size ?

3.) is interesting. If I got it right, the camera configuration looks
like {stream1, stream2, ..., streamN}. The pipeline handler first try
{stream1} then {stream1, stream2} and so on until the configuration is
not valid. Again, the distinction between what is Invalid or
Adjustable might be tricky.

> 4.) (Optional) Add a new return value between Adjusted and Invalid
> like Dropped/Filtered, so that we can know not all the streams are
> accepted. Though we can detect this in a caller by seeing the returned
> config.

Dropped/Filtered is actually an 'Adjusted' special case, they can also
be provided as an 'hint' to validate() to suggest which adjustment
strategy to adopt.

However, I see your problem and I think mapping:

        [Android] x [software converter] x [camera capabilities]

Is a problem in a too complex space to be solved to the level
requested by the typical Android certified devices. A configuration
file for the HAL that tells which post-processor to use, how to map
android requests to [processors * libcamera::Camera] and other product
specific details (I'm thinking about in example properties of the
installed lenses) might be a more effective solution.

Thanks
   j

>
> Any comments are appreciated.
> Thanks in advance!
>
> Best Regards,
> -Hiro
> ---------
> [1] https://git.linuxtv.org/libcamera.git/tree/src/libcamera/pipeline/ipu3/ipu3.cpp#n154
> [2] https://git.linuxtv.org/libcamera.git/tree/src/libcamera/pipeline/ipu3/ipu3.cpp#n175
> [3] https://git.linuxtv.org/libcamera.git/tree/src/libcamera/pipeline/ipu3/ipu3.cpp#n306