Message ID | 20180518230951.13276-1-rshaffer@tunein.com |
---|---|
State | New |
Headers | show |
On Fri, May 18, 2018 at 4:09 PM, <rshaffer@tunein.com> wrote: > From: Richard Shaffer <rshaffer@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. > --- > > Changes in this version: > * Only set metadata updated flag if we have non-timestamp metadata. > * Fix clearing of metadata updated flag in hls_read_header. > * Specifically cast operands to unsigned when using bitwise operators. > > (This last one is almost certainly pedantic, but it doesn't hurt anything.) > > libavformat/hls.c | 39 ++++++++++++++++++++++++--------------- > 1 file changed, 24 insertions(+), 15 deletions(-) > > diff --git a/libavformat/hls.c b/libavformat/hls.c > index 3d4f7f2647..342a022975 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,19 @@ 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. Note > that > + * we only set the update flag if we have metadata other than a > timestamp. > + */ > + if (av_dict_count(metadata) > (timestamp == AV_NOPTS_VALUE ? 0 : 1)) { > + pls->ctx->event_flags = (unsigned)pls->ctx->event_flags | > + (unsigned)AVFMT_EVENT_FLAG_METADATA_UPDATED; > + } > > if (timestamp != AV_NOPTS_VALUE) { > pls->id3_mpegts_timestamp = timestamp; > @@ -1037,12 +1040,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 +1940,16 @@ 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 = (unsigned)pls->ctx->event_flags & > + ~(unsigned)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) > > Just wanted to ping the list one last time and see if I can get anyone to take a look at this changeset. Thanks, -Richard
diff --git a/libavformat/hls.c b/libavformat/hls.c index 3d4f7f2647..342a022975 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,19 @@ 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. Note that + * we only set the update flag if we have metadata other than a timestamp. + */ + if (av_dict_count(metadata) > (timestamp == AV_NOPTS_VALUE ? 0 : 1)) { + pls->ctx->event_flags = (unsigned)pls->ctx->event_flags | + (unsigned)AVFMT_EVENT_FLAG_METADATA_UPDATED; + } if (timestamp != AV_NOPTS_VALUE) { pls->id3_mpegts_timestamp = timestamp; @@ -1037,12 +1040,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 +1940,16 @@ 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 = (unsigned)pls->ctx->event_flags & + ~(unsigned)AVFMT_EVENT_FLAG_METADATA_UPDATED; + } add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_AUDIO); add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_VIDEO);
From: Richard Shaffer <rshaffer@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. --- Changes in this version: * Only set metadata updated flag if we have non-timestamp metadata. * Fix clearing of metadata updated flag in hls_read_header. * Specifically cast operands to unsigned when using bitwise operators. (This last one is almost certainly pedantic, but it doesn't hurt anything.) libavformat/hls.c | 39 ++++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-)