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 780679 - qtmux: Error out on discontinuities/gaps when muxing raw audio
qtmux: Error out on discontinuities/gaps when muxing raw audio
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
unspecified
Other All
: Normal normal
: 1.13.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-03-29 11:03 UTC by Sebastian Dröge (slomo)
Modified: 2017-05-09 12:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
qtmux: Error out on discontinuities/gaps when muxing raw audio (3.89 KB, patch)
2017-03-29 11:03 UTC, Sebastian Dröge (slomo)
none Details | Review
qtmux: Error out on discontinuities/gaps when muxing raw audio (3.91 KB, patch)
2017-04-04 12:25 UTC, Sebastian Dröge (slomo)
none Details | Review
qtmux: Error out on discontinuities/gaps when muxing raw audio (7.14 KB, patch)
2017-04-18 11:32 UTC, Sebastian Dröge (slomo)
committed Details | Review

Description Sebastian Dröge (slomo) 2017-03-29 11:03:24 UTC
See commit message. Question is if this should be a warning instead, and if
40ms is good enough of a limit.
Comment 1 Sebastian Dröge (slomo) 2017-03-29 11:03:29 UTC
Created attachment 348926 [details] [review]
qtmux: Error out on discontinuities/gaps when muxing raw audio

When muxing raw audio, we have no way of storing timestamps but are just
storing a continuous stream of audio samples. If the difference between
the expected and the real timestamp becomes to big, we should error out
instead of silently creating files with wrong A/V sync.
Comment 2 Thiago Sousa Santos 2017-04-01 18:33:05 UTC
I think we could use edit lists to store gaps in streams. We already do it for the beginning of the stream when video and audio won't start aligned.
Comment 3 Nicolas Dufresne (ndufresne) 2017-04-01 20:03:11 UTC
At the same time, remember that edit lists are poorly supported on non-gst base players. Can't we special case the raw audio case and fill the gaps with silence ?
Comment 4 Sebastian Dröge (slomo) 2017-04-01 21:55:21 UTC
We could, and I actually have a patch locally (adding an unrelated feature) that indeed does that.

Problem here is that this is not only raw audio, but also ADPCM and similar formats.
Comment 5 Sebastian Dröge (slomo) 2017-04-03 15:33:00 UTC
Any further comments? Should we go ahead with this for now, or what should be implemented instead?
Comment 6 Nicolas Dufresne (ndufresne) 2017-04-03 16:12:13 UTC
Review of attachment 348926 [details] [review]:

::: gst/isomp4/gstqtmux.c
@@ +3337,3 @@
+    if (GST_BUFFER_DTS_OR_PTS (last_buf) < expected_timestamp
+        || GST_BUFFER_DTS_OR_PTS (last_buf) - expected_timestamp >
+        40 * GST_MSECOND)

Don't hardcode that though, make this configurable. And allow ignoring the shifts too. On audio sink we have alignment-threshold that trigger skews.
Comment 7 Nicolas Dufresne (ndufresne) 2017-04-03 16:13:15 UTC
Otherwise, no further comment on my side. Edit List is the only solution that covers gaps in encoded formats, meanwhile we should warn, error-out or do nothing, but to not break existing application, this needs to be configurable.
Comment 8 Thiago Sousa Santos 2017-04-03 17:33:57 UTC
Review of attachment 348926 [details] [review]:

::: gst/isomp4/gstqtmux.c
@@ +3504,3 @@
+            GST_TIME_ARGS (gst_util_uint64_scale (pad->sample_offset,
+                    GST_SECOND,
+                    atom_trak_get_timescale (pad->trak)) + pad->first_ts)));

I don't mind merging this but I'd make it more clear that we are missing a feature so that users can file bugs if they see this happening and they'd would like to have it implemented. A FIXME log or something equivalent should suffice.
Comment 9 Sebastian Dröge (slomo) 2017-04-04 12:25:51 UTC
Created attachment 349237 [details] [review]
qtmux: Error out on discontinuities/gaps when muxing raw audio

When muxing raw audio, we have no way of storing timestamps but are just
storing a continuous stream of audio samples. If the difference between
the expected and the real timestamp becomes to big, we should error out
instead of silently creating files with wrong A/V sync.
Comment 10 Sebastian Dröge (slomo) 2017-04-18 11:32:31 UTC
Created attachment 349992 [details] [review]
qtmux: Error out on discontinuities/gaps when muxing raw audio

When muxing raw audio, we have no way of storing timestamps but are just
storing a continuous stream of audio samples. If the difference between
the expected and the real timestamp becomes to big, we should error out
instead of silently creating files with wrong A/V sync.
Comment 11 Tim-Philipp Müller 2017-04-18 11:41:38 UTC
That sounds like something for after 1.12 in any case.

I'm sure it will break some people's stuff (for better or worse) :)
Comment 12 Sebastian Dröge (slomo) 2017-04-18 11:43:08 UTC
Of course :)
Comment 13 Sebastian Dröge (slomo) 2017-05-09 12:19:04 UTC
Attachment 349992 [details] pushed as f0163a0 - qtmux: Error out on discontinuities/gaps when muxing raw audio