GNOME Bugzilla – Bug 731782
waylandsink: video position is wrong when caps change and the sink is embedded in another window
Last modified: 2014-11-08 14:15:04 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.
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.