GNOME Bugzilla – Bug 749090
hlsdemux: set uri handler blocksize to reduce CPU usage
Last modified: 2018-11-03 13:35:10 UTC
When streaming some encrypted HD content (bitrate about 7 Mbps) using HLS demux, I found CPU usage was hitting 100% on my embedded processor (a Sigma SMP8674). An unencrypted version of the same content was using 47% of the CPU. A bit of digging shows that is because of the default 4K blocksize used by souphttpsrc. Increasing this to 64K reduced CPU usage to 47% for the encrypted stream, and 27% for the clear stream. The solution I've implemented is to modify hlsdemux to set the blocksize property on it's uri_handler. The value set for blocksize is calculated from the bitrate of the current variant, to ensure we can fill a block in 125ms (which seems like a nice low number to me - this shouldn't introduce too much jitter into the pipeline). I set a lower blocksize limit of 4K (as used by gstbasesrc), and a higher limit of 64K. Testing with both clear and encrypted versions of my test stream shows no improvement once blocksize is increased beyond 64K. Block sizes are incremented in 4K steps. With this algorithm, all variants below 262144 bps use a 4K blocksize, and above 4194304 bps use a 64K blocksize.
Created attachment 303055 [details] [review] Implementation against 1.4.5 This patch applies against 1.4.5. It modifies the update_source() function, but this won't be a runner for master, as this function in adaptivedemux.c doesn't have any knowledge of segment bandwidth. One solution would be to implement in gst_hls_demux_select_bitrate(), but then the other adaptive demuxers don't get the benefit. I'm not familiar with the DASH and Smooth Streaming demux elements, so I'll look there to try and determine whether it makes sense to put the solution into the adaptive demux base class, or leave it as HLS only.
This makes sense, but as you expected we'll want a solution for git master as well.
Created attachment 313443 [details] [review] adaptivedemux: control GstUriDownloader block size based upon bitrate Set the downloader blocksize to a value calculated from the stream's current estimated download bandwidth, with a minimum of 4K, and a maximum of 64K. This patch is based against current git master.
Created attachment 313452 [details] [review] uridownloader: allow the block size to be specified. Add a new option "blocksize" (of type guint) that allows the block size to be specified. This patch creates a new function gst_uri_downloader_fetch_uri_with_options. The gst_uri_downloader_fetch_uri_with_options() function uses a GstStructure to specify options to use for the download. This has the advantage of allowing new options to be added in the future without having to create a new function prototype for each option. This patch is not required to support the use case of adjusting the blocksize used by hlsdemux (for that see patch 313443). I thought it would be a good idea to add the feature to uridownloader as well as adaptivedemux.
Review of attachment 313443 [details] [review]: This looks like it'll end up setting the blocksize property on the uridownloader when downloading metadata, as well as content. I don't think this'll be an issue for HLS, where playlists are smaller than a block anyway (whether it be 4 or 64K) - is it ok for the other adaptive demux elements? This change looks a bit cleaner if the CLAMP() macro is used in place of my original tests on blocksize higher and lower bounds.
How about having, in addition to a max block size, a "timeout" value ? This would enable various sources to end up with a decent balance between filling up buffers as much as possible, and not introducing too much of a latency.
(In reply to Duncan Palmer from comment #5) > Review of attachment 313443 [details] [review] [review]: > > This looks like it'll end up setting the blocksize property on the > uridownloader when downloading metadata, as well as content. I don't think > this'll be an issue for HLS, where playlists are smaller than a block anyway > (whether it be 4 or 64K) - is it ok for the other adaptive demux elements? > No, the gst_adaptive_demux_update_manifest_default() function uses gst_uri_downloader_fetch_uri() to download the manifest. Therefore the change to the gst_adaptive_demux_stream_download_uri() does not take part in manifest requests. > This change looks a bit cleaner if the CLAMP() macro is used in place of my > original tests on blocksize higher and lower bounds. Yes, CLAMP would be a bit cleaner.
Created attachment 313677 [details] [review] adaptivedemux: control GstUriDownloader block size based upon bitrate If the blocksize of the downloader is not set, it defaults to 4K. When playing a high bitrate stream, this small blocksize results in significantly increased CPU usage. Update of patch 313443 to use the CLAMP macro and also to fix a bug that caused the block size to not be set if a souphttpsrc element was re-used.
(In reply to Edward Hervey from comment #6) > How about having, in addition to a max block size, a "timeout" value ? > > This would enable various sources to end up with a decent balance between > filling up buffers as much as possible, and not introducing too much of a > latency. While souphttpsrc has a timeout parameter, it is a timeout for waiting for a response [1]. Given that we know the bitrate before the request is sent, using a buffer size that takes at most 125ms to fill doesn't seem too unreasonable. If an application cannot cope with 125ms of jitter, it probably shouldn't be based upon HLS/DASH/MSS adaptive streaming! I guess we could have something like: /* max_buffer_time is specified in milliseconds */ blocksize = gst_util_uint64_scale (stream->current_download_rate, max_buffer_time, 8000) & ~0xFFF; [1] https://developer.gnome.org/libsoup/stable/SoupSession.html#SoupSession--timeout
Ashley, Duncan, Those patches would indeed be nice to have. Could you rebase and test again on current master (or latest release) ? Thanks !
Created attachment 373277 [details] [review] uridownloader: allow the block size to be specified.
Created attachment 373278 [details] [review] adaptivedemux: control GstUriDownloader block size based upon bitrate
I've rebased the code to gst-plugins-bad master.
-- 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/250.