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

[David Plowman]

Hi Robert, everyone

Thanks for the message. I think that's helpful and makes sense to me.
It seems to me that a large part of this is to clarify what's meant to
happen, to resolve inconsistencies (and bugs!), and to make sure that
everyone gets what they need. To try and summarise my own
requirements:

1. I want any transform to be auto-corrected, that is, for any
mounting rotation to be corrected transparently for me. However, I
still think we should report what the mounting rotation is.

2. When it's possible to produce the requested transform, then
obviously I want it to do that!

3. When it's not possible to deliver the requested transform, then I
don't really mind what we choose to do, contingent on having
requirement 4. For example, I would be fine with libcamera doing
nothing at all, so that the "actual" transform returned would be the
sensor mounting rotation (does everyone agree that this is what would
happen?).

4. I want to be able to use the libcamera classes to compare what I
requested versus what I got, so that I can figure out what I need to
do. This should have absolutely no impact on the libcamera API (which
is principally a question of what happens to the transform during
validate()). Using the libcamera classes to work out what to do is
entirely optional.

A few final observations:

* I'm fine with changing the meaning of the "rotation" property.
Though we need to ensure this doesn't change the existing behaviour
(on the Pi, can't speak for certain about anything else!), and as
noted above, I think we should report the mounting rotation in another
way.

* I'm fine with introducing an Orientation if that's helpful to
people. If we put the orientation into the camera configuration
(replacing the transform) then I want the orientation class to
interact correctly with the transform class so that we still have
requirement 4. In this case the orientation class also needs to be
fully implemented in the Python bindings, as transforms are.

* I still have this niggle in my mind that the sense of the rotations
was not settled in our previous discussions. We need to resolve this.

That's everything I can think of at the minute. Does it seem reasonable?

Thanks!
David

On Fri, 23 Jun 2023 at 12:05, Robert Mader via libcamera-devel
<libcamera-devel at lists.libcamera.org> wrote:
>
> 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:
>
> Should the sensor do partial transforms?
> 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