After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 737293 - dashdemux: fix gst_mpdparser_get_rep_idx_with_max_bandwidth return
dashdemux: fix gst_mpdparser_get_rep_idx_with_max_bandwidth return
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
1.4.1
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-09-24 20:43 UTC by Matthieu Bouron
Modified: 2018-11-03 13:26 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH] dashdemux: fix gst_mpdparser_get_rep_idx_with_max_bandwidth return (1.79 KB, patch)
2014-09-24 20:44 UTC, Matthieu Bouron
reviewed Details | Review
gst-plugins-bad: Add "download-bitrate" property to souphttpsrc (5.65 KB, patch)
2014-10-20 15:17 UTC, Brendan Long
none Details | Review
gst-plugins-bad: dash: Use pipeline source's "download-bitrate" as the default bitrate (3.22 KB, patch)
2014-10-20 15:19 UTC, Brendan Long
none Details | Review
gst-plugins-good: soup: Record response time and send custom event with bitrate (4.17 KB, patch)
2014-10-20 16:15 UTC, Brendan Long
none Details | Review
gst-plugins-bad: dash: Use bitrate from soup's custom event as the default bitrate (2.57 KB, patch)
2014-10-20 16:18 UTC, Brendan Long
none Details | Review
gst-plugins-good: soup: Record response time and send custom event with bitrate (4.16 KB, patch)
2014-10-21 14:02 UTC, Brendan Long
none Details | Review
gst-plugins-bad: dash: Use bitrates from soup's custom event for all bitrates (6.30 KB, patch)
2014-10-21 14:12 UTC, Brendan Long
none Details | Review

Description Matthieu Bouron 2014-09-24 20:43:12 UTC
If bandwidth <=0, gst_mpdparser_get_rep_idx_with_max_bandwidth should return -1 as no corresponding representation is found.

This makes dashdemux select the lowest representation for the first fragment.
Comment 1 Matthieu Bouron 2014-09-24 20:44:37 UTC
Created attachment 287013 [details] [review]
[PATCH] dashdemux: fix gst_mpdparser_get_rep_idx_with_max_bandwidth return
Comment 2 Sebastian Dröge (slomo) 2014-09-25 07:47:05 UTC
Comment on attachment 287013 [details] [review]
[PATCH] dashdemux: fix gst_mpdparser_get_rep_idx_with_max_bandwidth return

Does the standard say that it should get the lowest by default? IIRC for HLS it gets the first representation in the manifest as default.


