[Bug 743703] http connectivity robustness of hlsdemux (and now adaptivedemux)

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=743703
  GStreamer | gst-plugins-bad | unspecified

--- Comment #11 from Mathieu Duponchelle <mduponchelle1 at gmail.com> 2015-02-05 16:26:57 UTC ---
OK so after extensive testing and looking into the history of hlsdemux, I can
be affirmative that the issues this patch was fixing are mostly taken care of
by adaptivedemux, yay :)

First off, adaptivedemux mostly uses its own wrapped http source element, with
the only exception being the update of the manifest.

It handles the messages emitted by souphttpsrc and correctly retries
downloading a fragment until MAX_DOWNLOAD_ERROR_COUNT is reached.

There are a few ways in which adaptivedemux handles errors differently:

1) The patches made a difference between actual http errors (< 100), and failed
directly when error codes were > 100, adaptivedemux will retry for all errors.

2) The number of retries is hardcoded

3) The patches exposed min and max retry times, waited first for min_retry_time
before retrying, then multiplied the waiting time by two until either
max_retry_time was reached or the number of retries exceeded max_retries.
AdaptiveDemux always waits for half the duration of the fragment to retry.

I think 1) would be desirable, I'm not sure whether 2) is needed or a sane
dafault is enough, and I'm not sure either about 3), the current approach in
adaptivedemux seems a bit ad hoc, maybe the heuristic in these patches is
better?

What do you think thiago?

Side note, I have had some trouble understanding against which exact moment in
the history the uridownloader patch was made :

We can see here :

https://github.com/ford-prefect/gst-plugins-bad/commit/d445374067996b5c5fefdf03582930bd97c86762

that Sebastian introduced the "err" field in uridownloader->priv, and at the
same type modified the prototype of fetch_uri and fetch_uri_with_range to add
an error parameter, but this patch acts on a version that has the old
prototype, but where the err field already exists, a bit confusing, it would
help me to know against what this commit was made.

Right now, when I use the new -validate action to override the libc's standard
receive and set new errno values, I can see that the code appropriately
retries, and that when the maximum number of retries is exceeded a descriptive
error is surfaced, which is IMO the expected behaviour, and renders these
patches mostly obsolete.

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