After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 731782 - waylandsink: video position is wrong when caps change and the sink is embedded in another window
waylandsink: video position is wrong when caps change and the sink is embedde...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-06-17 13:30 UTC by George Kiagiadakis
Modified: 2014-11-08 14:15 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description George Kiagiadakis 2014-06-17 13:30:13 UTC
When waylandsink is embedded with GstVideoOverlay inside another window and the caps change (possibly because we are switching from one video to another), the aspect ratio preservation algorithm changes the viewport size and the subsurface position correctly, but although the size changes, the position doesn't. This is because the subsurface position is essentially the parent surface's state, so we have to commit the parent, over which we have no control inside waylandsink.

There are two possible solutions to this:

1) Extend the GstWaylandVideo interface to include some API that allows us to inform the parent that we want it to re-layout the video subsurface and do the same dance that we do on window resizing, but extending it so that we make sure the new video buffer is committed before the parent commits its new state. I am not fond of this idea as it will add much complexity for the application and high risk for deadlocks if not implemented correctly, but it has the advantage of using only one subsurface (which is supposedly faster - not sure how much, or if it's worth consideration at all)

2) Add another subsurface layer between the video subsurface and the parent surface. This subsurface would be the "video area" surface, which is going to statically cover the whole render rectangle (as set from GstVideoOverlay) and provide the "black border" background. Then the video subsurface will be stacked on top of it and will take the size of the video, respecting the aspect ratio. By having this, when we want to change just the size of the video surface, without touching the application surface, we can commit its new position relative to the video area surface without synchronizing with the application. This also has the feature of allowing us to draw the black borders inside waylandsink, instead of expecting the application to do it, if we want. The drawback is that we add complexity for the compositor. I think, however, that it will have a better result.
Comment 1 George Kiagiadakis 2014-10-11 15:42:48 UTC
commit 5b1c5dbf995cefa2f090522b802c3a24d639e7de
Author: George Kiagiadakis <george.kiagiadakis@collabora.com>
Date:   Tue Jul 1 11:43:20 2014 +0300

    waylandsink: stack the video subsurface into another subsurface that covers the whole render rectangle

    The main reason behind this is that when the video caps change and the video
    subsurface needs to resize and change position, the wl_subsurface.set_position
    call needs a commit in its parent in order to take effect. Previously,
    the parent was the application's surface, over which there is no control.
    Now, the parent is inside the sink, so we can commit it and change size smoothly.

    As a side effect, this also allows the sink to draw its black borders on
    its own, without the need for the application to do that. And another side
    effect is that this can now allow resizing the sink when it is in top-level
    mode and have it respect the aspect ratio.