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

[GStreamer (GNOME Bugzilla)]

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

--- Comment #10 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
(In reply to Florin Apostol from comment #9)
> I agree that segment_index is the index of the next sequence chunk to be
> downloaded. So, the comment from its definition is correct.
> 
> All the function that uses it seems to have the same understanding of its
> meaning:
> gst_mpd_client_get_next_fragment, gst_mpd_client_get_next_fragment_duration,
> gst_mpd_client_get_next_fragment_timestamp all use segment_index value to
> retrieve the next fragment. They don't do any incrementing.

Correct, and they use the current value of the segment_index.

> I'm not sure what is the difference between a segment and a fragment. I
> believe they are synonyms and they refer to the same thing. Can you confirm
> this?

Correct

> I still believe the code was originally correct and you do not need to add 1
> in gst_mpd_client_has_next_segment. If you still think that the 1 offset
> needs to be there, then we probably need to update all other functions that
> validate the index.

Yeah, need to check the others too then :)

> 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.


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.

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