ubports / qtubuntu-camera Goto Github PK

Hi,
while you can specify camera.deviceId when using QtMultimedia, as per: https://doc.qt.io/qt-5/qml-qtmultimedia-qtmultimedia.html
and even QtMultimedia.availableCameras apparently returns more than just two cameras if available, you can see starting on this line:

One thing is that it seems to use enum values as defined here:
https://github.com/libhybris/libhybris/blob/9d2860a5a78cb5c6c6beacf544682ce75b6adf9b/hybris/include/hybris/camera/camera_compatibility_layer.h#L32
(I am not sure what's the purpose of that in libhybris, but I guess it more or less matches since probably all the devices so far were working with that device assumption where 0 is back, 1 is front, so might be ok to keep as is if useful somewhere)

However, I am not sure if AalCameraService actually has any use for that and that toggling is clearly wrong. If I set camera.deviceId to some id I want it to be followed. And it actually seems after a lot of trial and error (I was initially creating new methods for that, not understanding at all how this behaves, since I still have barely any understanding of how this works) to have an easy fix just by using m_deviceSelectControl->selectedDevice() directly instead.

You can find a test branch here:
https://gitlab.com/zubozrout/qtubuntu-camera/-/tree/xenial-dont-limit-camera-device

And test app(s) here:

• https://gitlab.com/zubozrout/lenses
• https://gitlab.com/zubozrout/camera-app-playground

I am sure this "hack" is not sufficient and it is not going to be accepted. However, that's about as much as I was able to do from my ignorant position.

Bugs

I have not noticed anything extra wrong with neither of the existing/available lenses. However, I have noticed an unpleasant error requiring the lenses app restart in order for the app to be able to access the cameras again ... when trying to access some cameras which QtMultimedia.availableCameras list, but they are physically not present.

For my phone (Redmi Note 9 Pro) I get the following list:
[{"deviceId":"0","displayName":"Camera 0 Back facing","position":1,"orientation":270},{"deviceId":"1","displayName":"Camera 1 Front facing","position":2,"orientation":90},{"deviceId":"2","displayName":"Camera 2 Back facing","position":1,"orientation":270},{"deviceId":"3","displayName":"Camera 3 Back facing","position":1,"orientation":270},{"deviceId":"4","displayName":"Camera 4 Back facing","position":1,"orientation":270},{"deviceId":"5","displayName":"Camera 5 Front facing","position":2,"orientation":90}]
But for device id 4 and 5 I get the following error:

Already have a texture id and video sink, not creating a new one
	ext1: 1, ext2: 0

after which I just get: no valid device selected: 3 no matter the selection and have to restart the lenses app (camera-app would likely behave the same)

That I guess makes sense, as I have one front camera and 3 back cameras on my device, as per e.g. this image:
https://cdn2.smart-gsm.com//2020/03/Xiaomi-Redmi-Note-9-Pro-4.jpg

Then there is a depth sensor (no idea how it works, but doesn't seem accessible with that). Maybe that's what is listed as deviceId 4, and the fifth device (deviceId 5) is a complete mystery to me.

However, I think that if these two cameras were not listed it would have likely worked just fine. Also, I have not noticed any issues/regressions in the default camera app (without tweaks), nor in Tagger or the Corona App which I've tried.

Any thoughts are highly appreciated. Thank you! :)

This is a code comment to https://github.com/ubports/qtubuntu-camera/blob/xenial/src/aalcameraserviceplugin.cpp#L131

    // Android's orientation means differently compared to QT's orientation.
    // On Android, it means "the angle that the camera image needs to be
    // rotated", but on QT, it means "the physical orientation of the camera
    // sensor". So, the value will have to be inverted.

    return (360 - orientation) % 360;

The TL;DR is that some dark corners of the modulo operator may not have been taken into account by the transformation applied. If I'm totally wrong, sorry for the noise!

The transformation applied to variable orientation seems to serve two purposes:

1. A logic inversion due to the fact that orientation in Android and Qt API has different semantics.
2. A translation between valid range of Android API and Qt API.

Let me make clear that I have no knowledge of either API and I have no broader view about the purpose of this code. But nevertheless the transformation applied looks suspicious. Here is why:

Here's a selection of hypothetical valid orientation angle ranges:

• 0 to 360 (guaranteed positive, guaranteed spread of 360 degrees),
• 0 to +inf (guaranteed positive)
• -180 to +180 (guaranteed spread)
• -inf to +inf (no limits)

The easiest case may be that both, Android API as well as Qt API use the range [0, 360). Then the only reason for the transformation applied is the different semantics. But then, the transformation 360 - orientation would be enough to translate orientation from Android to Qt API. The additional application of the modulo operator (360 - orientation) % 360 indicates that things may be more involved. This requires someone with insight into Android and Qt API to clarify. I cannot comment on this any further. Except, that I think a code comment documenting orientation source and target range would be useful.

Second, let's see if we got the maths right. For the remainder, I assume that target range is [0, 360) hinted to by the trailing modulo operator and that source range is not [0, 360) as the modulo operator would be redundant in that case. Additionally, I assume that source range doesn't contain negative values either, due to complete lack of any mentioning of this in code comments. So I assume source range is [0, +inf).

For input orientation values 0, 1, 2, …, 359, the transformation maps to 0, 359, 358, …, 1. And input orientation 360 maps to 0 again. OK. But what is the result of 361 fed to the transformation (360 - orientation) % 360? 360 - 361 equals -1. -1 % 360 equals … no, not 359. It's -1. While we wanted to clip target orientation angles to the range [0, 360), we get negative results. BAAMM! This is one of the dark corners similar to floor operator behaviour for negative arguments that I always have to look up … and then check with the given compiler. (I tested this with clickable on Ubuntu 18.04 x64. I did not compile the camera app, since I'm a beginner with anything clickable.)

The solution is simple, after the transformation one needs to check for negative result and add another 360 degrees in that case. As I said, I don't know what the correct transformation should look like due to complete ignorance of Android and Qt API. But, in light of some current camera rotation issues, maybe this reasoning helps solving any one of them. Or does this matter at all?

Best!