[Bug 796559] qtdemux: Various segment fixes to properly take segment.offset into account

[GStreamer (GNOME Bugzilla)]

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

--- Comment #7 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
(In reply to Alicia Boya García from comment #6)
> Review of attachment 372632 [details] [review]:
> 
> ::: gst/isomp4/qtdemux.c
> @@ +1566,3 @@
> +  /* TODO: If !update we can keep all the streams at their current
> +   * position and don't have to go through all the edst/segment finding
> +   * machinary. We only need to send an updated segment in that case.
> 
> Isn't sending an updated segment required for all kinds of seeks?

Yes, "only" is the keyword here. Only a new segment is needed, not all the
other stuff that is done to find the correct edst segment, and then finding the
correct sample in there, etc.

> @@ -1566,3 @@
> -  /* may not have enough fragmented info to do this adjustment,
> -   * and we can't scan (and probably should not) at this time with
> -   * possibly flushing upstream */
> 
> What was this about "flushing upstream"? gst_qtdemux_perform_seek() is only
> called by gst_qtdemux_do_seek(), which is only called in pull mode.

I don't know, I just moved that comment with the code. It made no sense to me
either.

> @@ +1731,3 @@
>    } else {
>      /* now do the seek */
> +    ret = gst_qtdemux_perform_seek (qtdemux, &seeksegment, TRUE, seqnum,
> flags);
> 
> Not something to be fixed by this patch, but maybe worth pointing out: this
> branch can never execute since `event` cannot be NULL. For event to be NULL,
> gst_qtdemux_do_seek() would have to be called with a NULL event. The only
> call is in gst_qtdemux_handle_src_event(), when the event type is
> GST_EVENT_SEEK... Since the type of the event has been checked at that point
> -- which required indirecting the pointer, event cannot be NULL.

This is probably leftover from an older version of the code, where it maybe did
a NULL seek in pull mode at the very beginning. Not sure.

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