Details
-
Bug
-
Resolution: Fixed
-
P1: Critical
-
None
-
6.8
-
None
-
-
3ef980abb (dev), c5a991b47 (6.8)
-
Multimedia wk 39-40
Description
These commits caused a chain of regressions in QVideoFrame transformation
https://codereview.qt-project.org/c/qt/qtmultimedia/+/557991
https://codereview.qt-project.org/c/qt/qtmultimedia/+/585403
Explanation.
QVideoFrame contains QVideoFrameFormat that describes different format specifications, including transformation: rotation, and mirroring. Also, QVideoFrame has properties 'mirrored' and 'rotation'. Before the CRs above, QVideoFrame used its own variables for rotation and mirroring, in other words, QVideoFrame::mirrored and QVideoFrameFormat::mirrored, QVideoFrame::rotation and QVideoFrameFormat::rotation were independent of each other. The CRs make the QVideoFrame's properties referred to QVideoFrameFormat's ones. In 6.8, users are supposed to create frames with custom data and possibly set a transformation to the frames, so it's nice to provide a clear and non-ambiguous mechanism for setting transformation. This is the rationale for the refactoring.
The problem
Front cameras on mobile devices make the following issue. In most use cases, video from front cameras should be displayed with mirroring (it's typical for seeing selfies), but recorded without it (because the data is not actually mirrored). With independent properties of frame and format, the problem was solved by setting the actual transformations to QVideoFrameFormat, which impacted displaying and recording, and setting extra transformation to QVideoFrame, which was to be shown only. Currently, with unified properties, the frame can be only mirrored or not, the same for both, recording and showing. Note, that we should also consider that a user can set their own transformation, so we can't just ignore any of it.
How it should have been designed
QVideoFrame's transformations should have been non-ambiguous,
Solutions
1. Revert logic that was added in these 2 CRs and a bunch of fixes on the top of them. Also, document that QVideoFrameFormat's transformation is related to the surface and it is to be recorded, whereas QVideoFrame's transformations are applied of frames on the top of first ones, and used for rendering only. Reverting can be implemented as amending to the existing code in order to fix previous transformation issues.
Pros:
- the user logic is the same as before, even though it's clumsy.
Cons:
- the API of setting QVideoFrame's transformations is tricky and not self-descriptive.
- If both, frame and format have their own transformations, it's harder for users to know the final transform.
2. Leave the discussed refactoring in the code, make front cameras emit non-mirrored frames, but add a private property to QVideoFrame that points that it can have extra mirroring, and read the property and mirror frames in QVideoWidget, and qml VideoOutput.
Pros:
- we get non-ambiguous QVideoFrame API.
Cons:
- having private property in frames, which impacts on rendering, is not transparent for users.
3. Leave the discussed refactoring in the code, and add an emergent small API change with setting mirroring flag for rendering to qml VideoOutput and QVideoWidget. The rationale is that in general case the business logic of the application decides how the view should be displayed.
Pros:
- probably the best solution design-wise.
Cons:
- API change before release, which is not good.
- we need to modify our examples as well.
- users will have to modify their code.