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.

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.

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 ?

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.

Any further comments? Should we go ahead with this for now, or what should be implemented instead?

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.

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.

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.

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.

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.

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) :)

Of course :)

Attachment 349992 [details] pushed as f0163a0 - qtmux: Error out on discontinuities/gaps when muxing raw audio