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 797133 - qtmux: Allow up to 1 audio sample of lateness in prefill mode
qtmux: Allow up to 1 audio sample of lateness in prefill mode
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
unspecified
Other All
: Normal normal
: 1.15.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-09-12 14:27 UTC by Vivia Nikolaidou
Modified: 2018-09-13 10:52 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
qtmux: Allow up to 1 audio sample of lateness in prefill mode (1.89 KB, patch)
2018-09-12 14:27 UTC, Vivia Nikolaidou
needs-work Details | Review
qtmux: Allow up to 1 trak timescale unit of lateness in prefill mode (1.91 KB, patch)
2018-09-13 09:18 UTC, Vivia Nikolaidou
none Details | Review
qtmux: Allow up to 1 trak timescale unit of lateness in prefill mode (2.06 KB, patch)
2018-09-13 10:49 UTC, Vivia Nikolaidou
committed Details | Review

Description Vivia Nikolaidou 2018-09-12 14:27:50 UTC
See commit message
Comment 1 Vivia Nikolaidou 2018-09-12 14:27:55 UTC
Created attachment 373621 [details] [review]
qtmux: Allow up to 1 audio sample of lateness in prefill mode

For 59.94 FPS, it's common to set 60000 as timescale. For that
timescale, if the audio is late by as little as 0:00:00.000016666
(definitely less than one audio sample), lateness gets rounded to 1.
Added a safeguard that allows lateness up to 1 sample with the specific
trak's timescale, to make sure that values less than one audio sample
won't break the prefill mode. What will happen in this case is that the
audio will get squeezed back to the video's timestamp, which in practice
means that the audio will be 0.000016666 seconds early (with the patch).
Comment 2 Sebastian Dröge (slomo) 2018-09-13 07:10:41 UTC
Review of attachment 373621 [details] [review]:

::: gst/isomp4/gstqtmux.c
@@ +3483,3 @@
+        /* allow up to 1 sample of lateness */
+        trak_lateness = gst_util_uint64_scale (diff,
+            atom_trak_get_timescale (qtpad->trak), GST_SECOND);

The timescale is not necessarily 1 sample (i.e. 1 audio sample, 1 video frame, etc: for video it usually is a 100th of a frame) but can be almost anything.

I think this generally makes sense though. While you do this now not only in prefill mode (please change the commit message), there's not much point in adding such a small entry.
Comment 3 Vivia Nikolaidou 2018-09-13 09:18:11 UTC
Created attachment 373623 [details] [review]
qtmux: Allow up to 1 trak timescale unit of lateness in prefill mode

For 59.94 FPS, it's common to set 60000 as timescale. For that
timescale, if the audio is late by as little as 0:00:00.000016666
(definitely less than one audio sample), lateness gets rounded to 1.
Added a safeguard that allows lateness up to 1 sample with the specific
trak's timescale, to make sure that values less than e.g. one audio
sample won't break the prefill mode. What will happen in this case is
that the audio will get squeezed back to the video's timestamp, which in
practice means that the audio will be 0.000016666 seconds early (with
the patch).
Comment 4 Vivia Nikolaidou 2018-09-13 09:19:47 UTC
Review of attachment 373621 [details] [review]:

(obsolete)
Comment 5 Sebastian Dröge (slomo) 2018-09-13 10:41:48 UTC
Comment on attachment 373623 [details] [review]
qtmux: Allow up to 1 trak timescale unit of lateness in prefill mode

Please also mention why it's fine to ignore the <1 trak timescale units difference here. Simply because that can't possibly be represented as a timestamp/duration by the trak specific parts of the headers, so it's irrelevantly small.
Comment 6 Vivia Nikolaidou 2018-09-13 10:49:37 UTC
Created attachment 373633 [details] [review]
qtmux: Allow up to 1 trak timescale unit of lateness in prefill mode

For 59.94 FPS, it's common to set 60000 as timescale. For that
timescale, if the audio is late by as little as 0:00:00.000016666
(definitely less than one audio sample), lateness gets rounded to 1.
Added a safeguard that allows lateness up to 1 sample with the specific
trak's timescale, to make sure that values less than e.g. one audio
sample won't break the prefill mode. What will happen in this case is
that the audio will get squeezed back to the video's timestamp, which in
practice means that the audio will be 0.000016666 seconds early (with
the patch).
Comment 7 Vivia Nikolaidou 2018-09-13 10:52:08 UTC
Review of attachment 373633 [details] [review]:

commit 94f8603411d607aa125f31d48786c8bbf10eab25 (HEAD -> upstream_master, upstream/master)
Author: Vivia Nikolaidou <vivia@ahiru.eu>
Date:   Wed Sep 12 17:24:00 2018 +0300

    qtmux: Allow up to 1 trak timescale unit of lateness in prefill mode

    For 59.94 FPS, it's common to set 60000 as timescale. For that
    timescale, if the audio is late by as little as 0:00:00.000016666
    (definitely less than one audio sample), lateness gets rounded to 1.
    Added a safeguard that allows lateness up to 1 sample with the specific
    trak's timescale, to make sure that values less than e.g. one audio
    sample won't break the prefill mode. What will happen in this case is
    that the audio will get squeezed back to the video's timestamp, which in
    practice means that the audio will be 0.000016666 seconds early (with
    the patch).

    https://bugzilla.gnome.org/show_bug.cgi?id=797133