[Bug 796544] qtdemux: Add comment about qtdemux->segment

[GStreamer (GNOME Bugzilla)]

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

--- Comment #6 from Nicolas Dufresne (ndufresne) <nicolas at ndufresne.ca> ---
Review of attachment 372607:
 --> (https://bugzilla.gnome.org/review?bug=796544&attachment=372607)

Looks like a pretty good comment to me.

::: gst/isomp4/qtdemux.h
@@ +119,3 @@
+  /* Configured playback region.
+   *
+   * segment.format is always GST_FORMAT_TIME.

I know Thibault said otherwise, but reading basesrc code, FORMAT_BYTES seems to
be used there. And FORMAT_PERCENT is implemented also in there, so I have my
doubt about deprecation. For the demuxer output format, I think always TIME is
perfect.

@@ +125,3 @@
+   *
+   * segment.time is never read. It's not useful in this context, since the
+   * mapping from buffer time to stream time changes from track to track.

It's never read, though will it be of the correct value if you want to
translate .start/stop to streaming time ? Or is that always == start ?

@@ +131,3 @@
+   *
+   * segment.duration contains a translation of qtdemux->duration to
+   * nanoseconds. Both fields should be updated at the same time.

Good to document, seems error prone to have two copies of the same value.

@@ +139,3 @@
+   * time (FIXME). This field is used to reply position queries. It's also
used
+   * (with a 2 extra second allowance) to signal EOS at the end of a track in
+   * gst_qtdemux_sync_streams().

What does the FIXME means ? A bug ?

@@ +146,3 @@
+   * segments. (FIXME: In consequence, edits with rate != 1.0 will have wrong
+   * stream time... although probably nobody uses such an obscure feature with
+   * so little player support anyway).

I'd say "edits with rate different then 1.0 are not currently supported". We
could have a todo list comment at the top, as we do in some other sources.

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