Note that by default the bitrate property is going to be set to -1 (aka unknown).
Comment 3 Matthieu Bouron 2014-09-25 09:32:49 UTC
(In reply to comment #2)
> (From update of attachment 287013 [details] [review])
> Does the standard say that it should get the lowest by default? IIRC for HLS it
> gets the first representation in the manifest as default.
> 

I had a quick look at the spec but didn't find any information about which representation should be used when we do not know the bandwidth yet.

I modified the code to make it consistent with what it is expected according to the comments in gst_mpdparser_get_rep_idx_with_max_bandwidth which says to get the lowest representation if bitrate <= 0.

gst_mpdparser_get_rep_idx_with_max_bandwidth now returns -1 and then gst_mpdparser_get_rep_idx_with_min_bandwidth is called to get the index of the lowest representation.

IMHO, It feels better/more natural to begin with the lowest stream to avoid stuttering at the cost of quality.

What do you think ?
Comment 4 Felix Schwarz 2014-09-25 14:51:07 UTC
(In reply to comment #3)
> IMHO, It feels better/more natural to begin with the lowest stream to avoid
> stuttering at the cost of quality.

I got quite a bit of customer feedback about this. Actually all of them preferred having the best possible quality initially. There are a lot of people with fat pipes and they want to see something great. Most people were willing to wait a bit longer initially in exchange for good quality.

In my ideal world, all software would measure the time to download the first segment and automatically calculate which quality is sustainable for future segments given the download speed ;-)
Comment 5 Brendan Long 2014-10-19 12:57:24 UTC
Will the DASH element give up if it detects that network conditions have changed and downloading a segment would take too long? For example, if the top quality was 8K video at 120 fps, would it spend 20 minutes downloading the first segment, or would it give up once it knows its download speed and try a different file?

If so, then picking the highest quality makes sense.

If not, we should considering either adding that, or adding an attribute to designate the expected bandwidth (defaulting to something like 10-20 Mbps, so we'd get good quality by default, but if we're wrong, we'll only end up buffering for a short time).
Comment 6 Brendan Long 2014-10-20 09:48:18 UTC
Could we make a reasonable guess based on how long it took to download the MPD? I don't think souphttpsrc keeps track of that, but it probably could. I doubt it would be very accurate, since the MPD isn't very big, but it's probably accurate enough.
Comment 7 Brendan Long 2014-10-20 11:17:43 UTC
As a proof of concept, I tried adding a GTimer to souphttpsrc to time requests and expose it as a property ("request-time"). A lot of test DASH files have tiny MPDs (~4 KB) and in those cases we always get useless values because the download time is dominated by latency, but once I tried with a real file I got reasonable values.

With http://www-itec.uni-klu.ac.at/ftp/datasets/mmsys12/Valkaama/MPDs/Valkaama_1s_act_isoffmain_DIS_23009_1_v_2_1c2_2011_08_30.mpd (1 hour 33 minutes long according to http://www-itec.uni-klu.ac.at/dash/?page_id=207): 

0:00:04.778981598  7349       0xac5590 WARN             souphttpsrc gstsouphttpsrc.c:1286:gst_soup_http_src_got_body_cb:<source> Downloaded 4165938 bytes in 4.601469 seconds, download speed is 7242796 bits/s

With http://download.tsi.telecom-paristech.fr/gpac/DASH_CONFORMANCE/TelecomParisTech/mp4-main-multi/mp4-main-multi-mpd-AV-BS.mpd (listed in dashdemux's README):

0:00:02.626909926  7416      0x24f3590 WARN             souphttpsrc gstsouphttpsrc.c:1286:gst_soup_http_src_got_body_cb:<source> Downloaded 24281 bytes in 2.458095 seconds, download speed is 79023 bits/s

What do you think of this approach?

If we do this, I assume we would want to send this as some sort of event instead of a property?
Comment 8 Brendan Long 2014-10-20 15:17:38 UTC
Created attachment 288955 [details] [review]
gst-plugins-bad: Add "download-bitrate" property to souphttpsrc

This is probably not how we actually want to architect this, but it's a proof of concept. In this patch, souphttpsrc times its requests and exposes the bitrate for its download as "download-bitrate".

Based on a suggestion from Tim, this can be improved by only timing from when we get the headers until the download finishes (so we don't include the round trip time in the calculation).
Comment 9 Brendan Long 2014-10-20 15:19:43 UTC
Created attachment 288956 [details] [review]
gst-plugins-bad: dash: Use pipeline source's "download-bitrate" as the default bitrate

This patch makes dashdemux look up the "source" of the pipeline and ask it for the "download-bitrate". I assume that the way we actually want to do this is with a custom event, but I couldn't get dashdemux to detect a custom event from the souphttpsrc. Presumably that can be fixed though.
Comment 10 Brendan Long 2014-10-20 16:15:30 UTC
Created attachment 288963 [details] [review]
gst-plugins-good: soup: Record response time and send custom event with bitrate

I changed the soup patch to send a custom event instead of a property. It also uses the time after receiving the headers so that we can ignore the round trip time.
Comment 11 Brendan Long 2014-10-20 16:18:00 UTC
Created attachment 288964 [details] [review]
gst-plugins-bad: dash: Use bitrate from soup's custom event as the default bitrate

This uses the bitrate from souphttpsrc as the default bitrate.

I also fixed stream->current_download_rate to default to -1 instead of 0, since gst_dash_demux_stream_select_representation_unlocked considers -1 to be "unset".

This patch could be improved to use the same soup event to determine bitrates from the segment files.
Comment 12 Brendan Long 2014-10-21 14:02:52 UTC
Created attachment 289039 [details] [review]
gst-plugins-good: soup: Record response time and send custom event with bitrate

I changed the soup event to send a gint64 instead of a gdouble for consistency with existing dash code.
Comment 13 Brendan Long 2014-10-21 14:12:00 UTC
Created attachment 289040 [details] [review]
gst-plugins-bad: dash: Use bitrates from soup's custom event for all bitrates

This patch uses souphttpsrc's info for all bitrate information. I just realized that this would completely fail if the source doesn't implement this event though. Since we probably don't want to require the source to do that, should we just continue calculating it ourselves when we can? If we have to have the code to time this, then it seems like we might as well always use it instead of having a second unnecessary code path..
Comment 14 Matthieu Bouron 2014-10-27 13:05:39 UTC
IMHO, we should continue calcultating the bandwidth ourselves as we can't depend on this new event.

Btw, here is a patch that tries to improve the bandwidth calculation by using a sliding window to smooth the bandwidth: https://bugzilla.gnome.org/show_bug.cgi?id=737703
Comment 15 Sebastian Dröge (slomo) 2014-10-27 13:21:13 UTC
Why can't we depend on this event? Also see bug #735789 for the general problem with the way how we currently calculate the bitrate.
Comment 16 Matthieu Bouron 2014-10-27 13:26:41 UTC
(In reply to comment #15)
> Why can't we depend on this event? Also see bug #735789 for the general problem
> with the way how we currently calculate the bitrate.

I meant that if we don't receive the event we need to keep a way to compute the bandwidth. Or do you think this is something that we should force on every source element ?
Comment 17 Brendan Long 2014-10-27 14:11:17 UTC
(In reply to comment #15)
> Why can't we depend on this event?

I was under the impression that the source isn't required to be a souphttpsrc. If it is, then we can depend on the event. If not, another source might not send it.
Comment 18 Brendan Long 2014-10-27 14:12:12 UTC
Should I move my patches to that other bug?
Comment 19 Matthieu Bouron 2014-10-27 14:13:29 UTC
(In reply to comment #17)
> (In reply to comment #15)
> > Why can't we depend on this event?
> 
> I was under the impression that the source isn't required to be a souphttpsrc.
> If it is, then we can depend on the event. If not, another source might not
> send it.

It could be filesrc for example when doing local tests.
Comment 20 Brendan Long 2014-11-17 23:43:16 UTC
Right now we create the sources for each stream with:

    stream->src = gst_element_make_from_uri (GST_URI_SRC, uri, NULL, NULL);

We could just directly request a souphttpsrc here (I think those handle files too?). Or we could ask the source if it supports this, maybe by adding a property with the currently detected bandwidth, and assume any source with that property is also sending the event.
Comment 21 Thiago Sousa Santos 2015-02-21 13:02:08 UTC
I'd propose adding an enum property to allow the user to select how to pick the starting fragment.

We need the options: 'lowest', 'highest', 'default'

default would be tricky as only HLS has a meaning for that, for DASH and MSS I don't think the spec has any recommendation for it.

In the future some automatic could be added that would try to guess the best bitrate by doing some download test before streaming begins.
Comment 22 Brendan Long 2015-02-27 16:39:52 UTC
(In reply to Thiago Sousa Santos from comment #21)
> I'd propose adding an enum property to allow the user to select how to pick
> the starting fragment.
> 
> We need the options: 'lowest', 'highest', 'default'

'highest' might be misleading, since we probably wouldn't select the highest bandwidth representation if the resolution is larger than we need. Presumably the first two options would literally mean, "set estimated bandwidth to 0", and "set estimated bandwidth to infinity" (and then follow the normal selection process).
Comment 23 GStreamer system administrator 2018-11-03 13:26:45 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/issues/175.