Created attachment 305727 [details] [review]
qtmux: Set edit list to compensate DTS shift

We shift DTS forward to avoid negative timestamps which cannot be
represented with version 0 of the CTTS table. To stick with that
version (backward compatibility), the spec recommend using an
edit list entry to move back the presentation time to where it
should be.

Created attachment 305728 [details] [review]
qtmux: Use PTS to figure-out presence of gaps

We need to look at the presentation timestamp in order to conclude if
there is a gap at the start of a stream.

Created attachment 305729 [details] [review]
qtmux: Test gaps at start of stream

Review of attachment 305727 [details] [review]:

::: gst/isomp4/gstqtmux.c
@@ +2476,3 @@
+      }
+
+      if (has_gap || has_shift) {

Why do you need to check for has_gap here? Shouldn't it be only has_shift?

@@ +2481,3 @@
+
+        atom_trak_set_elst_entry (qtpad->trak, 1, duration, shift,
+            (guint32) (1 * 65536.0));

I'd rather have a single empty entry at the beginning, having 2 empty ones looks odd. It should work but I never saw files doing it.

Or is this needed due to that ambiguity of the empty entries at the beginning?

Review of attachment 305728 [details] [review]:

Looks good together with the other fixes.

Review of attachment 305727 [details] [review]:

::: gst/isomp4/gstqtmux.c
@@ +2476,3 @@
+      }
+
+      if (has_gap || has_shift) {

To not change the behaviour. In the past, we'd send two entry in presence of a gap. The empty entry, and the edit. Now we have tree cases.

- The case we only have a gap
- The case we have a gap, and a shift
- the case we only have a shift

The 'empty' entry is sent when there is a gap. The second entry is sent if media start need to be adjusted (shift) or if we had a gap (to keep the old behaviour) I'm not sure why we need this empty entry, since in theory we could just add the gap to the shift and that would be it. But I thought that this was done with care in the past, and that there is most likely a good reason.

@@ +2481,3 @@
+
+        atom_trak_set_elst_entry (qtpad->trak, 1, duration, shift,
+            (guint32) (1 * 65536.0));

Only one is empty. The second entry is not, but it will adjust the media start so time can be mapped properly.

Got it, I misread the second elst entry. Looks fine, then.

Attachment 305727 [details] pushed as feda525 - qtmux: Set edit list to compensate DTS shift
Attachment 305728 [details] pushed as 7b8615d - qtmux: Use PTS to figure-out presence of gaps
Attachment 305729 [details] pushed as 89104e3 - qtmux: Test gaps at start of stream