[FFmpeg-devel] [PATCH] libavformat/hls: Support metadata updates from subdemuxers

[wm4]

On Thu,  1 Feb 2018 18:44:34 -0800
rshaffer at tunein.com wrote:

> From: Richard Shaffer <rshaffer at tunein.com>
> 
> If a subdemuxer has the updated metadata event flag set, the metadata is copied
> to the corresponding stream. The flag is cleared on the subdemuxer and the
> appropriate event flag is set on the stream.
> ---
> This is semi-related to a patch I recently sent to enable parsing ID3 tags from
> .aac files between ADTS headers. However, it may be generically useful for
> other segment formats that support metadata updates.
> 
> -Richard
> 
>  libavformat/hls.c | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index 9bd54c84cc..e48845de34 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -1062,6 +1062,7 @@ static void handle_id3(AVIOContext *pb, struct playlist *pls)
>              /* demuxer not yet opened, defer picture attachment */
>              pls->id3_deferred_extra = extra_meta;
>  
> +        ff_id3v2_parse_priv_dict(&metadata, &extra_meta);
>          av_dict_copy(&pls->ctx->metadata, metadata, 0);
>          pls->id3_initial = metadata;
>  
> @@ -1589,6 +1590,34 @@ static void add_metadata_from_renditions(AVFormatContext *s, struct playlist *pl
>      }
>  }
>  
> +/* update metadata on main streams, if necessary */
> +static void update_metadata_from_subdemuxer(struct playlist *pls, int ignore_flags) {

Normally we put the { on a separate line for functions.

> +    int i;
> +
> +    if (pls->n_main_streams) {
> +        AVStream *st = pls->main_streams[0];
> +        if (ignore_flags) {
> +            av_dict_copy(&st->metadata, pls->ctx->metadata, 0);
> +        } else if (pls->ctx->event_flags & AVFMT_EVENT_FLAG_METADATA_UPDATED) {
> +            av_dict_copy(&st->metadata, pls->ctx->metadata, 0);
> +            pls->ctx->event_flags &= ~AVFMT_EVENT_FLAG_METADATA_UPDATED;
> +            st->event_flags |= AVSTREAM_EVENT_FLAG_METADATA_UPDATED;
> +        }

I don't get understand this: why only stream 0? Isn't this done below
already?

> +    }
> +
> +    for (i = 0; i < pls->ctx->nb_streams; i++) {
> +        AVStream *ist = pls->ctx->streams[i];
> +        AVStream *st = pls->main_streams[i];
> +        if (ignore_flags) {
> +            av_dict_copy(&st->metadata, ist->metadata, 0);
> +        } else if (ist->event_flags & AVSTREAM_EVENT_FLAG_METADATA_UPDATED) {
> +            av_dict_copy(&st->metadata, ist->metadata, 0);
> +            ist->event_flags &= ~AVSTREAM_EVENT_FLAG_METADATA_UPDATED;
> +            st->event_flags |= AVSTREAM_EVENT_FLAG_METADATA_UPDATED;
> +        }
> +    }
> +}

Like mentioned in the other patch, av_dict_copy not clearing the target
dict might be unintended.

> +
>  /* if timestamp was in valid range: returns 1 and sets seq_no
>   * if not: returns 0 and sets seq_no to closest segment */
>  static int find_timestamp_in_playlist(HLSContext *c, struct playlist *pls,
> @@ -1960,6 +1989,7 @@ static int hls_read_header(AVFormatContext *s)
>          if (pls->id3_deferred_extra && pls->ctx->nb_streams == 1) {
>              ff_id3v2_parse_apic(pls->ctx, &pls->id3_deferred_extra);
>              avformat_queue_attached_pictures(pls->ctx);
> +            ff_id3v2_parse_priv(pls->ctx, &pls->id3_deferred_extra);
>              ff_id3v2_free_extra_meta(&pls->id3_deferred_extra);
>              pls->id3_deferred_extra = NULL;
>          }
> @@ -1986,6 +2016,12 @@ static int hls_read_header(AVFormatContext *s)
>          if (ret < 0)
>              goto fail;
>  
> +        /*
> +         * Copy any metadata from playlist to main streams, but do not set
> +         * event flags.
> +         */
> +        update_metadata_from_subdemuxer(pls, 1);
> +

Possibly would be nicer to drop the ignore_flags parameter, and just
unset the event flag at the end of read_header (maybe even in generic
code).

>          add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_AUDIO);
>          add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_VIDEO);
>          add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_SUBTITLE);
> @@ -2170,6 +2206,8 @@ static int hls_read_packet(AVFormatContext *s, AVPacket *pkt)
>              return ret;
>          }
>  
> +        update_metadata_from_subdemuxer(pls, 0);
> +
>          /* check if noheader flag has been cleared by the subdemuxer */
>          if (pls->has_noheader_flag && !(pls->ctx->ctx_flags & AVFMTCTX_NOHEADER)) {
>              pls->has_noheader_flag = 0;