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 736965 - matroskademux: Output raw video in unaligned buffers causing crashing in ORC video conversion code
matroskademux: Output raw video in unaligned buffers causing crashing in ORC ...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
1.4.1
Other Linux
: Normal blocker
: 1.5.91
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-09-19 11:31 UTC by Jan Alexander Steffens (heftig)
Modified: 2015-08-31 11:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH] matroskademux: Align raw video frames to 32 bytes (1.07 KB, patch)
2015-08-31 11:20 UTC, Jan Alexander Steffens (heftig)
committed Details | Review

Description Jan Alexander Steffens (heftig) 2014-09-19 11:31:04 UTC
Given the following pair of pipelines:

gst-launch-1.0 videotestsrc ! video/x-raw,framerate=25/1,width=1280,height=720,format=AYUV ! matroskamux ! shmsink socket-path=/tmp/foo

gst-launch-1.0 shmsrc socket-path=/tmp/foo ! matroskademux ! videoconvert ! video/x-raw,format=YUY2 ! fakesink

The latter causes a segfault in the orc-generated code in videoconvert:

Thread 3 (Thread 0x7f13201fb700 (LWP 11294))

  • #0 nanosleep
    from /usr/lib/libpthread.so.0
  • #1 g_usleep
    at gtimer.c line 259
  • #2 fault_spin
    at gst-launch.c line 109
  • #3 fault_handler_sighandler
    at gst-launch.c line 90
  • #4 <signal handler called>
  • #5 ??
  • #6 video_convert_orc_convert_AYUV_YUY2
    at tmp-orc.c line 4190
  • #7 gst_video_convert_transform_frame
    at gstvideoconvert.c line 574
  • #8 gst_video_filter_transform
    at gstvideofilter.c line 270
  • #9 gst_base_transform_handle_buffer
    at gstbasetransform.c line 2117
  • #10 gst_base_transform_chain
    at gstbasetransform.c line 2224
  • #11 gst_pad_chain_data_unchecked
    at gstpad.c line 3836
  • #12 gst_pad_push_data
    at gstpad.c line 4069
  • #13 gst_matroska_demux_parse_blockgroup_or_simpleblock
    at matroska-demux.c line 3593
  • #14 gst_matroska_demux_parse_id
    at matroska-demux.c line 4237
  • #15 gst_matroska_demux_chain
    at matroska-demux.c line 4565
  • #16 gst_pad_chain_data_unchecked
    at gstpad.c line 3836
  • #17 gst_pad_push_data
    at gstpad.c line 4069
  • #18 gst_base_src_loop
    at gstbasesrc.c line 2835
  • #19 gst_task_func
    at gsttask.c line 317
  • #20 g_thread_pool_thread_proxy
    at gthreadpool.c line 307
  • #21 g_thread_proxy
    at gthread.c line 764
  • #22 start_thread
    from /usr/lib/libpthread.so.0
  • #23 clone
    from /usr/lib/libc.so.6

Removing either the shm transport, the muxing, or the orc-accelerated converting (ORC_CODE=backup) makes the segfault disappear.

Could this be a problem with orc working on unaligned buffers in the shm segment due to the tiny metadata buffers matroskamux injects?
Comment 1 Nicolas Dufresne (ndufresne) 2014-09-19 12:54:23 UTC
In 1.2 it cleanly fail with this error:

ERROR: from element /GstPipeline:pipeline0/GstShmSink:shmsink0: Shared memory area is too small
Additional debug info:
Shared memory area of size 262144 is smaller thanbuffer of size 368640

And fixing the size makes GstVideoFrame fails t map buffer, invalid size. Deep down, I think matroska might not be setting raw buffers information correctly, or is missing some required VideoMeta maybe.
Comment 2 Jan Alexander Steffens (heftig) 2014-12-08 13:00:17 UTC
Seems to be a problem with matroskademux not creating aligned video buffers if it is pushed a whole frame in a single buffer. Tested using the following setup:

Create a test file:

gst-launch-1.0 videotestsrc num-buffers=100 ! video/x-raw,format=I420 ! matroskamux ! filesink location=foo.mkv

With a small blocksize matroskademux creates perfectly sized and aligned buffers and everything is fine:

GST_DEBUG="GST_MEMORY:LOG" gst-launch-1.0 pushfilesrc real-filesrc::blocksize=4096 real-filesrc::location=foo.mkv ! matroskademux ! videoconvert ! video/x-raw,format=UYVY ! fakesink

gstmemory.c:137:gst_memory_init: new memory 0x13ff640, maxsize:115208 offset:0 size:115208

With a blocksize large enough to capture an entire frame, matroskademux creates buffers using unaligned child memory and videoconvert crashes:

GST_DEBUG="GST_MEMORY:LOG" gst-launch-1.0 pushfilesrc real-filesrc::blocksize=256000 real-filesrc::location=foo.mkv ! matroskademux ! videoconvert ! video/x-raw,format=UYVY ! fakesink

