GNOME Bugzilla – Bug 778337
qtdemux: sample_description_index not evaluated when parsing the stsd-box
Last modified: 2017-04-21 04:47:19 UTC
In function qtdemux_parse_trak() when the 'stsd' child is evaluated the current sample_description_index is not evaluated. There is alway the first entry used. The sample_description_index must be read from 'trex'-node (moov) and sometimes is overwritten by following 'tfhd' (moof) box. As of this failure the German DVB-T2 DASH streams (common init format) can not be decoded (wrong capabilies) when lower bitrates are used To reproduce use for example gst-play-1.0 with the stream: gst-play-1.0 http://akamai-progressive.irt.de/irt_avstandard_2014/M4S/1280x720p50/V-1280x720p50_libx264_high_yuv420p_gop100_bit3584k_term5-2-5-14-1/seg2_frag2_hbbtv15_live_multi/V-1280x720p50_libx264_high_yuv420p_gop100_bit3584k_term5-2-5-14-1.mpd
Meanwhile some more official German Internet Link service cannot be decoded caused by this problem (Delivered via DVB-T2 freenet.tv, MediaBroadcast) All this streams are using DASH and all higher bitrate representations cannot be decoded with GStreamer. http://freenettvcon017-i.akamaihd.net/dash/live/284555/freenet017/dash.mpd http://freenettvcon018-i.akamaihd.net/dash/live/284569/freenet018/dash.mpd http://freenettvcon012-i.akamaihd.net/dash/live/284514/freenet012/dash.mpd http://freenettvcon011-i.akamaihd.net/dash/live/284503/freenet011/dash.mpd http://freenettvcon004-i.akamaihd.net/dash/live/284470/freenet004/dash.mpd http://freenettvcon010-i.akamaihd.net/dash/live/284497/freenet010/dash.mpd http://freenettvcon007-i.akamaihd.net/dash/live/284480/freenet007/dash.mpd http://freenettvcon009-i.akamaihd.net/dash/live/284490/freenet009/dash.mpd http://freenettvcon005-i.akamaihd.net/dash/live/284475/freenet005/dash.mpd http://freenettvcon008-i.akamaihd.net/dash/live/284484/freenet008/dash.mpd http://freenettvcon003-i.akamaihd.net/dash/live/284425/freenet003/dash.mpd http://freenettvcon002-i.akamaihd.net/dash/live/284415/freenet002/dash.mpd http://freenettvcon006-i.akamaihd.net/dash/live/284478/freenet006/dash.mpd http://freenettvcon014-i.akamaihd.net/dash/live/284533/freenet014/dash.mpd http://freenettvcon013-i.akamaihd.net/dash/live/284524/freenet013/dash.mpd http://freenettvcon001-i.akamaihd.net/dash/live/281545/freenet001/dash.mpd http://freenettvcon016-i.akamaihd.net/dash/live/284548/freenet016/dash.mpd http://freenettvcon015-i.akamaihd.net/dash/live/284540/freenet015/dash.mpd
Created attachment 349480 [details] testcase to show the issue with a simple ISO-MP4 container file
Are you planning to work on this and provide a patch? From your description above it sounds like you exactly know what needs to be done already :)
Yes, but before starting, I would like to check that no other is already working at this issue at the same time.
I'm not aware of anybody working on this, go ahead :)
Unfortunately this appeares to become a bigger change deal. I need to access the stbl (in moov) in from inside the moof. At moof-parse-time the moov is already freed. To fix this, the whole stdt-parsing (a 2000 lines case switch) must be refactored out of the parse_trak() to be call-able from qtdemux_parse_tfhd().
Created attachment 349658 [details] [review] Try to implement a workaround to fix the problem I'm not really happy with this patch because it duplicates some code parts and bumps the code. Additionally when the sample_description_index changes, at least one unnecessary (wrong) caps record is sent downstream before the correct caps follows. Another lack is, that the patch works for HEVC and AVC type media only (this are the only formats used by HbbTV/DASH currently). It would be better to extract and separate the stsd-parsing from parse_trak() and call the same code from tfhd. But there is a lot of usage of other local variables that makes this difficultly. Currently we do not have the man-power and the detailed knowledge to dare such a deep refactoring/redesign of qtdemux.
Hello all Just for inform related thing. I also tried to solve this issue, and found what Thiago did on bug #729371 and his branch https://cgit.freedesktop.org/~thiagoss/gst-plugins-good/log/?h=qtdemux-stsd-all-entries It really works well and will be helpful :)
(In reply to Juergen Sachs from comment #7) > Created attachment 349658 [details] [review] [review] > Try to implement a workaround to fix the problem > > I'm not really happy with this patch because it duplicates some code parts > and bumps the code. Additionally when the sample_description_index changes, > at least one unnecessary (wrong) caps record is sent downstream before the > correct caps follows. Another lack is, that the patch works for HEVC and AVC > type media only (this are the only formats used by HbbTV/DASH currently). > It would be better to extract and separate the stsd-parsing from > parse_trak() and call the same code from tfhd. But there is a lot of usage > of other local variables that makes this difficultly. > Currently we do not have the man-power and the detailed knowledge to dare > such a deep refactoring/redesign of qtdemux. Nice work ! As Seungha mentioned, there is a "proper" refactor of the stsd parsing which you could base your patch on. It might make your patch a lot smaller.
Created attachment 349668 [details] [review] Patch to fix the problem based on Thiagios branch Thiagos branch makes it really easy to fix the problem. The patch is only 4 lines. Patch is to be applied on https://cgit.freedesktop.org/~thiagoss/gst-plugins-good/commit/?h=qtdemux-stsd-all-entries&id=09fa35ce0d6b7f1eca431846479150673723a0a6 Is it planned to merge Thiagos work to master?
commit 9684c88c601482f99697f25d6bcec5165867c0e5 Author: Jürgen Sachs <juergen.sachs@metz-ce.de> Date: Tue Apr 11 13:41:48 2017 +0200 qtdemux: fix: sample description index override in tfhd not evaluated https://bugzilla.gnome.org/show_bug.cgi?id=778337
Sadly the original stream (in the first comment) still fails.
This (the original) stream does not contain a sample_description_index override in the tfhd-box for the "default" representation (with the highest bitrate). When dashdemux selects this representation the last stsd_sample_description_id keeps present and active. Let me check where to reset this to default.
Created attachment 350106 [details] [review] fixes the problem still occurs with original stream This patch resets the sample_description_id before checking tfhd-flags.
Created attachment 350107 [details] [review] fixes the problem still occurs with original stream remove a warning regarding unused variable
Thanks ! Small nitpicks for the future: * Make sure you run gst-indent before committing (might not be automatic on non-linux systems) * First commit line should be succinct (< 80 columns) commit 7c658c3c0465d5c09c830ff1d3944a91757ac78c Author: Jürgen Sachs <juergen.sachs@metz-ce.de> Date: Thu Apr 20 11:22:15 2017 +0200 qtdemux: reset sample_description_id to default Fixes stream where sample_description_id is specified in the tfhd https://bugzilla.gnome.org/show_bug.cgi?id=778337