[Bug 752085] dashdemux: revert changes introduced in fix for bug 751850

[GStreamer (GNOME Bugzilla)]

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

--- Comment #11 from Florin Apostol <florin.apostol at oregan.net> ---
> 
> > I expect the usage of these functions to be something like:
> > while (gst_mpd_client_has_next_segment())
> > {
> >   gst_mpd_client_get_next_fragment (fragment);
> >   download fragment
> >   gst_mpd_client_advance_segment()
> >   if download successful
> >     use downloaded data
> > }
> > 
> > This works correct with initializing segment_index to 0 and does not need to
> > be initialized to 1.
> 
> It doesn't. Consider the case with segment_index=0, segments_count=1:
> First loop: has_next_segment==TRUE (0 < 1), advance (0 -> 1)
> Second loop: has_next_segment==FALSE (1 < 1)
> 
> Now for the first loop, there is no segment after the first one. So it
> should not return TRUE there.

It should return true. Because the first segment is the "next segment"
(segment_index=0). Nothing has been downloaded yet, so there is a next segment.
The function returns true, the get_next_fragment will obtain the uri, the
download will download it. Your get_segment() should contain:
   gst_mpd_client_get_next_fragment (fragment);
   download fragment
After download you call advance. Only after this, a call to get_next_segment
will return false (because the stream had 1 next segment, we downloaded it, and
now there is no next segment).

> 
> 
> The code in adaptivedemux is more like
> 
> do {
>   get_segment();
>   if (success)
>     advance();
>   else if (failed_too_often)
>     error();
> } while (has_next_segment());
> 
> > The fact that download failed should not impact the index advance. We just
> > can't use that data, but the index should be updated. What if the download
> > failed for packet 100/305. Will you still consider that the stream has no
> > next segment?
> 
> What do you mean? The problem is that has_next_segment() returns TRUE for
> the last segment without my change. It only returns FALSE once the current
> segment is after the last one then.

It returns true because you did not advance it yet. segment_index points to the
same segment you attempted to download. That is the "next segment". Until you
confirm the download with an advance_segment call, every call to
get_next_segment will return true.

Your problem is that in the download, in case of errors, you want to see if
there is a segment beyond the one you are downloading. That should be a "next
next segment". And with current API, the only way to see that is to call
advance and then call has_next_segment. But you cannot call advance in there
because it will be called again after the download (by
gst_adaptive_demux_stream_advance_fragment_unlocked).

With the current API, I don't think you can make the EOS conversion in
gst_adaptive_demux_stream_download_fragment.

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