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 762717 - mpegtsmux: m2ts-mode crashes (abort)
mpegtsmux: m2ts-mode crashes (abort)
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.10.0
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-02-26 10:57 UTC by Tobias Modschiedler
Modified: 2016-10-31 14:33 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fix buffer size mismatch (2.60 KB, patch)
2016-03-18 14:13 UTC, Vincent Penquerc'h
none Details | Review
fix buffer size mismatch (2.60 KB, patch)
2016-10-14 09:48 UTC, Vincent Penquerc'h
committed Details | Review

Description Tobias Modschiedler 2016-02-26 10:57:30 UTC
The following pipeline crashes with SIGABORT:

> gst-launch-1.0 filesrc location=big_buck_bunny_720p_1mb.mp4 ! qtdemux ! queue ! h264parse ! mpegtsmux m2ts-mode=true ! filesink location=test.ts

Setting pipeline to PAUSED ...
Pipeline is PREROLLING ...

(gst-launch-1.0:5193): GStreamer-CRITICAL **: gst_buffer_resize_range: assertion 'bufmax >= bufoffs + offset + size' failed

(gst-launch-1.0:5193): GStreamer-CRITICAL **: gst_buffer_resize_range: assertion 'bufmax >= bufoffs + offset + size' failed
**
ERROR:mpegtsmux.c:1656:new_packet_m2ts: assertion failed: (out_buf)
zsh: abort (core dumped)  gst-launch-1.0 filesrc location=big_buck_bunny_720p_1mb.mp4 ! qtdemux ! queue


Without m2ts-mode, things go smoothly:

> gst-launch-1.0 filesrc location=big_buck_bunny_720p_1mb.mp4 ! qtdemux ! queue ! h264parse ! mpegtsmux m2ts-mode=false ! filesink location=test.ts

Setting pipeline to PAUSED ...
Pipeline is PREROLLING ...
Pipeline is PREROLLED ...
Setting pipeline to PLAYING ...
New clock: GstSystemClock
Got EOS from element "pipeline0".
Execution ended after 0:00:00.008166108
Setting pipeline to PAUSED ...
Setting pipeline to READY ...
Setting pipeline to NULL ...
Freeing pipeline ...

The file used for testing was downloaded from here: http://www.sample-videos.com/video/mp4/720/big_buck_bunny_720p_1mb.mp4

This was tested on Arch Linux with Linux 4.4.1-2-ARCH
Comment 1 Sebastian Dröge (slomo) 2016-02-26 11:34:41 UTC
Problem is that in new_packet_cb() for whatever reason the buffer only contains 188 bytes of memory although it initially was allocated with 192. Something probably reallocated it at some point.


Simpler testcase:
> gst-launch-1.0 videotestsrc ! x264enc ! h264parse ! mpegtsmux m2ts-mode=true ! fakesink
Comment 2 Vincent Penquerc'h 2016-03-17 16:54:32 UTC
The buffer doesn't seem to be allocated with 192 AFAICT. It's constructed in a roundabout way in tsmux_section_write_packet, which creates it with 188 bytes (g_malloc at line 884). That memory is used to create a GstMemory at line 904. There is some prepending later on. A test adding 4 to the malloc and the max size at both those locations doesn't work, the buffer is still seen as 188 bytes.
Comment 3 Vincent Penquerc'h 2016-03-17 17:02:02 UTC
er yes, this is normal it's still seen as 188 bytes. Still, it doesn't seem to allow resize to 192 even with those 4 extra slack bytes.
Comment 4 Vincent Penquerc'h 2016-03-17 17:08:50 UTC
Ah, that'd likely be because that memory is prepended, so I guess it's the last memory on a buffer that needs to have the slack for a resize to work.
Comment 5 Vincent Penquerc'h 2016-03-17 17:14:19 UTC
Right, some hack like this makes it work:

@@ -913,8 +914,15 @@ tsmux_section_write_packet (GstMpegtsSectionType * type,
     /* Prepend the header to the section data */
     gst_buffer_prepend_memory (packet_buffer, mem);
 
+{
+void *p = g_malloc(4);
+GstMemory *extra = gst_memory_new_wrapped (GST_MEMORY_FLAG_READONLY, p, 4, 0, 0, p, g_free);
+gst_buffer_append_memory (packet_buffer, extra);
+}
+
     TS_DEBUG ("Writing %d bytes to section. %d bytes remaining",
         len, section->pi.stream_avail - len);

I've no idea what m2ts mode is, but would something like that be an OK fix if it was done only if the muxer is in m2ts mode ?
Comment 6 Sebastian Dröge (slomo) 2016-03-17 17:58:11 UTC
The real fix would be to allocate 192 bytes :) Maybe even always and resizing down if non-m2ts mode.

The extra memory in your hack-fix is going to cause a lot of overhead.
Comment 7 Vincent Penquerc'h 2016-03-18 11:58:26 UTC
The last chunk in this buffer is from a gst_buffer_copy_region though, not a straight alloc, so in order to just have 4 more bytes there, you'd need to ensure the original buffer always has an extra 4 bytes available (and copy 4 extra bytes, then resize the buffer down, ensuring there's at least 4 slack bytes there). Am I missing a simpler way ?
Comment 8 Sebastian Dröge (slomo) 2016-03-18 12:28:48 UTC
Indeed. It's all awful :)
Comment 9 Vincent Penquerc'h 2016-03-18 14:13:42 UTC
Created attachment 324273 [details] [review]
fix buffer size mismatch

Well, here's a cleaned up version. It tries to get the bytes from the source buffer if it can.
Comment 10 Vincent Penquerc'h 2016-10-14 09:48:58 UTC
Created attachment 337695 [details] [review]
fix buffer size mismatch

Update to latest tree.
Comment 11 Vincent Penquerc'h 2016-10-17 11:37:32 UTC
commit 1630d2a0d7228765a169bbca4a745a46a9484e4a
Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk>
Date:   Fri Mar 18 14:09:37 2016 +0000

    mpegtsmux: fix buffer size mismatch in M2TS mode
    
    In M2TS mode, we need an extra 4 bytes in the buffer, so need
    to ensure the buffer can contain these. The allocation site
    does not know the mode, so this is done in all cases.