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 582238 - [videobox] Add support for autocrop to caps
[videobox] Add support for autocrop to caps
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other All
: Normal enhancement
: 0.10.17
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2009-05-11 21:00 UTC by Stephen Jungels
Modified: 2009-08-14 12:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch against gstvideobox.c which adds autocrop feature (9.59 KB, patch)
2009-05-11 21:01 UTC, Stephen Jungels
needs-work Details | Review
Updated autocrop patch (9.71 KB, patch)
2009-05-19 00:21 UTC, Stephen Jungels
committed Details | Review

Description Stephen Jungels 2009-05-11 21:00:31 UTC
When the patch is attached, this enhancement will add support for an "autocrop" property to the videobox element.  Setting autocrop=1 causes videobox to calculate positive or negative crop values such that a video stream with the width and height specified by upstream caps will be centered in a video stream with the width and height specified by downstream caps.
Comment 1 Stephen Jungels 2009-05-11 21:01:56 UTC
Created attachment 134440 [details] [review]
Patch against gstvideobox.c which adds autocrop feature
Comment 2 Sebastian Dröge (slomo) 2009-05-13 15:16:28 UTC
Some comments:

- Use a boolean property
- Don't use // comments, use /**/
- You don't call gst_base_transform_reconfigure() anymore, this should really be called ;)
Comment 3 Stephen Jungels 2009-05-19 00:21:32 UTC
Created attachment 134903 [details] [review]
Updated autocrop patch

Thanks for the code review, fixes added.  In the case of gst_base_transform_reconfigure(), this patch was initially written against an earlier version of videobox which did not make that call, and my test case freezes when the call is added.  I am leaving it out for now, advice sought.
Comment 4 Tim-Philipp Müller 2009-08-01 19:19:19 UTC
Re-opening, since updated patch available.

(Note: the "Setting autocrop to true..." comment should be incorporated into the gtk-doc blurb, so it shows up in the html plugin description and/or in a property-specific gtk-doc blurb for the autocrop property; the latter also needs to be added so we can put a 'Since: 0.10.xyz' marker there.)
Comment 5 Sebastian Dröge (slomo) 2009-08-14 11:46:46 UTC
I'll push this after 0.10.15 release.

commit e7402664c292dbcd1a7ca4da789fed90bd22dc31
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Fri Aug 14 13:45:22 2009 +0200

    videobox: Correctly add to the docs

commit c11c79fbac322cc225b49b4712cc41431ca9e27e
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Fri Aug 14 13:40:21 2009 +0200

    videobox: Split declarations into a header file and add autocrop stuff to th

commit 374e24d4b71916518da7158d5af5f1167829afce
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Fri Aug 14 13:26:36 2009 +0200

    videobox: Reconfigure basetransform if something changes again
    
    For this invent a new lock and don't abuse the basetransform lock,
    otherwise we'll end up in deadlocks.

commit 606d729237fe88d0f2042af3ff3c528dbbc9f195
Author: Stephen Jungels <stephen@jungels.net>
Date:   Fri Aug 14 13:15:57 2009 +0200

    videobox: Add support for autocropping according to the caps
    
    Fixes bug #582238.
Comment 6 Tim-Philipp Müller 2009-08-14 12:03:59 UTC
> I'll push this after 0.10.15 release.

I think you mean after the 0.10.16 release ;)