[libcamera-devel] [RFC 0/4] libcamera: Replace CameraConfiguration::transform

[Robert Mader]

Hi David, thanks for initiative to bring us all on the same page.

 From what I understand, this series aims to do two things:


1.: Make the public API follow the EXIF specification

I personally, having implemented the existing API in Pipewire and, still 
pending, the Gstreamer element, am rather neutral on this, as the new 
and old values have a bijective relationship. All I care about that it's 
easy for apps to transition and support both APIs for at least a short 
while.


2.: Clarifying the expected behavior and fixing existing inconsistencies.

The main issue with the existing implementation I personally have been 
running into, that is fixed by this series, was the returned Transform 
value from `validate()`.

To give an example, consider the following two cases:

  * A camera mounted 90 deg rotated, a config requesting
    `Transform::Identity`
  * No camera rotation, a config requesting `Transform::rot90`

In the first case (typical for Linux phones like the Pinephone (Pro) and 
Librem5) the returned value will be `Transform::rot270` (IIRC) and the 
user can use that to adapt the output downstream.

In the second case, the returned value will be `Transform::Identity` and 
the user has to remember that it asked for something different.

Combining a non-identity mounting rotation and a non-identity requested 
transform thus essentially leads to random results and makes a clean 
implementation almost impossible.

I personally like the proposed behavior - either you get the requested 
result or you have to apply the combined transform yourself - a lot.

 From what I understand there are only two open questions here:

 1. Should the sensor do partial transforms?
 2. Do we really want/need the new `Orientation` enum or could/should we
    instead with `Transform` and just update/clarify the expected results?


Hope you agree that these are the issues to agree on.

Thanks everyone and cheers,

Robert

On 23.06.23 10:26, David Plowman via libcamera-devel wrote:
> Hi everyone
>
> Thanks for that message. I think it's very helpful to quote the source
> documentation:
>
>   * The transform is a user-specified 2D plane transform that will be applied
>   * to the camera images by the processing pipeline before being handed to
>   * the application. This is subsequent to any transform that is already
>   * required to fix up any platform-defined rotation.
>
> What this is trying to say, badly, is that the transform you request
> is applied after ("subsequent to") fixing up the platform rotation. So
> you do indeed get auto-correction. This is also the actual behaviour
> of the code on the Pi, has been for, I don't know, nearly 2 years? At
> this point, the earth does indeed get my official permission to
> swallow me up for not writing something that was less prone to
> misinterpretation,
>
> But I think this does leave us in a position where large amounts of
> this thread are a mixture of not very helpful or even plain confusing,
> and with people's permission, I'd honestly like to start again, with
> everyone on the same page.
>
> You might have noticed that "orientations" were really bugging me as
> to what they were, and how one could use them. I've managed to resolve
> these problems in my own mind and think that there's a very simple
> connection between "orientations" and "transforms" that I would be OK
> to implement and use. I think our next attempt at this discussion
> should start by being clear on our requirements, and then we can look
> at how "orientations" and "transforms" can get us there.
>
> (Rest of thread purposefully deleted... hope that's ok!)
>
> Thanks!
> David
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20230623/40a2b2c8/attachment.htm>