gstmemory.c:137:gst_memory_init: new memory 0x7fd5ac0061c0, maxsize:256007 offset:437 size:115200
Comment 3 Jan Alexander Steffens (heftig) 2014-12-08 13:06:44 UTC
If the video frame size is small enough, even the default blocksize causes problems:

gst-launch-1.0 videotestsrc num-buffers=100 ! video/x-raw,format=I420,width=32,height=24 ! matroskamux ! filesink location=foo.mkv

GST_DEBUG="GST_MEMORY:LOG" gst-launch-1.0 filesrc location=foo.mkv ! matroskademux ! videoconvert ! video/x-raw,format=UYVY ! fakesink
Comment 4 Jan Alexander Steffens (heftig) 2015-01-21 08:39:39 UTC
So I guess the thing to do here would be to teach the video converters to copy into an aligned buffer if necessary?
Comment 5 Jan Alexander Steffens (heftig) 2015-03-10 10:43:31 UTC
Any comment? I know there has already been a lot of change to the video converting code in the 1.5 cycle.
Comment 6 Jan Alexander Steffens (heftig) 2015-05-12 08:19:01 UTC
The pipeline from comment 2 and 3 don't crash here anymore, but this latter one does:

gst-launch-1.0 videotestsrc num-buffers=100 ! video/x-raw,format=AYUV,width=32,height=24 ! matroskamux ! filesink location=foo.mkv

GST_DEBUG="GST_MEMORY:LOG" gst-launch-1.0 filesrc location=foo.mkv ! matroskademux ! videoconvert ! video/x-raw,format=YUY2 ! fakesink
Comment 7 Jan Alexander Steffens (heftig) 2015-08-26 13:16:47 UTC
Since 1.5.90 I'm seeing this with pipes and big frames, too.

gst-launch-1.0 -q videotestsrc ! video/x-raw,format=I420,width=1920,height=1080 ! matroskamux streamable=1 ! fdsink fd=1 | gst-launch-1.0 fdsrc fd=0 ! matroskademux ! videoscale ! video/x-raw,width=960,height=540 ! fakesink
Comment 8 Jan Alexander Steffens (heftig) 2015-08-26 13:47:18 UTC
It seems I've found a workaround:

diff --git i/gst/matroska/matroska-demux.c w/gst/matroska/matroska-demux.c
index 1d84abc..01a65ff 100644
--- i/gst/matroska/matroska-demux.c
+++ w/gst/matroska/matroska-demux.c
@@ -5012,6 +5012,7 @@ gst_matroska_demux_video_caps (GstMatroskaTrackVideoContext *
         videocontext->pixel_height);
     caps = gst_video_info_to_caps (&info);
     *codec_name = gst_pb_utils_get_codec_description (caps);
+    context->alignment = 32;
   } else if (!strcmp (codec_id, GST_MATROSKA_CODEC_ID_VIDEO_MPEG4_SP)) {
     caps = gst_caps_new_simple ("video/x-divx",
         "divxversion", G_TYPE_INT, 4, NULL);
Comment 9 Sebastian Dröge (slomo) 2015-08-26 14:40:40 UTC
So the problem here looks like matroskademux never made the alignment correct before, and older videoconvert was just more forgiving :)
Comment 10 Sebastian Dröge (slomo) 2015-08-31 09:07:52 UTC
Jan, can you confirm that the problem is that matroskademux (in 1.4 and 1.5) outputs improperly aligned buffers and that the only problem is that in 1.5 the video converter code is a bit more picky about that?
Comment 11 Jan Alexander Steffens (heftig) 2015-08-31 10:35:58 UTC
I think it's definitely matroskademux which now outputs more unaligned buffers. Even with context->alignment = 8 it doesn't crash, and I see:

matroska-demux.c:3143:gst_matroska_demux_align_buffer:<matroskademux0> We want output aligned on 8, reallocated
Comment 12 Sebastian Dröge (slomo) 2015-08-31 10:43:46 UTC
I think it's reasonable to enforce 32 byte alignment for the raw video formats in matroskademux. Should check if the same is also needed in qtdemux and avidemux, they also support raw video.

Jan, can you attach a proper patch for matroskademux and shortly check those other demuxers? Thanks!
Comment 13 Jan Alexander Steffens (heftig) 2015-08-31 11:20:07 UTC
Created attachment 310344 [details] [review]
[PATCH] matroskademux: Align raw video frames to 32 bytes

Patch for the change above.

qtdemux and avidemux just take the buffers from a GstAdapter (in push mode) or pull a range (in pull mode) without checking aligment.
Comment 14 Sebastian Dröge (slomo) 2015-08-31 11:36:57 UTC
Ok, so avidemux and qtdemux are equally broken then just that nobody noticed yet. Want to provide patches? :)
Comment 15 Jan Alexander Steffens (heftig) 2015-08-31 11:39:47 UTC
So far I've been unable to get them to crash.
Comment 16 Tim-Philipp Müller 2015-08-31 11:41:49 UTC
We should do the other ones after 1.6 then, they're not regressions either as far as we know, are they?