[Bug 707530] qtdemux: Handle segments correctly in push mode seeks

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=707530
  GStreamer | gst-plugins-good | git

--- Comment #8 from Sebastian Dröge (slomo) <slomo at circular-chaos.org> 2013-09-05 13:15:53 UTC ---
(In reply to comment #7)
> (In reply to comment #5)
> > Review of attachment 254150 [details] [details]:
> > 
> > Looks sensible but:
> > 
> > ::: gst/isomp4/qtdemux.c
> > @@ +1907,3 @@
> > +      GST_DEBUG_OBJECT (demux, "Stored seek seqnum: %u, received segment "
> > +          "seqnum %u", demux->push_seek_seqnum, seqnum);
> > +      if (seqnum == demux->push_seek_seqnum) {
> > 
> > Instead of comparing the seqnum it should already be enough just compare the
> > start position
> 
> Just to make sure I understood your comment: Store the seek start position (in
> bytes) and use that instead of the seqnum? Likely because other elements might
> not preserve the seqnum?

Yes, and also because you don't care if it's the same seqnum or not. You only
really care about the start position.

> (In reply to comment #6)
> > Review of attachment 254151 [details] [details]:
> > 
> > ::: gst/isomp4/qtdemux.c
> > @@ +4923,3 @@
> > +          /* check for segment end */
> > +          if (G_UNLIKELY (demux->segment.stop != -1
> > +                  && demux->segment.stop <= pts && stream->on_keyframe)) {
> > 
> > Why the on_keyframe here? It should do that always, independent of the seek
> > type
> 
> This 'on_keyframe' is not related to the seek flags, this is updated for every
> buffer pushed and allows qtdemux to only stop pushing a stream on keyframes,
> allowing decoders downstream to have all data needed to reach the stop
> position, too.

Everything good then :)

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.