I just wanted to send a reminder about this patch...
wm4 had some concerns about publishing a metadata update on each timestamp
(which would essentially be on each segment). I updated it to not set the
metadata updated event flag in those cases, although it will still add that
metadata to the dictionary. I'm not sure if this is exactly the compromise
he had in mind. Any comments on this would be appreciated.
Of course, if anyone else has an opinion or can take the time to review
this, that would be great, too.
Much thanks,
-Richard
On Thu, May 17, 2018 at 8:49 PM, <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;
>
> 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;
> + }
>
> add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_AUDIO);
> add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_VIDEO);
> --
> 2.15.1 (Apple Git-101)
>
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
