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 738984 - basetextoverlay: segfault for min/max values of element properties
basetextoverlay: segfault for min/max values of element properties
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
unspecified
Other Linux
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-10-22 04:19 UTC by Vineeth
Modified: 2015-03-29 14:19 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed patch (2.31 KB, patch)
2014-10-22 04:19 UTC, Vineeth
needs-work Details | Review
basetextoverlay segfault fix (1.60 KB, patch)
2014-10-23 09:20 UTC, Vineeth
rejected Details | Review
video-blend segfault fix (1.18 KB, patch)
2014-10-23 09:21 UTC, Vineeth
committed Details | Review

Description Vineeth 2014-10-22 04:19:23 UTC
Created attachment 289101 [details] [review]
proposed patch

In basetextoverlay added
*ypos = 0;
   Since setting ypos is added in else case, there are cases where ypos has a
junk value when the else condition is not satisfied. Hence initializing the value to 0 in the beginning.

 In Video-blend changes
Right now the condition is like this
   if (x + src_width > dest_width)
     if (dest_width > x)
       src_width = dest_width - x;
   But with this chances are there when src_width will not be re-calculated and it will be more that the width of the video, which results in segfault.
 The better way is to set the *xpos and *ypos to 0 in basetextoverlay, in case they lie outside the video width and height, since anyways we are not going to show the same.

  Please check if they are valid.
Comment 1 Sebastian Dröge (slomo) 2014-10-23 08:28:48 UTC
Can you split this patch into two and create a testcase?
Comment 2 Sebastian Dröge (slomo) 2014-10-23 08:29:45 UTC
Review of attachment 289101 [details] [review]:

::: gst-libs/gst/video/video-blend.c
@@ -331,3 @@
   if (y + src_height > dest_height)
-    if (dest_height > y)
-      src_height = dest_height - y;

Can't this become negative now?
Comment 3 Vineeth 2014-10-23 09:20:17 UTC
Created attachment 289190 [details] [review]
basetextoverlay segfault fix

Splitting the patch into two

i have been testing with below two test cases.. 
gst-launch-1.0 filesrc location=../wonder.mp4 ! decodebin ! videoconvert ! textoverlay deltay=-2147483648 text=text ! videoconvert ! ximagesink

gst-launch-1.0 filesrc location=../wonder.mp4 ! decodebin ! videoconvert ! textoverlay deltax=-2147483648 text=adasldjalskdjlsakjdklasjdlkjsalkdjslakdjklasjdlkasjdlkasjdklasjdkljaskldjaslkjdlkasjdlkasjdklasjdkasjdlkasjdlkajskdljaslkdjalksjdlkasjdlkasjdlkasjdlksajdlkjaskdljaskldjlaksjdlksajdalksjdaslkjdklasjdlkasjdlksajdmake ! videoconvert ! ximagesink


in video-blend.c
I am just reverting the changes done in last submission.
x position is calculated in basetextoverlay, such that when x crosses the dest width, it is reset to 0..

This should be taken care by all elements which uses video-blend.
This is similar to how src width can never be greater than dest width, which should again be taken care by all elements using video-blend

PS:IRC is not accesible in office due to proxy issues :(. Else it would be easier to explain..
Comment 4 Vineeth 2014-10-23 09:21:02 UTC
Created attachment 289191 [details] [review]
video-blend segfault fix
Comment 5 Luis de Bethencourt 2014-10-27 10:31:31 UTC
After some consideration, I agree this is a better solution.

Sorry about my bad patch.

Merged these two patches.
Comment 6 Luis de Bethencourt 2014-10-27 10:32:04 UTC
Comment on attachment 289190 [details] [review]
basetextoverlay segfault fix

Accepted
Comment 7 Luis de Bethencourt 2014-10-27 10:32:20 UTC
Comment on attachment 289191 [details] [review]
video-blend segfault fix

accepted
Comment 8 Tim-Philipp Müller 2015-03-29 14:19:44 UTC
Comment on attachment 289190 [details] [review]
basetextoverlay segfault fix

(This was reverted later, marking as such)