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 711478 - vaapisink: listen to window size changes
vaapisink: listen to window size changes
Status: RESOLVED FIXED
Product: gstreamer-vaapi
Classification: Other
Component: general
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: sreerenj
gstreamer-vaapi maintainer(s)
Depends on: 722248
Blocks: 731852
 
 
Reported: 2013-11-05 11:56 UTC by Sebastian Dröge (slomo)
Modified: 2014-07-30 13:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Handle the events of internally created xwindow in a separate thread (5.14 KB, patch)
2013-12-11 08:29 UTC, sreerenj
none Details | Review
Window: Allow for updating window size from geometry (1.41 KB, patch)
2014-07-16 09:27 UTC, Fabrice Bellet
committed Details | Review
vaapisink: handle geometry changes of X11 windows (9.29 KB, patch)
2014-07-16 09:29 UTC, Fabrice Bellet
accepted-commit_after_freeze Details | Review
vaapisink: listen to window size changes on X11 (10.76 KB, patch)
2014-07-28 16:42 UTC, Gwenole Beauchesne
none Details | Review

Description Sebastian Dröge (slomo) 2013-11-05 11:56:30 UTC
Hi,

when embedding vaapisink into another X11 window (or when using the automatically created window), and resizing it, vaapisink does not react to the window size changes and does not scale the video to fill the complete available area (while preserving the aspect ratio).
Comment 1 Sebastian Dröge (slomo) 2013-11-05 12:01:34 UTC
The ConfigureNotify event is sent to the window when that happens, check xvimagesink or ximagesink for code that handles it.
Comment 2 Holger Kaelberer 2013-11-05 13:22:30 UTC
We implemented this some months ago. Pushed our changes to my gitorious-clone if it helps you:

https://gitorious.org/vaapi/hkaelbers-gstreamer-vaapi/commit/aff531aa53f781050dd054afdf87f7067e10299b

https://gitorious.org/vaapi/hkaelbers-gstreamer-vaapi/commit/922f256c2517aebf019dbf60f724c49c57a5cf52

Sorry, we still have not rebased against upstream master, still based on our branching from 0.2 (or 0.3?) :-(

Holger
Comment 3 Gwenole Beauchesne 2013-11-06 03:16:50 UTC
Hi, IIRC sree also had some code for that. I don't really want to have too X11 specific code in vaapisink.
Comment 4 Sebastian Dröge (slomo) 2013-11-06 08:08:41 UTC
I don't think you can get around that if you want vaapisink to behave properly in applications and not just have it usable for fullscreen mode. Related to that also bug #711479, you'll also need to listen for X11 key events.
Comment 5 Gwenole Beauchesne 2013-11-29 05:36:34 UTC
I believe we can make it for 0.5.8. Sree, could you please post your updated patch? Note that you'd need to replace the GObject property with an adequate accessor now that GstVaapiWindow objects are no longer based on GObjects. :) Thanks.
Comment 6 sreerenj 2013-12-11 08:29:13 UTC
Created attachment 263965 [details] [review]
Handle the events of internally created xwindow in a  separate thread

This was the patch i sent to you on Feb 7,2013 :)..Didn't get time to rebase on top of current gstremaer-vaapi master since i am too busy with some other things at the moment :(
Comment 7 Gwenole Beauchesne 2014-01-15 09:29:57 UTC
Sorry, I changed my mind and would rather handle events in vaapisink only. The trend is to have small enough libgstvaapi* helper libraries and I no longer foresee any usage of those render backends beyond vaapisink at this time.

Historically, my interest was directed towards Gnash and we didn't have auto-plugging, neither context propagation at that time. :)

I would most likely defer this task once cleaner GstVaapiSinkBackend implementations are in place, and where we could actually stuff in all the relevant event thread handling and friends.
Comment 8 Fabrice Bellet 2014-07-16 09:27:43 UTC
Created attachment 280781 [details] [review]
Window: Allow for updating window size from geometry
Comment 9 Fabrice Bellet 2014-07-16 09:29:41 UTC
Created attachment 280782 [details] [review]
vaapisink: handle geometry changes of X11 windows

Here are the two patches from comment #2 rebased against upstream.
Comment 10 Gwenole Beauchesne 2014-07-25 15:39:10 UTC
Review of attachment 280781 [details] [review]:

Pushed to git master branch. Thanks. Changed the interface to gst_vaapi_window_reconfigure().
Comment 11 Gwenole Beauchesne 2014-07-28 16:42:49 UTC
Created attachment 281876 [details] [review]
vaapisink: listen to window size changes on X11

Changes:
- Dropped XInitThreads()
- Cleaned up the code a little
- Made sure that it still builds with --disable-x11 configure option
Comment 12 Gwenole Beauchesne 2014-07-28 16:44:18 UTC
Comment on attachment 280782 [details] [review]
vaapisink: handle geometry changes of X11 windows

Replaced with the attached update. Could you please give it a try?

There are additional changes I would like to make in subsequent patches. That's why I postpone that after 0.5.9 release.
Comment 13 Fabrice Bellet 2014-07-29 20:42:15 UTC
Patch from comment #11 works here, thanks!
Comment 14 Gwenole Beauchesne 2014-07-30 13:48:06 UTC
commit 4b61cc3cd71dcb7671faf43636010fd3b58404fb
Author: Holger Kaelberer <hk@getslash.de>
Date:   Tue Nov 5 14:01:11 2013 +0100

    vaapisink: listen to window size changes on X11.
    
    Allow dynamic changes to the window, e.g. performed by the user, and
    make sure to refresh its contents, while preserving aspect ratio.
    
    In practice, Expose and ConfigureNotify events are tracked in X11
    display mode by default. This occurs in a separte event thread, and
    this is similar to what xvimagesink does. Any of those events will
    trigger a reconfiguration of the window "soft" size, subsequently
    the render-rect when necessary, and finally _expose() the result.
    
    The default of handle_events=true can be changed programatically via
    gst_x_overlay_handle_events().
    
    Thanks to Fabrice Bellet for rebasing the patch.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=711478
    
    [dropped XInitThreads(), cleaned up the code a little]
    Signed-off-by: Gwenole Beauchesne <gwenole.beauchesne@intel.com>