[FFmpeg-devel] [PATCH v4 2/2] lavf/hls: add option to defer parsing of variants

[Anssi Hannula]

Rainer Hochecker kirjoitti 2017-12-03 16:54:
> ---
>  doc/demuxers.texi |   6 ++++
>  libavformat/hls.c | 106 
> ++++++++++++++++++++++++++++++++++++++++++++++--------
>  2 files changed, 98 insertions(+), 14 deletions(-)
> 
> diff --git a/doc/demuxers.texi b/doc/demuxers.texi
> index 73dc0feec1..33643f966a 100644
> --- a/doc/demuxers.texi
> +++ b/doc/demuxers.texi
> @@ -316,6 +316,12 @@ segment index to start live streams at (negative
> values are from the end).
>  @item max_reload
>  Maximum number of times a insufficient list is attempted to be 
> reloaded.
>  Default value is 1000.
> +
> + at item load_all_variants
> +If 0, only the first variant/playlist is loaded on open. All other 
> variants
> +get disabled and can be enabled by setting discard option in program.
> +Default value is 1.
> +
>  @end table
> 
>  @section image2
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index 3c2c720abe..500e3c15de 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -112,6 +112,7 @@ struct playlist {
>      int n_segments;
>      struct segment **segments;
>      int needed;
> +    int parsed;
>      int cur_seq_no;
>      int64_t cur_seg_offset;
>      int64_t last_load_time;
> @@ -206,6 +207,7 @@ typedef struct HLSContext {
>      int strict_std_compliance;
>      char *allowed_extensions;
>      int max_reload;
> +    int load_all_variants;
>  } HLSContext;
[...]
> -static int playlist_needed(struct playlist *pls)
> +static int playlist_needed(AVFormatContext *s, struct playlist *pls,
> int check_parsed)
>  {
> -    AVFormatContext *s = pls->parent;
> +    HLSContext *c = s->priv_data;
>      int i, j;
>      int stream_needed = 0;
>      int first_st;
> 
>      /* If there is no context or streams yet, the playlist is needed 
> */
> -    if (!pls->ctx || !pls->n_main_streams)
> +    if (check_parsed && (!pls->ctx || !pls->n_main_streams))
>          return 1;
> 
> +    /* If the playlist belongs to a non discarded variant and is not 
> parsed,
> +     * we need to parse and activate it later */
> +    for (i = 0; i < s->nb_programs; i++) {
> +        AVProgram *program = s->programs[i];
> +        struct variant *var = c->variants[i];
> +        if (program->discard < AVDISCARD_ALL) {
> +            for (j = 0; j < var->n_playlists; j++) {
> +                if (var->playlists[j] == pls && 
> !var->playlists[j]->parsed)
> +                return  1;

I think this is mostly the same check that is in the same function a 
couple of
lines down, except with the added extra is_parsed check and that your 
version
works even without streams.

So maybe we could avoid duplication by making the flow in the function 
like this:
1. If any streams in the playlist are needed => stream_needed = 1
2. If streams_needed == 0 and playlist has streams => return 0
3. If playlist in undiscarded program (your version) => return 1
4. All program were discarded => return 0

So we avoid having to check for ->parsed and separate "check_parsed" 
handling, I think.
Unparsed discarded playlists would return 0 in step 4 and unparsed 
non-discarded playlists
would return 1 in step 3.

> +            }
> +        }
> +    }
> +
>      /* check if any of the streams in the playlist are needed */
>      for (i = 0; i < pls->n_main_streams; i++) {
>          if (pls->main_streams[i]->discard < AVDISCARD_ALL) {
> @@ -1324,7 +1344,7 @@ restart:
> 
>          /* Check that the playlist is still needed before opening a 
> new
>           * segment. */
> -        v->needed = playlist_needed(v);
> +        v->needed = playlist_needed(v->parent, v, 1);
> 
>          if (!v->needed) {
>              av_log(v->parent, AV_LOG_INFO, "No longer receiving 
> playlist %d\n",
> @@ -1418,23 +1438,41 @@ reload:
>  static void add_renditions_to_variant(HLSContext *c, struct variant 
> *var,
>                                        enum AVMediaType type, const
> char *group_id)
>  {
> -    int i;
> +    int i, j;
> +    int found;
> 
>      for (i = 0; i < c->n_renditions; i++) {
>          struct rendition *rend = c->renditions[i];
> 
>          if (rend->type == type && !strcmp(rend->group_id, group_id)) {
> 
> -            if (rend->playlist)
> +            if (rend->playlist) {
>                  /* rendition is an external playlist
>                   * => add the playlist to the variant */
> -                dynarray_add(&var->playlists, &var->n_playlists,
> rend->playlist);
> -            else
> +                found = 0;
> +                for (j = 0; j < var->n_playlists; j++) {
> +                    if (var->playlists[j] == rend->playlist) {
> +                        found = 1;
> +                        break;
> +                    }
> +                }
> +                if (!found)
> +                    dynarray_add(&var->playlists, &var->n_playlists,
> rend->playlist);
> +            } else {
>                  /* rendition is part of the variant main Media 
> Playlist
>                   * => add the rendition to the main Media Playlist */
> -                dynarray_add(&var->playlists[0]->renditions,
> -                             &var->playlists[0]->n_renditions,
> -                             rend);
> +                found = 0;
> +                for (j = 0; j < var->playlists[0]->n_renditions; j++) 
> {
> +                    if (var->playlists[0]->renditions[j] == rend) {
> +                        found = 1;
> +                        break;
> +                    }
> +                }
> +                if (!found)
> +                    dynarray_add(&var->playlists[0]->renditions,
> +                                 &var->playlists[0]->n_renditions,
> +                                 rend);
> +            }
>          }
>      }
[...]
> +static void activate_playlist(AVFormatContext *s, struct playlist 
> *pls) {
> +
> +	HLSContext *c = s->priv_data;
> +
> +	if (pls->index < c->n_variants) {
> +
> +		struct variant *var = c->variants[pls->index];
> +
> +		if (parse_playlist(c, pls->url, pls, NULL) < 0)
> +			return;
> +		if (var->audio_group[0])
> +			add_renditions_to_variant(c, var, AVMEDIA_TYPE_AUDIO, 
> var->audio_group);
> +		if (var->video_group[0])
> +			add_renditions_to_variant(c, var, AVMEDIA_TYPE_VIDEO, 
> var->video_group);
> +		if (var->subtitles_group[0])
> +			add_renditions_to_variant(c, var, AVMEDIA_TYPE_SUBTITLE,
> var->subtitles_group);

Hmm, didn't notice this before, but I don't think these 
add_renditions_to_variant() calls
use anything from the parse_playlist(media_pls) call so there should be 
no need to re-call
them as the calls in hls_read_header() should have already worked 
perfectly.
The video_group, audio_group, subtitles_group strings come from the 
master playlist which is
always parsed in hls_read_header().

Then we could also drop all the changes you made in 
add_renditions_to_variant()
as well as it would not be called more than once per variant.

Or maybe I am missing something?

Trying to keep variants, renditions, playlists, streams and programs 
straight in
one's head does tend to cause headaches, after all...


> +		init_playlist(c, pls);
> +	}
[...]

-- 
Anssi Hannula