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 628548 - [mpegtsmux] Initialize PES packet before getting the header size
[mpegtsmux] Initialize PES packet before getting the header size
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other All
: Normal normal
: 0.10.21
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-09-01 20:37 UTC by Andoni Morales
Modified: 2010-09-04 18:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Initialize PES packet before getting the header size (5.63 KB, patch)
2010-09-01 20:37 UTC, Andoni Morales
committed Details | Review

Description Andoni Morales 2010-09-01 20:37:05 UTC
Created attachment 169291 [details] [review]
Initialize PES packet before getting the header size

The PES header length is calculated before setting the dynamic flags, returning    a wrong value. Small frames (audio ones) that should be sent in a single TS packet are spawned to a new packet because of that error. For audio streams where a single frame can cope in one TS packet it introduces a huge overhead.
    
In tsmux_write_stream_packet(), for a 100B packet, we prepare a TS packet with a payload of(100+9)B. Then, we write the TS header using this value in tsmux_write_ts_header(), and we call tsmux_stream_get_data(). The dynamic flags where not set yet (get_data() does it), and now tsmux_stream_pes_header_length() returns 14B instead of 9B. The payload of the TS packet is 114B, 5B more than what was calculated. 109B are sent in a first packet and the remaining 5B are sent in another one.

Using the following pipeline :
gst-launch audiotestsrc num-buffers=1000 ! ffenc_aac bitrate=30000 ! mpegtsmux ! filesink location=test.ts

:~gstreamer/head/gst-plugins-bad (mpegts)$ ls -al ts*
-rw-r--r-- 1 andoni andoni 263952 2010-09-01 22:01 /home/andoni/tspost.ts
-rw-r--r-- 1 andoni andoni 451764 2010-09-01 22:00 /home/andoni/tspre.ts

This is 58% less bandwith use, from 155kbps to 91kbps
Comment 1 Andoni Morales 2010-09-01 20:52:16 UTC
the gain this case is 42% and not 58% :P
Comment 2 Sebastian Dröge (slomo) 2010-09-02 08:30:50 UTC
Looks good, I'll push this after release. Thanks for the patch
Comment 3 Sebastian Dröge (slomo) 2010-09-04 13:17:42 UTC
commit 4668330bdc810256027637cf9aac3178e769d6b9
Author: Andoni Morales Alastruey <ylatuya@gmail.com>
Date:   Wed Sep 1 22:05:43 2010 +0200

    mpegtsmux: Initialize PES packet before getting the header size.
    
    The PES header length is calculated before setting the dynamic flags, retur
    a wrong value. Small frames that should be sent in a single TS packet are
    spawned to a new packet because of that error. For audio streams where a si
    frame can cope in one TS packet it introduces a huge overhead.
    
    For a 100B packet, we prepare a TS packet with a payload of(100+9)B. Then, 
    write the TS header using this value in tsmux_write_ts_header, and call
    tsmux_stream_get_data(). The dynamic flags where not set yet and now
    tsmux_stream_pes_header_length() returns 14B instead of 9B. The payload of 
    TS packet is 114B, 5B more than what was calculated. 109B are sent in a fir
    packet and the remaining 5B are sent in another one.
    
    Fixes bug #628548.