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 759332 - vaapisink segfaults with corrupted buffers
vaapisink segfaults with corrupted buffers
Status: RESOLVED FIXED
Product: gstreamer-vaapi
Classification: Other
Component: general
0.6.0
Other Linux
: Normal major
: ---
Assigned To: gstreamer-vaapi maintainer(s)
gstreamer-vaapi maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2015-12-11 08:58 UTC by Florent Thiéry
Modified: 2016-01-19 11:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
corrupted frames (gdp) (823.62 KB, application/octet-stream)
2015-12-11 14:03 UTC, Florent Thiéry
  Details
vaapisink: halt if buffer upload fails (1.41 KB, patch)
2016-01-13 18:53 UTC, Víctor Manuel Jáquez Leal
none Details | Review
vaapisink: ignore frame if its upload failed (1.63 KB, patch)
2016-01-14 10:32 UTC, Víctor Manuel Jáquez Leal
committed Details | Review

Description Florent Thiéry 2015-12-11 08:58:44 UTC
The v4l2src is a UVC device; not sure it is related but the crash does not happen with videotestsrc.

gst-vaapi version is 0.6.1 (didn't find it in the bugzilla version list).

Without the tee, having no problems; with the tee, crashing ~70% of the time.

gst-launch-1.0 v4l2src ! video/x-raw\,\ format\=\(string\)YUY2\,\ width\=\(int\)1920\,\ height\=\(int\)1080\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ framerate=\(fraction\)60/1 ! tee name=tee ! autovideosink

$ gdb gst-launch-1.0
(gdb) run v4l2src ! video/x-raw\,\ format\=\(string\)YUY2\,\ width\=\(int\)1920\,\ height\=\(int\)1080\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ framerate=\(fraction\)60/1 ! tee name=tee ! autovideosink
Starting program: /usr/bin/gst-launch-1.0 v4l2src ! video/x-raw\,\ format\=\(string\)YUY2\,\ width\=\(int\)1920\,\ height\=\(int\)1080\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ framerate=\(fraction\)60/1 ! tee name=tee ! autovideosink
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/libthread_db.so.1".
Définition du pipeline à PAUSED...
libva info: VA-API version 0.38.0
libva info: va_getDriverName() returns 0
libva info: Trying to open /usr/lib/dri/i965_drv_video.so
libva info: Found init function __vaDriverInit_0_38
libva info: va_openDriver() returns 0
[New Thread 0x7fffed22c700 (LWP 32507)]
Le pipeline est actif et n'a pas besoin de phase PREROLL…
[New Thread 0x7fffeca2b700 (LWP 32508)]
Passage du pipeline à la phase PLAYING…
New clock: GstSystemClock
[New Thread 0x7fffe7fff700 (LWP 32509)]
** (gst-launch-1.0:32503): CRITICAL **: gst_buffer_get_vaapi_video_meta: assertion 'GST_IS_BUFFER (buffer)' failed
** (gst-launch-1.0:32503): CRITICAL **: gst_vaapi_video_meta_get_display: assertion 'GST_VAAPI_IS_VIDEO_META (meta)' failed
** (gst-launch-1.0:32503): CRITICAL **: gst_vaapi_video_meta_get_surface_proxy: assertion 'GST_VAAPI_IS_VIDEO_META (meta)' failed
Program received signal SIGSEGV, Segmentation fault.

Thread 140737171867392 (LWP 32507)

  • #0 gst_mini_object_unref
    from /usr/lib/libgstreamer-1.0.so.0
  • #1 ??
    from /usr/lib/gstreamer-1.0/libgstvaapi.so
  • #2 gst_base_sink_do_preroll
    from /usr/lib/libgstbase-1.0.so.0
  • #3 ??
    from /usr/lib/libgstbase-1.0.so.0
  • #4 ??
    from /usr/lib/libgstbase-1.0.so.0
  • #5 ??
    from /usr/lib/libgstbase-1.0.so.0
  • #6 ??
    from /usr/lib/libgstreamer-1.0.so.0
  • #7 gst_proxy_pad_chain_default
    from /usr/lib/libgstreamer-1.0.so.0
  • #8 ??
    from /usr/lib/libgstreamer-1.0.so.0
  • #9 ??
    from /usr/lib/gstreamer-1.0/libgstcoreelements.so
  • #10 ??
    from /usr/lib/gstreamer-1.0/libgstcoreelements.so
  • #11 ??
    from /usr/lib/libgstreamer-1.0.so.0
  • #12 ??
    from /usr/lib/libgstbase-1.0.so.0
  • #13 ??
    from /usr/lib/libgstreamer-1.0.so.0
  • #14 ??
    from /usr/lib/libgstbase-1.0.so.0
  • #15 ??
    from /usr/lib/libgstreamer-1.0.so.0
  • #16 ??
    from /usr/lib/libglib-2.0.so.0
  • #17 ??
    from /usr/lib/libglib-2.0.so.0
  • #18 start_thread
    from /usr/lib/libpthread.so.0
  • #19 clone
    from /usr/lib/libc.so.6

Comment 1 Florent Thiéry 2015-12-11 10:47:16 UTC
I also experimented with adding vaapipostproc before autovideosink; this time, i am getting a random Internal data flow error from elements v4l2src (also a race).

GST_DEBUG=vaapi*:5 gst-launch-1.0 v4l2src ! video/x-raw\,\ format\=\(string\)YUY2\,\ width\=\(int\)1920\,\ height\=\(int\)1080\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ framerate=\(fraction\)60/1 ! tee name=tee ! vaapipostproc format=i420 ! autovideosink -v

0:00:00.197262852  5678       0xf8c9e0 WARN           vaapipostproc gstvaapipluginbase.c:914:gst_vaapi_plugin_base_get_input_buffer: failed to map buffer
ERROR: from element /GstPipeline:pipeline0/GstV4l2Src:v4l2src0: Internal data flow error.
gstbasesrc.c(2943): gst_base_src_loop (): /GstPipeline:pipeline0/GstV4l2Src:v4l2src0:
streaming task paused, reason error (-5)

The v4l2src error intrigued me, so i tested with other v4l2 capture devices, and it seems to be device-related; i'll investigate more and report.

Thanks
Comment 2 Florent Thiéry 2015-12-11 13:19:23 UTC
Okay, just found out that it's also happening with glimagesink. Sorry for the noise
Comment 3 Florent Thiéry 2015-12-11 14:03:28 UTC
For reference, the problem was that the UVC v4l2src device is sending corrupt frames (incomplete), and the tee calls gst_video_frame_map_id which 

 0:00:00.179926758 11369      0x15c4280 ERROR                default video-frame.c:161:gst_video_frame_map_id: invalid buffer size 2621440 < 4608000


You can reproduce the crash with:
gst-launch-1.0 filesrc location=crash.gdp ! gdpdepay ! vaapisink
Comment 4 Florent Thiéry 2015-12-11 14:03:59 UTC
Created attachment 317209 [details]
corrupted frames (gdp)
Comment 5 Víctor Manuel Jáquez Leal 2016-01-13 18:53:38 UTC
Created attachment 318985 [details] [review]
vaapisink: halt if buffer upload fails

When gst_vaapi_plugin_base_get_input_buffer() fail to copy the input buffer
into a VAAPI buffer, the return value is GST_FLOW_NOT_SUPPORTED, and it was
ignored by the vaapisink, leading to segfaults.

This patch makes to return the value GST_FLOW_NOT_SUPPORTED if it was returned
by gst_vaapi_plugin_base_get_input_buffer(), avoiding the segmentation fault.

Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>
Comment 6 Víctor Manuel Jáquez Leal 2016-01-14 09:52:00 UTC
After sleep on it I wonder if this patch is enough.

1\ Shall we change gst_vaapi_plugin_base_get_input_buffer() to return a fatal error if we could not upload the input buffer completely, so as do most of the filters.

2\ Or, we should ignore that buffer an try to continue with the next one.
Comment 7 Víctor Manuel Jáquez Leal 2016-01-14 10:26:13 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #6)

> 2\ Or, we should ignore that buffer an try to continue with the next one.

Reading the code of ximagesink it ignores the frame and goes on.

Let's mimic that behavior.
Comment 8 Víctor Manuel Jáquez Leal 2016-01-14 10:32:56 UTC
Created attachment 319005 [details] [review]
vaapisink: ignore frame if its upload failed

When gst_vaapi_plugin_base_get_input_buffer() fail to copy the input buffer
into a VAAPI buffer, the return value is GST_FLOW_NOT_SUPPORTED, and it was
ignored by the vaapisink, leading to a segmentation fault.

This patch ignores the frame that generated the GST_FLOW_NOT_SUPPORTED
returned by gst_vaapi_plugin_base_get_input_buffer(), avoiding the
segmentation fault, but doing and effort to continue rendering. This is
the same behavior of ximagesink.

Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>
Comment 9 sreerenj 2016-01-19 06:18:55 UTC
Review of attachment 319005 [details] [review]:

::: gst/vaapi/gstvaapisink.c
@@ +1335,3 @@
       src_buffer, &buffer);
+  if (ret == GST_FLOW_NOT_SUPPORTED)
+    return GST_FLOW_OK; /* let's ignore the frame if it couldn't be uploaded */

I think we need a buffer_unref too, in case  buffer acquired from the pool but failed to map...right?
Comment 10 Víctor Manuel Jáquez Leal 2016-01-19 09:05:45 UTC
(In reply to sreerenj from comment #9)
> Review of attachment 319005 [details] [review] [review]:
> 
> ::: gst/vaapi/gstvaapisink.c
> @@ +1335,3 @@
>        src_buffer, &buffer);
> +  if (ret == GST_FLOW_NOT_SUPPORTED)
> +    return GST_FLOW_OK; /* let's ignore the frame if it couldn't be
> uploaded */
> 
> I think we need a buffer_unref too, in case  buffer acquired from the pool
> but failed to map...right?

That's already done in gst_vaapi_plugin_base_get_input_buffer(), but perhaps we should set the returned outbuf to NULL, after unref it.
Comment 11 Víctor Manuel Jáquez Leal 2016-01-19 11:48:00 UTC
Attachment 319005 [details] pushed as 8f77d54 - vaapisink: ignore frame if its upload failed