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 739145 - [regression] With GStreamer GLimagesink, changing the aspect ratio results in the sink having the wrong aspect ratio and being letterboxed until the widget containing the sink has been resized
[regression] With GStreamer GLimagesink, changing the aspect ratio results in...
Status: RESOLVED FIXED
Product: pitivi
Classification: Other
Component: Viewer
0.94
Other Linux
: Normal normal
: 0.94
Assigned To: Pitivi maintainers
Pitivi maintainers
Depends on:
Blocks:
 
 
Reported: 2014-10-24 20:30 UTC by Jean-François Fortin Tam
Modified: 2014-11-02 07:58 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
screencast (792.86 KB, application/octet-stream)
2014-10-24 20:32 UTC, Jean-François Fortin Tam 		  Details
viewer: Do not call the expose function when we set the aspect ratio (1.83 KB, patch)
2014-10-24 21:51 UTC, Thibault Saunier 		committed Details | Review

Description Jean-François Fortin Tam 2014-10-24 20:30:47 UTC 
The following commit, aiming to fix bug #737530, revealed a bug, probably some race in glimagesink:
-----------------
commit 28464252f80645c4aa1b372a5b0ac3c49f0b319e
Author: Thibault Saunier <tsaunier@gnome.org>
Date:   Fri Oct 24 21:24:40 2014 +0200

    viewer: Do not forget to set ViewerWidget sink when setting our pipeline
    
    Making the expose call working when we need to redraw the viewer.
    
    + Rename Pipeline._glimage_sink to Pipeline.videosink and making it
      public and simpler to read.
------------------

The bug is that it breaks the aspect ratio handling until you resize the viewer. To reproduce in Pitivi:
- Grab my usual "métro 1.MOV"
- Insert it to the timeiine
- Go to the clip properties dialog and apply project settings from it

Result: side-letterboxed and squished image, even when you seek in the timeline.
This does not affect the pipeline/rendered output.
This bug happens almost 100% of the time, but in very rare instances it does not occur.


Workaround: if you resize the Pitivi ViewerWidget containing the sink, the output gets corrected to fit the widget.
Comment 1 Jean-François Fortin Tam 2014-10-24 20:32:56 UTC 
Created attachment 289294 [details]
screencast

Demonstration of the problem
Comment 2 Thibault Saunier 2014-10-24 21:09:07 UTC 
The problem is happening when we call glimagesink.expose() here: https://git.gnome.org/browse/pitivi/tree/pitivi/viewer.py#n843

It seems to actually be a race because if right before calling expose() I add a time.sleep(1) (0.5 is enough), the bug does not happen.
Comment 3 Thibault Saunier 2014-10-24 21:51:54 UTC 
Created attachment 289297 [details] [review]
viewer: Do not call the expose function when we set the aspect ratio

During caps renogotiation resulting from the change of the rendering
setting/aspect ratio changes, we could end up with the viewer displaying
broken frames. Avoid calling the videosink.expose() while we change the
resolution in the pipeline
Comment 4 Thibault Saunier 2014-10-25 07:45:39 UTC 
Attachment 289297 [details] pushed as 8f252a4 - viewer: Do not call the expose function when we set the aspect ratio
Comment 5 Jean-François Fortin Tam 2014-10-26 03:53:00 UTC 
Don't you want to reassign this bug to GStreamer to fix the race in glimagesink?
Comment 6 Matthew Waters (ystreet00) 2014-11-02 02:06:18 UTC 
Most Probably fixed in glimagesink gst-plugins-bad master by

commit 0537cbfa5be452c5d9828a2e511f1fcba6d755c5
Author: Matthew Waters <matthew@centricular.com>
Date:   Fri Oct 31 12:52:50 2014 +1100

    glimagesink: resize the viewport correctly on a caps change
    
    with force-aspect-ratio=true, if the width or height changed, the
    viewport wasn't being updated to respect the new video width and height
    until a resize occured.

I'll backport that to 1.4 soon.
Comment 7 Thibault Saunier 2014-11-02 07:58:12 UTC 
Indeed it looks like that patch in glimagesink fixed our issue, I do not understand why it was racy then?