GNOME Bugzilla – Bug 738242
textoverlay: segfault when trying to position text outside of the video frame
Last modified: 2014-11-17 09:57:33 UTC
How to reproduce: Take any small video ( width smaller than 1000 ) or create one like this : gst-launch-1.0 videotestsrc num-buffers=100 ! video/x-raw,width=852,height=480 ! x264enc ! mp4mux ! filesink location=testsrc.mp4 And produce segfault with this : gst-launch-1.0 filesrc location=testsrc.mp4 ! qtdemux ! h264parse ! avdec_h264 ! textoverlay deltax="1001" text="test" ! fakesink
Created attachment 288182 [details] [review] basetextoverlay segfault fix This is happening because, when xpos is being calculated, it is exceeding the size of the video frame Added a check such that when the pos of x or pos of y exceeds the size, it automatically adjust to the max possible position of x/y Please check if this is a valid fix
Actually, eventhough this patch is correct it creates the wrong behaviour. If the text overlay is set outside the frame, we should respect that and instead of defaulting to the center we should not show the text at all. For example if the deltax or deltay is based on an animation that slides the overlay outside of the frame.
Created attachment 288228 [details] [review] patch to clip the text if out of the frame Alternative patch to clip the text if positioned out of the frame.
Created attachment 288231 [details] [review] If X position out of the frame don't evaluate Y position
Created attachment 288250 [details] [review] Remove the line where we set x to 0 In the previous line I resetted x to 0 to avoid hitting a bug in videoblend which was the actual reason of the segfault reported. But I don't need this anymore since I am fixing that bug in the next attachment.
Created attachment 288251 [details] [review] Fix issue in videoblend See previous comment.
Merged fixing patches.
Comment on attachment 288250 [details] [review] Remove the line where we set x to 0 Merged.
Comment on attachment 288251 [details] [review] Fix issue in videoblend Merged.
Created attachment 288499 [details] [review] Patch Hi Luis, The above patches still does not fix the crash properly. I am adding a patch which i think fixes all the crashes related to x and y pos Added *xpos = 0; *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 both the values to 0 in the beginning. In Video-blend changes "if (x + src_width > dest_width)" if this condition is satisfied, there is no way, dest_width will be > src_width which means the condition added in the previous patch is not useful. The better way is to set the *xpos and *ypos to 0, 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. Regards, Vineeth
commit 46e727ebdeaf1379b32cbf99e22bfa7788c37bd8 Author: Tim-Philipp Müller <tim@centricular.com> Date: Wed Nov 5 21:52:44 2014 +0000 Revert "basetextoverlay: Fix segfault when overlay outside the frame" This is not correct. overlay->silent is a property and we should not just flip the property forever because one text we render is outside of the frame. The next one might not be, the positioning properties can be changed after all. The lower layers should handle clipping, and now do. This reverts commit 1cc311156cc3908d1d9888fbcda67305fc647337. https://bugzilla.gnome.org/show_bug.cgi?id=738984 https://bugzilla.gnome.org/show_bug.cgi?id=739281 commit a003423bc35f04f061479a8dba26a958b40c82ac Author: Tim-Philipp Müller <tim@centricular.com> Date: Wed Nov 5 21:46:47 2014 +0000 Revert "basetextoverlay: segfault when xpos >= video size" This is not right, even if it might avoid a crash. We don't want to just set xpos/ypos to 0 in those cases. Clipping should be done properly, see bug #739281 for that. This reverts commit 900d0267d511e9553eec44d948d7e33ead7dc903. https://bugzilla.gnome.org/show_bug.cgi?id=738984 https://bugzilla.gnome.org/show_bug.cgi?id=739281