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 687330 - videobox, videomixer: height obtained using _WIDTH macros
videobox, videomixer: height obtained using _WIDTH macros
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
unspecified
Other Linux
: Normal normal
: 1.0.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-11-01 10:52 UTC by Douglas Bagnall
Modified: 2012-11-01 21:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
a patch (2.08 KB, patch)
2012-11-01 10:53 UTC, Douglas Bagnall
committed Details | Review

Description Douglas Bagnall 2012-11-01 10:52:22 UTC
[from the patch comment]

In videomixer/blend.c, I found a couple of cut-and-pastos like this:

  dest_width = GST_VIDEO_FRAME_WIDTH (destframe); \
  dest_height = GST_VIDEO_FRAME_WIDTH (destframe); \

where the second one should almost certainly be using
GST_VIDEO_FRAME_HEIGHT, not _WIDTH.

Out of interest, I ran this:

$ git grep 'height.\+WIDTH'

and found a similar case in videobox.
Comment 1 Douglas Bagnall 2012-11-01 10:53:26 UTC
Created attachment 227789 [details] [review]
a patch
Comment 2 Douglas Bagnall 2012-11-01 11:06:39 UTC
For what its worth, similar grepping in my other gst* repos finds nothing that looks wrong.
Comment 3 Tim-Philipp Müller 2012-11-01 13:16:38 UTC
Thanks, pushed as separate patches (so I could describe what it actually might fix in case of videobox - don't know if it had any ill effects in case of videomixer - did it crash? cause artefacts?)

 commit e3c77ba709386acf03840252673d4519ff8c257c
 Author: Douglas Bagnall <douglas@paradise.net.nz>
 Date:   Thu Nov 1 13:03:44 2012 +0000

    videomixer: get height via GST_VIDEO_FRAME_HEIGHT, not _WIDTH
    
    https://bugzilla.gnome.org/show_bug.cgi?id=687330

 commit 79403bcb0c2f8a4fc89ed79a4416eb5119efc58f
 Author: Douglas Bagnall <douglas@paradise.net.nz>
 Date:   Thu Nov 1 13:02:16 2012 +0000

    videbox: fix border filling for gray formats
    
    Get the height via GST_VIDEO_FRAME_HEIGHT, not _WIDTH.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=687330
Comment 4 Douglas Bagnall 2012-11-01 21:49:55 UTC
(In reply to comment #3)
> Thanks, pushed as separate patches (so I could describe what it actually might
> fix in case of videobox - don't know if it had any ill effects in case of
> videomixer - did it crash? cause artefacts?)

To be honest, I was only reading it.  But what seems to happen is portrait-aligned video sources are vertically truncated to squares:

 gst-launch-1.0 videotestsrc pattern=1 ! \
  video/x-raw,format =I420, framerate=\(fraction\)10/1, width=50, height=200 ! \
  videomixer  name=mix ! videoconvert ! xvimagesink \
  videotestsrc ! \
  video/x-raw,format=I420, framerate=\(fraction\)5/1, width=100, height=400 ! mix.

The more serious variation is when the output frame size is constrained to a smaller landscape rectangle than videomixer would invent for itself. Like this:

 gst-launch-1.0 videotestsrc pattern=1 ! \
  video/x-raw,format =I420, framerate=\(fraction\)10/1, width=250, height=200 !\
  videomixer  sink_1::ypos=50   name=mix ! videoconvert ! \
  video/x-raw, width=200, height=200 ! xvimagesink \
  videotestsrc ! \
  video/x-raw,format=I420, framerate=\(fraction\)5/1, width=300, height=200 ! \
  mix.

That's a crash, because videomixer mistakes width for height and overruns the buffer.

Sorry for the mixed up patch.