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 780410 - qtdemux: distinguish TFDT with value 0 from no TFDT at all
qtdemux: distinguish TFDT with value 0 from no TFDT at all
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
1.10.4
Other Linux
: Normal normal
: 1.10.5
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-03-22 17:47 UTC by Enrique Ocaña González
Modified: 2017-03-22 21:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patch (2.33 KB, patch)
2017-03-22 18:48 UTC, Enrique Ocaña González
committed Details | Review

Description Enrique Ocaña González 2017-03-22 17:47:24 UTC
Commit https://github.com/GStreamer/gst-plugins-good/commit/1fc3d42f from bug https://bugzilla.gnome.org/show_bug.cgi?id=754230 has caused a regression in our use case (YouTube TV MSE Conformance Tests 2016, test 40. AppendOutOfOrder).

It's an audio stream whose data is appended out of order (as per the test) in these batches (ranges are in seconds): [10, 20] [0, 10] [30, 40] [20, 30]. All the fragments have TFDT atoms, but the new condition added in the mentioned commit is misinterpreting a legitimate "0" value in the TFDT atom as "no TFDT atom at all".
Comment 1 Enrique Ocaña González 2017-03-22 17:48:58 UTC
I've thought about two possible solutions:

A) Have an extra boolean parameter to qtdemux_parse_trun() hinting if the tfdt_node has been found in qtdemux_parse_moof(). Use that info in the condition added by the mentioned commit, instead of checking decode_ts != 0. I've coded this solution locally and can confirm that it solves the issue.

B) Initialize decode_time to GST_CLOCK_TIME_NONE in qtdemux_parse_moof() and add a couple of extra cases in qtdemux_parse_trun() to use 0 when GST_CLOCK_TIME_NONE is passed. Except in the decode_ts != 0 condition, of course, which now would be decode_ts != GST_CLOCK_TIME_NONE.

Which one do you prefer me to submit as a patch?
Comment 2 Enrique Ocaña González 2017-03-22 18:48:37 UTC
Created attachment 348527 [details] [review]
Proposed patch

I'm attaching the patch which passes the extra parameter to qtdemux_parse_trun().
Comment 3 Jan Schmidt 2017-03-22 21:37:41 UTC
Thanks! Pushed:

commit 2f68f8bd891f1529474af644f4475ee516596344
Author: Enrique Ocaña González <eocanha@igalia.com>
Date:   Wed Mar 22 18:18:40 2017 +0000

    qtdemux: distinguish TFDT with value 0 from no TFDT at all
    
    TFDTs with time 0 are being ignored since commit 1fc3d42f. They're
    mistaken with the case of not having TFDT, but those two cases
    must be distinguished in some way.
    
    This patch passes an extra boolean flag when the TFDT is present.
    This is now the condition being evaluated, instead of checking for
    0 time.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=780410