GNOME Bugzilla – Bug 737293
dashdemux: fix gst_mpdparser_get_rep_idx_with_max_bandwidth return
Last modified: 2018-11-03 13:26:45 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.
Created attachment 287013 [details] [review] [PATCH] dashdemux: fix gst_mpdparser_get_rep_idx_with_max_bandwidth return
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).
(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 ?
(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 ;-)
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).
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.
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?
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).
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.
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.
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.
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.
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..
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
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.
(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 ?
(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.
Should I move my patches to that other bug?
(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.
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.
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.
(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).
-- 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.