[Bug 793058] qtdemux: add support for edit lists for fragmented files in push mode

[GStreamer (GNOME Bugzilla)]

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

Alicia Boya García <aboya at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #369399|none                        |rejected
             status|                            |

--- Comment #12 from Alicia Boya García <aboya at igalia.com> ---
Review of attachment 369399:
 --> (https://bugzilla.gnome.org/review?bug=793058&attachment=369399)

r-

I don't understand how this is supposed to work. I have added comments in the
points that look the most suspicious to me.

::: gst/isomp4/qtdemux.c
@@ +129,3 @@
 #define QTSAMPLE_DTS(stream,sample) (QTSTREAMTIME_TO_GSTTIME((stream),
(sample)->timestamp))
 /* timestamp + offset + cslg_shift is the outgoing PTS */
+#define QTSAMPLE_PTS(stream,sample) (QTSTREAMTIME_TO_GSTTIME((stream),
(sample)->timestamp + (stream)->cslg_shift + (stream)->elst_shift +
(sample)->pts_offset))

QTSAMPLE_PTS() is used to set buffer PTS. By adding elst_shift you are
introducing an inconsistency into qtdemux:

So far buffer PTS in qtdemux is always track presentation time. You need to
calculate stream time to get movie presentation time, where edits are applied.

With this change, you would change that definition for push mode streams with
basic edit lists. It would become track presentation time + media_time of the
edit. It's not even movie presentation time, you would need to subtract
elst_shift for that instead*.

*and that would be problematic because an edit list can cause negative movie
decode and presentation time... which cannot be handled in buffer time because
GstBuffer.{pts,dts} are unsigned.

@@ +352,3 @@
+  /* elst */
+  guint64 elst_shift;
+

The comment does not explain what that field is about. If I've understood
correctly the code it would be something like this.

/* In push mode, when there is an edit list with a single, non-empty edit with
duration=0, this field stores the media_start of that edit in trak timescale.
In other cases, this field is zero. */

@@ +4793,3 @@
   stream->segment.rate = rate;
   stream->segment.start = start + QTSTREAMTIME_TO_GSTTIME (stream,
+      stream->cslg_shift + stream->elst_shift);

Why do you shift buffer time above if you then shift the segment.start in the
same direction? Stream time will reverse the elst_shift that you added,
returning (unedited) track presentation time.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.