On Thu, 17 May 2018 20:49:42 -0700
rshaf...@tunein.com wrote:
> From: Richard Shaffer <rshaf...@tunein.com>
>
> The HLS demuxer will process any ID3 tags at the beginning of a segment in
> order to obtain timestamp data. However, when this data was parsed on a second
> or subsequent segment, the updated metadata would be discarded. This change
> preserves the data and also sets the AVSTREAM_EVENT_FLAG_METADATA_UPDATED
> event flag when appropriate.
> ---
> libavformat/hls.c | 34 +++++++++++++++++++---------------
> 1 file changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index 3d4f7f2647..3e2edb3484 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -982,18 +982,8 @@ static void parse_id3(AVFormatContext *s, AVIOContext
> *pb,
> }
>
> /* Check if the ID3 metadata contents have changed */
> -static int id3_has_changed_values(struct playlist *pls, AVDictionary
> *metadata,
> - ID3v2ExtraMetaAPIC *apic)
> +static int id3_has_changed_values(struct playlist *pls, ID3v2ExtraMetaAPIC
> *apic)
> {
> - AVDictionaryEntry *entry = NULL;
> - AVDictionaryEntry *oldentry;
> - /* check that no keys have changed values */
> - while ((entry = av_dict_get(metadata, "", entry,
> AV_DICT_IGNORE_SUFFIX))) {
> - oldentry = av_dict_get(pls->id3_initial, entry->key, NULL,
> AV_DICT_MATCH_CASE);
> - if (!oldentry || strcmp(oldentry->value, entry->value) != 0)
> - return 1;
> - }
> -
> /* check if apic appeared */
> if (apic && (pls->ctx->nb_streams != 2 ||
> !pls->ctx->streams[1]->attached_pic.data))
> return 1;
> @@ -1019,6 +1009,15 @@ static void handle_id3(AVIOContext *pb, struct
> playlist *pls)
> int64_t timestamp = AV_NOPTS_VALUE;
>
> parse_id3(pls->ctx, pb, &metadata, ×tamp, &apic, &extra_meta);
> + ff_id3v2_parse_priv_dict(&metadata, &extra_meta);
> + av_dict_copy(&pls->ctx->metadata, metadata, 0);
> + /*
> + * If we've handled any ID3 metadata here, it's not going to be seen by
> the
> + * sub-demuxer. In the case that we got here because of an IO call during
> + * hls_read_header, this will be cleared. Otherwise, it provides the
> + * necessary hint to hls_read_packet that there is new metadata.
> + */
> + pls->ctx->event_flags |= AVFMT_EVENT_FLAG_METADATA_UPDATED;
Can't ID3 tags happen in large quantities with that ID3 timestamp hack
(curse whoever invented it)? Wouldn't that lead to a large number of
redundant events? Not sure though, I don't have the overview.
>
> if (timestamp != AV_NOPTS_VALUE) {
> pls->id3_mpegts_timestamp = timestamp;
> @@ -1037,12 +1036,10 @@ 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;
>
> } else {
> - if (!pls->id3_changed && id3_has_changed_values(pls, metadata,
> apic)) {
> + if (!pls->id3_changed && id3_has_changed_values(pls, apic)) {
> avpriv_report_missing_feature(pls->ctx, "Changing ID3 metadata
> in HLS audio elementary stream");
> pls->id3_changed = 1;
> }
> @@ -1939,8 +1936,15 @@ static int hls_read_header(AVFormatContext *s)
> * Copy any metadata from playlist to main streams, but do not set
> * event flags.
> */
> - if (pls->n_main_streams)
> + if (pls->n_main_streams) {
> av_dict_copy(&pls->main_streams[0]->metadata,
> pls->ctx->metadata, 0);
> + /*
> + * If we've intercepted metadata, we will have set this event
> flag.
> + * Clear it to avoid confusion, since otherwise we will flag it
> as
> + * new metadata on the next call to hls_read_packet.
> + */
> + pls->ctx->event_flags = ~AVFMT_EVENT_FLAG_METADATA_UPDATED;
I think ~(unsigned)AVFMT_EVENT_... would be preferable for maximum
correctness.
> + }
>
> add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_AUDIO);
> add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_VIDEO);
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
