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 756207 - videoaggregator: Setting aspect ratio crops the frame
videoaggregator: Setting aspect ratio crops the frame
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
1.6.0
Other Linux
: Normal major
: 1.7.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 764363
 
 
Reported: 2015-10-07 21:46 UTC by Thibault Saunier
Modified: 2016-03-30 12:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
videoaggregator: don't do caps processing that is not overridable (31.71 KB, patch)
2015-10-14 11:42 UTC, Matthew Waters (ystreet00)
committed Details | Review

Description Thibault Saunier 2015-10-07 21:46:36 UTC
Implementing aspect ratio in:

commit 0871b7b43db796b50f432e4c892cca65c633b5ff
Author: Matthew Waters <matthew@centricular.com>
Date:   Thu May 14 13:04:21 2015 +1000

    compositor: implement proper par handling
    
    We were previously failing on different input and output par

pipelines like:

  gst-validate-1.0 uridecodebin uri=file:///$HOME/gst-validate/gst-integration-testsuites/medias/defaults/matroska/numerated_frames_blue.mkv ! videoscale ! videoconvert ! video/x-raw,pixel-aspect-ratio=64/45 ! compositor ! videoconvert ! autovideosink

outputs cropped and weirdly moved frames.
Comment 1 Matthew Waters (ystreet00) 2015-10-12 05:25:05 UTC
So the problem is that the caps negotiation code does not have the src pad peer query caps when "updating the caps" so does not have the overriden par to calculate the correct output width/height.  Ultimately the entire gst_videoaggregator_update_src_caps() function should be replaced by two vfuncs.  update_caps() and fixate_caps() with default implementations as they currently stand.
Comment 2 Matthew Waters (ystreet00) 2015-10-14 11:42:41 UTC
Created attachment 313256 [details] [review]
videoaggregator: don't do caps processing that is not overridable

So this is the fix everything approach which will not be reaching the 1.6 branch.  Setting the par on either the sink or the src pads now correctly outputs the correct size video in all cases I tested with using glvideomixer and compositor.  The other videoaggregator elements also seem to work fine (glstereomix, glmosaic)

@thiblahute: could you check your gst-validate line with this patch?
Comment 3 Alexander 2016-01-24 10:07:15 UTC
Matthew thanks for patch,

Could you please help me understand, to verify you patch I need:
1. download gst-plugin-bad
2. apply patch
3. prepare a build
4. test

Could you please confirm steps?

Which branch should I switch to to be able verity your fix?
Comment 4 Matthew Waters (ystreet00) 2016-01-27 10:04:31 UTC
commit 87031b14cb38075fe3ed168d477f1b682de92268
Author: Matthew Waters <matthew@centricular.com>
Date:   Wed Oct 14 21:13:57 2015 +1100

    videoaggregator: don't do caps processing that is not overridable
    
    Allows the subclass to completely override the chosen src caps.
    
    This is needed as videoaggregator generally has no idea exactly
    what operation is being performed.
    
    - Adds a fixate_caps vfunc for fixation
    - Merges gst_video_aggregator_update_converters() into
      gst_videoaggregator_update_src_caps() as we need some of its info
      for proper caps handling.
    - Pass the downstream caps to the update_caps vfunc
    
    https://bugzilla.gnome.org/show_bug.cgi?id=756207
Comment 5 Thibault Saunier 2016-03-30 12:26:40 UTC
That last patch breaks pipeline like:

gst-launch-1.0 videotestsrc num-buffers=30 pattern=solid-color foreground-color=0 ! videoconvert ! video/x-raw,format=RGBA !  textoverlay text='test yes yo' color=5293848812 valignment=center font-desc='Serif Italic 36' ! compositor !  videoconvert ! autovideosink

where the compositor should force an ALPHA channel downstream as upstream has one so that. Currently instead of showing the compositor checkerboard as expected it shows the text over a black frame.... which totally breaks titles in pitivi.
Comment 6 Sebastian Dröge (slomo) 2016-03-30 12:37:00 UTC
Let's track this in a new bug for proper target milestone handling.