| Message ID | 20240326005639.27000-1-toots@rastageeks.org |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/2] libavformat/hls.c: support in-stream ID3 metadata update. | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Mon, Mar 25, 2024, 19:58 Romain Beauxis <toots@rastageeks.org> wrote: > This patch adds support for updating HLS metadata passed as ID3 frames. > > This seems like a pretty straight-forward improvement. Updating the > metadaata of the first stream seems to be the mechanism is other places > in the code and works as expected. > Hello! Any interest in reviewing this? --- > libavformat/hls.c | 54 ++++++++++++++++++++++++++++------------------- > 1 file changed, 32 insertions(+), 22 deletions(-) > > diff --git a/libavformat/hls.c b/libavformat/hls.c > index f6b44c2e35..ba6634d57a 100644 > --- a/libavformat/hls.c > +++ b/libavformat/hls.c > @@ -93,6 +93,12 @@ enum PlaylistType { > PLS_TYPE_VOD > }; > > +#define ID3_PRIV_OWNER_TS "com.apple.streaming.transportStreamTimestamp" > +#define ID3_PRIV_OWNER_AUDIO_SETUP "com.apple.streaming.audioDescription" > + > +#define ID3v2_PRIV_OWNER_TS ID3v2_PRIV_METADATA_PREFIX ID3_PRIV_OWNER_TS > +#define ID3v2_PRIV_OWNER_AUDIO_SETUP ID3v2_PRIV_METADATA_PREFIX > ID3_PRIV_OWNER_AUDIO_SETUP > + > /* > * Each playlist has its own demuxer. If it currently is active, > * it has an open AVIOContext too, and potentially an AVPacket > @@ -150,9 +156,7 @@ struct playlist { > int64_t id3_offset; /* in stream original tb */ > uint8_t* id3_buf; /* temp buffer for id3 parsing */ > unsigned int id3_buf_size; > - AVDictionary *id3_initial; /* data from first id3 tag */ > - int id3_found; /* ID3 tag found at some point */ > - int id3_changed; /* ID3 tag data has changed at some point */ > + AVDictionary *last_id3; /* data from the last id3 tag */ > ID3v2ExtraMeta *id3_deferred_extra; /* stored here until subdemuxer > is opened */ > > HLSAudioSetupInfo audio_setup_info; > @@ -270,7 +274,7 @@ static void free_playlist_list(HLSContext *c) > av_freep(&pls->main_streams); > av_freep(&pls->renditions); > av_freep(&pls->id3_buf); > - av_dict_free(&pls->id3_initial); > + av_dict_free(&pls->last_id3); > ff_id3v2_free_extra_meta(&pls->id3_deferred_extra); > av_freep(&pls->init_sec_buf); > av_packet_free(&pls->pkt); > @@ -1083,15 +1087,13 @@ static void parse_id3(AVFormatContext *s, > AVIOContext *pb, > AVDictionary **metadata, int64_t *dts, > HLSAudioSetupInfo *audio_setup_info, > ID3v2ExtraMetaAPIC **apic, ID3v2ExtraMeta > **extra_meta) > { > - static const char id3_priv_owner_ts[] = > "com.apple.streaming.transportStreamTimestamp"; > - static const char id3_priv_owner_audio_setup[] = > "com.apple.streaming.audioDescription"; > ID3v2ExtraMeta *meta; > > ff_id3v2_read_dict(pb, metadata, ID3v2_DEFAULT_MAGIC, extra_meta); > for (meta = *extra_meta; meta; meta = meta->next) { > if (!strcmp(meta->tag, "PRIV")) { > ID3v2ExtraMetaPRIV *priv = &meta->data.priv; > - if (priv->datasize == 8 && !av_strncasecmp(priv->owner, > id3_priv_owner_ts, 44)) { > + if (priv->datasize == 8 && !av_strncasecmp(priv->owner, > ID3_PRIV_OWNER_TS, strlen(ID3_PRIV_OWNER_TS))) { > /* 33-bit MPEG timestamp */ > int64_t ts = AV_RB64(priv->data); > av_log(s, AV_LOG_DEBUG, "HLS ID3 audio timestamp > %"PRId64"\n", ts); > @@ -1099,7 +1101,9 @@ static void parse_id3(AVFormatContext *s, > AVIOContext *pb, > *dts = ts; > else > av_log(s, AV_LOG_ERROR, "Invalid HLS ID3 audio > timestamp %"PRId64"\n", ts); > - } else if (priv->datasize >= 8 && > !av_strncasecmp(priv->owner, id3_priv_owner_audio_setup, 36)) { > + } else if (priv->datasize >= 8 && > + !av_strncasecmp(priv->owner, > ID3_PRIV_OWNER_AUDIO_SETUP, 36) && > + audio_setup_info) { > ff_hls_senc_read_audio_setup_info(audio_setup_info, > priv->data, priv->datasize); > } > } else if (!strcmp(meta->tag, "APIC") && apic) > @@ -1113,9 +1117,10 @@ static int id3_has_changed_values(struct playlist > *pls, AVDictionary *metadata, > { > const AVDictionaryEntry *entry = NULL; > const AVDictionaryEntry *oldentry; > + > /* check that no keys have changed values */ > while ((entry = av_dict_iterate(metadata, entry))) { > - oldentry = av_dict_get(pls->id3_initial, entry->key, NULL, > AV_DICT_MATCH_CASE); > + oldentry = av_dict_get(pls->last_id3, entry->key, NULL, > AV_DICT_MATCH_CASE); > if (!oldentry || strcmp(oldentry->value, entry->value) != 0) > return 1; > } > @@ -1143,35 +1148,40 @@ static void handle_id3(AVIOContext *pb, struct > playlist *pls) > ID3v2ExtraMetaAPIC *apic = NULL; > ID3v2ExtraMeta *extra_meta = NULL; > int64_t timestamp = AV_NOPTS_VALUE; > + // Only set audio_setup_info on first id3 chunk. > + HLSAudioSetupInfo *audio_setup_info = pls->last_id3 ? NULL : > &pls->audio_setup_info; > > - parse_id3(pls->ctx, pb, &metadata, ×tamp, > &pls->audio_setup_info, &apic, &extra_meta); > + parse_id3(pls->ctx, pb, &metadata, ×tamp, audio_setup_info, > &apic, &extra_meta); > > - if (timestamp != AV_NOPTS_VALUE) { > + if (pls->id3_mpegts_timestamp == AV_NOPTS_VALUE && timestamp != > AV_NOPTS_VALUE) { > pls->id3_mpegts_timestamp = timestamp; > pls->id3_offset = 0; > } > > - if (!pls->id3_found) { > - /* initial ID3 tags */ > - av_assert0(!pls->id3_deferred_extra); > - pls->id3_found = 1; > - > + if (id3_has_changed_values(pls, metadata, apic)) { > /* get picture attachment and set text metadata */ > if (pls->ctx->nb_streams) > ff_id3v2_parse_apic(pls->ctx, extra_meta); > - else > + else { > + av_assert0(!pls->id3_deferred_extra); > /* demuxer not yet opened, defer picture attachment */ > pls->id3_deferred_extra = extra_meta; > + } > > ff_id3v2_parse_priv_dict(&metadata, extra_meta); > + > + av_dict_set(&metadata, ID3v2_PRIV_OWNER_TS, NULL, 0); > + av_dict_set(&metadata, ID3v2_PRIV_OWNER_AUDIO_SETUP, NULL, 0); > + > + av_dict_free(&pls->ctx->metadata); > av_dict_copy(&pls->ctx->metadata, metadata, 0); > - pls->id3_initial = metadata; > > + if (pls->n_main_streams) > + av_dict_copy(&pls->main_streams[0]->metadata, metadata, 0); > + > + if (pls->last_id3) av_dict_free(&pls->last_id3); > + pls->last_id3 = metadata; > } else { > - if (!pls->id3_changed && id3_has_changed_values(pls, metadata, > apic)) { > - avpriv_report_missing_feature(pls->parent, "Changing ID3 > metadata in HLS audio elementary stream"); > - pls->id3_changed = 1; > - } > av_dict_free(&metadata); > } > > -- > 2.39.3 (Apple Git-145) > >
> 在 2024年3月29日，06:51，Romain Beauxis <toots@rastageeks.org> 写道： > > On Mon, Mar 25, 2024, 19:58 Romain Beauxis <toots@rastageeks.org> wrote: > >> This patch adds support for updating HLS metadata passed as ID3 frames. >> >> This seems like a pretty straight-forward improvement. Updating the >> metadaata of the first stream seems to be the mechanism is other places >> in the code and works as expected. >> > > > Hello! > > Any interest in reviewing this? Let me test these patchset need some days, Thanks > > > --- >> libavformat/hls.c | 54 ++++++++++++++++++++++++++++------------------- >> 1 file changed, 32 insertions(+), 22 deletions(-) >> >> diff --git a/libavformat/hls.c b/libavformat/hls.c >> index f6b44c2e35..ba6634d57a 100644 >> --- a/libavformat/hls.c >> +++ b/libavformat/hls.c >> @@ -93,6 +93,12 @@ enum PlaylistType { >> PLS_TYPE_VOD >> }; >> >> +#define ID3_PRIV_OWNER_TS "com.apple.streaming.transportStreamTimestamp" >> +#define ID3_PRIV_OWNER_AUDIO_SETUP "com.apple.streaming.audioDescription" >> + >> +#define ID3v2_PRIV_OWNER_TS ID3v2_PRIV_METADATA_PREFIX ID3_PRIV_OWNER_TS >> +#define ID3v2_PRIV_OWNER_AUDIO_SETUP ID3v2_PRIV_METADATA_PREFIX >> ID3_PRIV_OWNER_AUDIO_SETUP >> + >> /* >> * Each playlist has its own demuxer. If it currently is active, >> * it has an open AVIOContext too, and potentially an AVPacket >> @@ -150,9 +156,7 @@ struct playlist { >> int64_t id3_offset; /* in stream original tb */ >> uint8_t* id3_buf; /* temp buffer for id3 parsing */ >> unsigned int id3_buf_size; >> - AVDictionary *id3_initial; /* data from first id3 tag */ >> - int id3_found; /* ID3 tag found at some point */ >> - int id3_changed; /* ID3 tag data has changed at some point */ >> + AVDictionary *last_id3; /* data from the last id3 tag */ >> ID3v2ExtraMeta *id3_deferred_extra; /* stored here until subdemuxer >> is opened */ >> >> HLSAudioSetupInfo audio_setup_info; >> @@ -270,7 +274,7 @@ static void free_playlist_list(HLSContext *c) >> av_freep(&pls->main_streams); >> av_freep(&pls->renditions); >> av_freep(&pls->id3_buf); >> - av_dict_free(&pls->id3_initial); >> + av_dict_free(&pls->last_id3); >> ff_id3v2_free_extra_meta(&pls->id3_deferred_extra); >> av_freep(&pls->init_sec_buf); >> av_packet_free(&pls->pkt); >> @@ -1083,15 +1087,13 @@ static void parse_id3(AVFormatContext *s, >> AVIOContext *pb, >> AVDictionary **metadata, int64_t *dts, >> HLSAudioSetupInfo *audio_setup_info, >> ID3v2ExtraMetaAPIC **apic, ID3v2ExtraMeta >> **extra_meta) >> { >> - static const char id3_priv_owner_ts[] = >> "com.apple.streaming.transportStreamTimestamp"; >> - static const char id3_priv_owner_audio_setup[] = >> "com.apple.streaming.audioDescription"; >> ID3v2ExtraMeta *meta; >> >> ff_id3v2_read_dict(pb, metadata, ID3v2_DEFAULT_MAGIC, extra_meta); >> for (meta = *extra_meta; meta; meta = meta->next) { >> if (!strcmp(meta->tag, "PRIV")) { >> ID3v2ExtraMetaPRIV *priv = &meta->data.priv; >> - if (priv->datasize == 8 && !av_strncasecmp(priv->owner, >> id3_priv_owner_ts, 44)) { >> + if (priv->datasize == 8 && !av_strncasecmp(priv->owner, >> ID3_PRIV_OWNER_TS, strlen(ID3_PRIV_OWNER_TS))) { >> /* 33-bit MPEG timestamp */ >> int64_t ts = AV_RB64(priv->data); >> av_log(s, AV_LOG_DEBUG, "HLS ID3 audio timestamp >> %"PRId64"\n", ts); >> @@ -1099,7 +1101,9 @@ static void parse_id3(AVFormatContext *s, >> AVIOContext *pb, >> *dts = ts; >> else >> av_log(s, AV_LOG_ERROR, "Invalid HLS ID3 audio >> timestamp %"PRId64"\n", ts); >> - } else if (priv->datasize >= 8 && >> !av_strncasecmp(priv->owner, id3_priv_owner_audio_setup, 36)) { >> + } else if (priv->datasize >= 8 && >> + !av_strncasecmp(priv->owner, >> ID3_PRIV_OWNER_AUDIO_SETUP, 36) && >> + audio_setup_info) { >> ff_hls_senc_read_audio_setup_info(audio_setup_info, >> priv->data, priv->datasize); >> } >> } else if (!strcmp(meta->tag, "APIC") && apic) >> @@ -1113,9 +1117,10 @@ static int id3_has_changed_values(struct playlist >> *pls, AVDictionary *metadata, >> { >> const AVDictionaryEntry *entry = NULL; >> const AVDictionaryEntry *oldentry; >> + >> /* check that no keys have changed values */ >> while ((entry = av_dict_iterate(metadata, entry))) { >> - oldentry = av_dict_get(pls->id3_initial, entry->key, NULL, >> AV_DICT_MATCH_CASE); >> + oldentry = av_dict_get(pls->last_id3, entry->key, NULL, >> AV_DICT_MATCH_CASE); >> if (!oldentry || strcmp(oldentry->value, entry->value) != 0) >> return 1; >> } >> @@ -1143,35 +1148,40 @@ static void handle_id3(AVIOContext *pb, struct >> playlist *pls) >> ID3v2ExtraMetaAPIC *apic = NULL; >> ID3v2ExtraMeta *extra_meta = NULL; >> int64_t timestamp = AV_NOPTS_VALUE; >> + // Only set audio_setup_info on first id3 chunk. >> + HLSAudioSetupInfo *audio_setup_info = pls->last_id3 ? NULL : >> &pls->audio_setup_info; >> >> - parse_id3(pls->ctx, pb, &metadata, ×tamp, >> &pls->audio_setup_info, &apic, &extra_meta); >> + parse_id3(pls->ctx, pb, &metadata, ×tamp, audio_setup_info, >> &apic, &extra_meta); >> >> - if (timestamp != AV_NOPTS_VALUE) { >> + if (pls->id3_mpegts_timestamp == AV_NOPTS_VALUE && timestamp != >> AV_NOPTS_VALUE) { >> pls->id3_mpegts_timestamp = timestamp; >> pls->id3_offset = 0; >> } >> >> - if (!pls->id3_found) { >> - /* initial ID3 tags */ >> - av_assert0(!pls->id3_deferred_extra); >> - pls->id3_found = 1; >> - >> + if (id3_has_changed_values(pls, metadata, apic)) { >> /* get picture attachment and set text metadata */ >> if (pls->ctx->nb_streams) >> ff_id3v2_parse_apic(pls->ctx, extra_meta); >> - else >> + else { >> + av_assert0(!pls->id3_deferred_extra); >> /* demuxer not yet opened, defer picture attachment */ >> pls->id3_deferred_extra = extra_meta; >> + } >> >> ff_id3v2_parse_priv_dict(&metadata, extra_meta); >> + >> + av_dict_set(&metadata, ID3v2_PRIV_OWNER_TS, NULL, 0); >> + av_dict_set(&metadata, ID3v2_PRIV_OWNER_AUDIO_SETUP, NULL, 0); >> + >> + av_dict_free(&pls->ctx->metadata); >> av_dict_copy(&pls->ctx->metadata, metadata, 0); >> - pls->id3_initial = metadata; >> >> + if (pls->n_main_streams) >> + av_dict_copy(&pls->main_streams[0]->metadata, metadata, 0); >> + >> + if (pls->last_id3) av_dict_free(&pls->last_id3); >> + pls->last_id3 = metadata; >> } else { >> - if (!pls->id3_changed && id3_has_changed_values(pls, metadata, >> apic)) { >> - avpriv_report_missing_feature(pls->parent, "Changing ID3 >> metadata in HLS audio elementary stream"); >> - pls->id3_changed = 1; >> - } >> av_dict_free(&metadata); >> } >> >> -- >> 2.39.3 (Apple Git-145) >> >> > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> On Mar 29, 2024, at 06:51, Romain Beauxis <toots@rastageeks.org> wrote: > > On Mon, Mar 25, 2024, 19:58 Romain Beauxis <toots@rastageeks.org> wrote: > >> This patch adds support for updating HLS metadata passed as ID3 frames. >> >> This seems like a pretty straight-forward improvement. Updating the >> metadaata of the first stream seems to be the mechanism is other places >> in the code and works as expected. >> > > > Hello! > > Any interest in reviewing this? Patchset looks good to me, looks better than before patch. > > > --- >> libavformat/hls.c | 54 ++++++++++++++++++++++++++++------------------- >> 1 file changed, 32 insertions(+), 22 deletions(-) >> >> diff --git a/libavformat/hls.c b/libavformat/hls.c >> index f6b44c2e35..ba6634d57a 100644 >> --- a/libavformat/hls.c >> +++ b/libavformat/hls.c >> @@ -93,6 +93,12 @@ enum PlaylistType { >> PLS_TYPE_VOD >> }; >> >> +#define ID3_PRIV_OWNER_TS "com.apple.streaming.transportStreamTimestamp" >> +#define ID3_PRIV_OWNER_AUDIO_SETUP "com.apple.streaming.audioDescription" >> + >> +#define ID3v2_PRIV_OWNER_TS ID3v2_PRIV_METADATA_PREFIX ID3_PRIV_OWNER_TS >> +#define ID3v2_PRIV_OWNER_AUDIO_SETUP ID3v2_PRIV_METADATA_PREFIX >> ID3_PRIV_OWNER_AUDIO_SETUP >> + >> /* >> * Each playlist has its own demuxer. If it currently is active, >> * it has an open AVIOContext too, and potentially an AVPacket >> @@ -150,9 +156,7 @@ struct playlist { >> int64_t id3_offset; /* in stream original tb */ >> uint8_t* id3_buf; /* temp buffer for id3 parsing */ >> unsigned int id3_buf_size; >> - AVDictionary *id3_initial; /* data from first id3 tag */ >> - int id3_found; /* ID3 tag found at some point */ >> - int id3_changed; /* ID3 tag data has changed at some point */ >> + AVDictionary *last_id3; /* data from the last id3 tag */ >> ID3v2ExtraMeta *id3_deferred_extra; /* stored here until subdemuxer >> is opened */ >> >> HLSAudioSetupInfo audio_setup_info; >> @@ -270,7 +274,7 @@ static void free_playlist_list(HLSContext *c) >> av_freep(&pls->main_streams); >> av_freep(&pls->renditions); >> av_freep(&pls->id3_buf); >> - av_dict_free(&pls->id3_initial); >> + av_dict_free(&pls->last_id3); >> ff_id3v2_free_extra_meta(&pls->id3_deferred_extra); >> av_freep(&pls->init_sec_buf); >> av_packet_free(&pls->pkt); >> @@ -1083,15 +1087,13 @@ static void parse_id3(AVFormatContext *s, >> AVIOContext *pb, >> AVDictionary **metadata, int64_t *dts, >> HLSAudioSetupInfo *audio_setup_info, >> ID3v2ExtraMetaAPIC **apic, ID3v2ExtraMeta >> **extra_meta) >> { >> - static const char id3_priv_owner_ts[] = >> "com.apple.streaming.transportStreamTimestamp"; >> - static const char id3_priv_owner_audio_setup[] = >> "com.apple.streaming.audioDescription"; >> ID3v2ExtraMeta *meta; >> >> ff_id3v2_read_dict(pb, metadata, ID3v2_DEFAULT_MAGIC, extra_meta); >> for (meta = *extra_meta; meta; meta = meta->next) { >> if (!strcmp(meta->tag, "PRIV")) { >> ID3v2ExtraMetaPRIV *priv = &meta->data.priv; >> - if (priv->datasize == 8 && !av_strncasecmp(priv->owner, >> id3_priv_owner_ts, 44)) { >> + if (priv->datasize == 8 && !av_strncasecmp(priv->owner, >> ID3_PRIV_OWNER_TS, strlen(ID3_PRIV_OWNER_TS))) { >> /* 33-bit MPEG timestamp */ >> int64_t ts = AV_RB64(priv->data); >> av_log(s, AV_LOG_DEBUG, "HLS ID3 audio timestamp >> %"PRId64"\n", ts); >> @@ -1099,7 +1101,9 @@ static void parse_id3(AVFormatContext *s, >> AVIOContext *pb, >> *dts = ts; >> else >> av_log(s, AV_LOG_ERROR, "Invalid HLS ID3 audio >> timestamp %"PRId64"\n", ts); >> - } else if (priv->datasize >= 8 && >> !av_strncasecmp(priv->owner, id3_priv_owner_audio_setup, 36)) { >> + } else if (priv->datasize >= 8 && >> + !av_strncasecmp(priv->owner, >> ID3_PRIV_OWNER_AUDIO_SETUP, 36) && >> + audio_setup_info) { >> ff_hls_senc_read_audio_setup_info(audio_setup_info, >> priv->data, priv->datasize); >> } >> } else if (!strcmp(meta->tag, "APIC") && apic) >> @@ -1113,9 +1117,10 @@ static int id3_has_changed_values(struct playlist >> *pls, AVDictionary *metadata, >> { >> const AVDictionaryEntry *entry = NULL; >> const AVDictionaryEntry *oldentry; >> + >> /* check that no keys have changed values */ >> while ((entry = av_dict_iterate(metadata, entry))) { >> - oldentry = av_dict_get(pls->id3_initial, entry->key, NULL, >> AV_DICT_MATCH_CASE); >> + oldentry = av_dict_get(pls->last_id3, entry->key, NULL, >> AV_DICT_MATCH_CASE); >> if (!oldentry || strcmp(oldentry->value, entry->value) != 0) >> return 1; >> } >> @@ -1143,35 +1148,40 @@ static void handle_id3(AVIOContext *pb, struct >> playlist *pls) >> ID3v2ExtraMetaAPIC *apic = NULL; >> ID3v2ExtraMeta *extra_meta = NULL; >> int64_t timestamp = AV_NOPTS_VALUE; >> + // Only set audio_setup_info on first id3 chunk. >> + HLSAudioSetupInfo *audio_setup_info = pls->last_id3 ? NULL : >> &pls->audio_setup_info; >> >> - parse_id3(pls->ctx, pb, &metadata, ×tamp, >> &pls->audio_setup_info, &apic, &extra_meta); >> + parse_id3(pls->ctx, pb, &metadata, ×tamp, audio_setup_info, >> &apic, &extra_meta); >> >> - if (timestamp != AV_NOPTS_VALUE) { >> + if (pls->id3_mpegts_timestamp == AV_NOPTS_VALUE && timestamp != >> AV_NOPTS_VALUE) { >> pls->id3_mpegts_timestamp = timestamp; >> pls->id3_offset = 0; >> } >> >> - if (!pls->id3_found) { >> - /* initial ID3 tags */ >> - av_assert0(!pls->id3_deferred_extra); >> - pls->id3_found = 1; >> - >> + if (id3_has_changed_values(pls, metadata, apic)) { >> /* get picture attachment and set text metadata */ >> if (pls->ctx->nb_streams) >> ff_id3v2_parse_apic(pls->ctx, extra_meta); >> - else >> + else { >> + av_assert0(!pls->id3_deferred_extra); >> /* demuxer not yet opened, defer picture attachment */ >> pls->id3_deferred_extra = extra_meta; >> + } >> >> ff_id3v2_parse_priv_dict(&metadata, extra_meta); >> + >> + av_dict_set(&metadata, ID3v2_PRIV_OWNER_TS, NULL, 0); >> + av_dict_set(&metadata, ID3v2_PRIV_OWNER_AUDIO_SETUP, NULL, 0); >> + >> + av_dict_free(&pls->ctx->metadata); >> av_dict_copy(&pls->ctx->metadata, metadata, 0); >> - pls->id3_initial = metadata; >> >> + if (pls->n_main_streams) >> + av_dict_copy(&pls->main_streams[0]->metadata, metadata, 0); >> + >> + if (pls->last_id3) av_dict_free(&pls->last_id3); >> + pls->last_id3 = metadata; >> } else { >> - if (!pls->id3_changed && id3_has_changed_values(pls, metadata, >> apic)) { >> - avpriv_report_missing_feature(pls->parent, "Changing ID3 >> metadata in HLS audio elementary stream"); >> - pls->id3_changed = 1; >> - } >> av_dict_free(&metadata); >> } >> >> -- >> 2.39.3 (Apple Git-145) >> >> > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On Sun, Mar 31, 2024, 05:52 Liu Steven <lq@chinaffmpeg.org> wrote: > > > > On Mar 29, 2024, at 06:51, Romain Beauxis <toots@rastageeks.org> wrote: > > > > On Mon, Mar 25, 2024, 19:58 Romain Beauxis <toots@rastageeks.org> wrote: > > > >> This patch adds support for updating HLS metadata passed as ID3 frames. > >> > >> This seems like a pretty straight-forward improvement. Updating the > >> metadaata of the first stream seems to be the mechanism is other places > >> in the code and works as expected. > >> > > > > > > Hello! > > > > Any interest in reviewing this? > > Patchset looks good to me, looks better than before patch. > Great! Happy to provide the fate samples too. > --- > >> libavformat/hls.c | 54 ++++++++++++++++++++++++++++------------------- > >> 1 file changed, 32 insertions(+), 22 deletions(-) > >> > >> diff --git a/libavformat/hls.c b/libavformat/hls.c > >> index f6b44c2e35..ba6634d57a 100644 > >> --- a/libavformat/hls.c > >> +++ b/libavformat/hls.c > >> @@ -93,6 +93,12 @@ enum PlaylistType { > >> PLS_TYPE_VOD > >> }; > >> > >> +#define ID3_PRIV_OWNER_TS > "com.apple.streaming.transportStreamTimestamp" > >> +#define ID3_PRIV_OWNER_AUDIO_SETUP > "com.apple.streaming.audioDescription" > >> + > >> +#define ID3v2_PRIV_OWNER_TS ID3v2_PRIV_METADATA_PREFIX > ID3_PRIV_OWNER_TS > >> +#define ID3v2_PRIV_OWNER_AUDIO_SETUP ID3v2_PRIV_METADATA_PREFIX > >> ID3_PRIV_OWNER_AUDIO_SETUP > >> + > >> /* > >> * Each playlist has its own demuxer. If it currently is active, > >> * it has an open AVIOContext too, and potentially an AVPacket > >> @@ -150,9 +156,7 @@ struct playlist { > >> int64_t id3_offset; /* in stream original tb */ > >> uint8_t* id3_buf; /* temp buffer for id3 parsing */ > >> unsigned int id3_buf_size; > >> - AVDictionary *id3_initial; /* data from first id3 tag */ > >> - int id3_found; /* ID3 tag found at some point */ > >> - int id3_changed; /* ID3 tag data has changed at some point */ > >> + AVDictionary *last_id3; /* data from the last id3 tag */ > >> ID3v2ExtraMeta *id3_deferred_extra; /* stored here until subdemuxer > >> is opened */ > >> > >> HLSAudioSetupInfo audio_setup_info; > >> @@ -270,7 +274,7 @@ static void free_playlist_list(HLSContext *c) > >> av_freep(&pls->main_streams); > >> av_freep(&pls->renditions); > >> av_freep(&pls->id3_buf); > >> - av_dict_free(&pls->id3_initial); > >> + av_dict_free(&pls->last_id3); > >> ff_id3v2_free_extra_meta(&pls->id3_deferred_extra); > >> av_freep(&pls->init_sec_buf); > >> av_packet_free(&pls->pkt); > >> @@ -1083,15 +1087,13 @@ static void parse_id3(AVFormatContext *s, > >> AVIOContext *pb, > >> AVDictionary **metadata, int64_t *dts, > >> HLSAudioSetupInfo *audio_setup_info, > >> ID3v2ExtraMetaAPIC **apic, ID3v2ExtraMeta > >> **extra_meta) > >> { > >> - static const char id3_priv_owner_ts[] = > >> "com.apple.streaming.transportStreamTimestamp"; > >> - static const char id3_priv_owner_audio_setup[] = > >> "com.apple.streaming.audioDescription"; > >> ID3v2ExtraMeta *meta; > >> > >> ff_id3v2_read_dict(pb, metadata, ID3v2_DEFAULT_MAGIC, extra_meta); > >> for (meta = *extra_meta; meta; meta = meta->next) { > >> if (!strcmp(meta->tag, "PRIV")) { > >> ID3v2ExtraMetaPRIV *priv = &meta->data.priv; > >> - if (priv->datasize == 8 && !av_strncasecmp(priv->owner, > >> id3_priv_owner_ts, 44)) { > >> + if (priv->datasize == 8 && !av_strncasecmp(priv->owner, > >> ID3_PRIV_OWNER_TS, strlen(ID3_PRIV_OWNER_TS))) { > >> /* 33-bit MPEG timestamp */ > >> int64_t ts = AV_RB64(priv->data); > >> av_log(s, AV_LOG_DEBUG, "HLS ID3 audio timestamp > >> %"PRId64"\n", ts); > >> @@ -1099,7 +1101,9 @@ static void parse_id3(AVFormatContext *s, > >> AVIOContext *pb, > >> *dts = ts; > >> else > >> av_log(s, AV_LOG_ERROR, "Invalid HLS ID3 audio > >> timestamp %"PRId64"\n", ts); > >> - } else if (priv->datasize >= 8 && > >> !av_strncasecmp(priv->owner, id3_priv_owner_audio_setup, 36)) { > >> + } else if (priv->datasize >= 8 && > >> + !av_strncasecmp(priv->owner, > >> ID3_PRIV_OWNER_AUDIO_SETUP, 36) && > >> + audio_setup_info) { > >> ff_hls_senc_read_audio_setup_info(audio_setup_info, > >> priv->data, priv->datasize); > >> } > >> } else if (!strcmp(meta->tag, "APIC") && apic) > >> @@ -1113,9 +1117,10 @@ static int id3_has_changed_values(struct playlist > >> *pls, AVDictionary *metadata, > >> { > >> const AVDictionaryEntry *entry = NULL; > >> const AVDictionaryEntry *oldentry; > >> + > >> /* check that no keys have changed values */ > >> while ((entry = av_dict_iterate(metadata, entry))) { > >> - oldentry = av_dict_get(pls->id3_initial, entry->key, NULL, > >> AV_DICT_MATCH_CASE); > >> + oldentry = av_dict_get(pls->last_id3, entry->key, NULL, > >> AV_DICT_MATCH_CASE); > >> if (!oldentry || strcmp(oldentry->value, entry->value) != 0) > >> return 1; > >> } > >> @@ -1143,35 +1148,40 @@ static void handle_id3(AVIOContext *pb, struct > >> playlist *pls) > >> ID3v2ExtraMetaAPIC *apic = NULL; > >> ID3v2ExtraMeta *extra_meta = NULL; > >> int64_t timestamp = AV_NOPTS_VALUE; > >> + // Only set audio_setup_info on first id3 chunk. > >> + HLSAudioSetupInfo *audio_setup_info = pls->last_id3 ? NULL : > >> &pls->audio_setup_info; > >> > >> - parse_id3(pls->ctx, pb, &metadata, ×tamp, > >> &pls->audio_setup_info, &apic, &extra_meta); > >> + parse_id3(pls->ctx, pb, &metadata, ×tamp, audio_setup_info, > >> &apic, &extra_meta); > >> > >> - if (timestamp != AV_NOPTS_VALUE) { > >> + if (pls->id3_mpegts_timestamp == AV_NOPTS_VALUE && timestamp != > >> AV_NOPTS_VALUE) { > >> pls->id3_mpegts_timestamp = timestamp; > >> pls->id3_offset = 0; > >> } > >> > >> - if (!pls->id3_found) { > >> - /* initial ID3 tags */ > >> - av_assert0(!pls->id3_deferred_extra); > >> - pls->id3_found = 1; > >> - > >> + if (id3_has_changed_values(pls, metadata, apic)) { > >> /* get picture attachment and set text metadata */ > >> if (pls->ctx->nb_streams) > >> ff_id3v2_parse_apic(pls->ctx, extra_meta); > >> - else > >> + else { > >> + av_assert0(!pls->id3_deferred_extra); > >> /* demuxer not yet opened, defer picture attachment */ > >> pls->id3_deferred_extra = extra_meta; > >> + } > >> > >> ff_id3v2_parse_priv_dict(&metadata, extra_meta); > >> + > >> + av_dict_set(&metadata, ID3v2_PRIV_OWNER_TS, NULL, 0); > >> + av_dict_set(&metadata, ID3v2_PRIV_OWNER_AUDIO_SETUP, NULL, 0); > >> + > >> + av_dict_free(&pls->ctx->metadata); > >> av_dict_copy(&pls->ctx->metadata, metadata, 0); > >> - pls->id3_initial = metadata; > >> > >> + if (pls->n_main_streams) > >> + av_dict_copy(&pls->main_streams[0]->metadata, metadata, 0); > >> + > >> + if (pls->last_id3) av_dict_free(&pls->last_id3); > >> + pls->last_id3 = metadata; > >> } else { > >> - if (!pls->id3_changed && id3_has_changed_values(pls, metadata, > >> apic)) { > >> - avpriv_report_missing_feature(pls->parent, "Changing ID3 > >> metadata in HLS audio elementary stream"); > >> - pls->id3_changed = 1; > >> - } > >> av_dict_free(&metadata); > >> } > >> > >> -- > >> 2.39.3 (Apple Git-145) > >> > >> > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On Sun, Mar 31, 2024, 12:46 Romain Beauxis <romain.beauxis@gmail.com> wrote: > > > On Sun, Mar 31, 2024, 05:52 Liu Steven <lq@chinaffmpeg.org> wrote: > >> >> >> > On Mar 29, 2024, at 06:51, Romain Beauxis <toots@rastageeks.org> wrote: >> > >> > On Mon, Mar 25, 2024, 19:58 Romain Beauxis <toots@rastageeks.org> >> wrote: >> > >> >> This patch adds support for updating HLS metadata passed as ID3 frames. >> >> >> >> This seems like a pretty straight-forward improvement. Updating the >> >> metadaata of the first stream seems to be the mechanism is other places >> >> in the code and works as expected. >> >> >> > >> > >> > Hello! >> > >> > Any interest in reviewing this? >> >> Patchset looks good to me, looks better than before patch. >> > > Great! Happy to provide the fate samples too. > > Hi! Any update on this? Let me know if I can do anything to help! > > --- >> >> libavformat/hls.c | 54 ++++++++++++++++++++++++++++------------------- >> >> 1 file changed, 32 insertions(+), 22 deletions(-) >> >> >> >> diff --git a/libavformat/hls.c b/libavformat/hls.c >> >> index f6b44c2e35..ba6634d57a 100644 >> >> --- a/libavformat/hls.c >> >> +++ b/libavformat/hls.c >> >> @@ -93,6 +93,12 @@ enum PlaylistType { >> >> PLS_TYPE_VOD >> >> }; >> >> >> >> +#define ID3_PRIV_OWNER_TS >> "com.apple.streaming.transportStreamTimestamp" >> >> +#define ID3_PRIV_OWNER_AUDIO_SETUP >> "com.apple.streaming.audioDescription" >> >> + >> >> +#define ID3v2_PRIV_OWNER_TS ID3v2_PRIV_METADATA_PREFIX >> ID3_PRIV_OWNER_TS >> >> +#define ID3v2_PRIV_OWNER_AUDIO_SETUP ID3v2_PRIV_METADATA_PREFIX >> >> ID3_PRIV_OWNER_AUDIO_SETUP >> >> + >> >> /* >> >> * Each playlist has its own demuxer. If it currently is active, >> >> * it has an open AVIOContext too, and potentially an AVPacket >> >> @@ -150,9 +156,7 @@ struct playlist { >> >> int64_t id3_offset; /* in stream original tb */ >> >> uint8_t* id3_buf; /* temp buffer for id3 parsing */ >> >> unsigned int id3_buf_size; >> >> - AVDictionary *id3_initial; /* data from first id3 tag */ >> >> - int id3_found; /* ID3 tag found at some point */ >> >> - int id3_changed; /* ID3 tag data has changed at some point */ >> >> + AVDictionary *last_id3; /* data from the last id3 tag */ >> >> ID3v2ExtraMeta *id3_deferred_extra; /* stored here until subdemuxer >> >> is opened */ >> >> >> >> HLSAudioSetupInfo audio_setup_info; >> >> @@ -270,7 +274,7 @@ static void free_playlist_list(HLSContext *c) >> >> av_freep(&pls->main_streams); >> >> av_freep(&pls->renditions); >> >> av_freep(&pls->id3_buf); >> >> - av_dict_free(&pls->id3_initial); >> >> + av_dict_free(&pls->last_id3); >> >> ff_id3v2_free_extra_meta(&pls->id3_deferred_extra); >> >> av_freep(&pls->init_sec_buf); >> >> av_packet_free(&pls->pkt); >> >> @@ -1083,15 +1087,13 @@ static void parse_id3(AVFormatContext *s, >> >> AVIOContext *pb, >> >> AVDictionary **metadata, int64_t *dts, >> >> HLSAudioSetupInfo *audio_setup_info, >> >> ID3v2ExtraMetaAPIC **apic, ID3v2ExtraMeta >> >> **extra_meta) >> >> { >> >> - static const char id3_priv_owner_ts[] = >> >> "com.apple.streaming.transportStreamTimestamp"; >> >> - static const char id3_priv_owner_audio_setup[] = >> >> "com.apple.streaming.audioDescription"; >> >> ID3v2ExtraMeta *meta; >> >> >> >> ff_id3v2_read_dict(pb, metadata, ID3v2_DEFAULT_MAGIC, extra_meta); >> >> for (meta = *extra_meta; meta; meta = meta->next) { >> >> if (!strcmp(meta->tag, "PRIV")) { >> >> ID3v2ExtraMetaPRIV *priv = &meta->data.priv; >> >> - if (priv->datasize == 8 && !av_strncasecmp(priv->owner, >> >> id3_priv_owner_ts, 44)) { >> >> + if (priv->datasize == 8 && !av_strncasecmp(priv->owner, >> >> ID3_PRIV_OWNER_TS, strlen(ID3_PRIV_OWNER_TS))) { >> >> /* 33-bit MPEG timestamp */ >> >> int64_t ts = AV_RB64(priv->data); >> >> av_log(s, AV_LOG_DEBUG, "HLS ID3 audio timestamp >> >> %"PRId64"\n", ts); >> >> @@ -1099,7 +1101,9 @@ static void parse_id3(AVFormatContext *s, >> >> AVIOContext *pb, >> >> *dts = ts; >> >> else >> >> av_log(s, AV_LOG_ERROR, "Invalid HLS ID3 audio >> >> timestamp %"PRId64"\n", ts); >> >> - } else if (priv->datasize >= 8 && >> >> !av_strncasecmp(priv->owner, id3_priv_owner_audio_setup, 36)) { >> >> + } else if (priv->datasize >= 8 && >> >> + !av_strncasecmp(priv->owner, >> >> ID3_PRIV_OWNER_AUDIO_SETUP, 36) && >> >> + audio_setup_info) { >> >> ff_hls_senc_read_audio_setup_info(audio_setup_info, >> >> priv->data, priv->datasize); >> >> } >> >> } else if (!strcmp(meta->tag, "APIC") && apic) >> >> @@ -1113,9 +1117,10 @@ static int id3_has_changed_values(struct >> playlist >> >> *pls, AVDictionary *metadata, >> >> { >> >> const AVDictionaryEntry *entry = NULL; >> >> const AVDictionaryEntry *oldentry; >> >> + >> >> /* check that no keys have changed values */ >> >> while ((entry = av_dict_iterate(metadata, entry))) { >> >> - oldentry = av_dict_get(pls->id3_initial, entry->key, NULL, >> >> AV_DICT_MATCH_CASE); >> >> + oldentry = av_dict_get(pls->last_id3, entry->key, NULL, >> >> AV_DICT_MATCH_CASE); >> >> if (!oldentry || strcmp(oldentry->value, entry->value) != 0) >> >> return 1; >> >> } >> >> @@ -1143,35 +1148,40 @@ static void handle_id3(AVIOContext *pb, struct >> >> playlist *pls) >> >> ID3v2ExtraMetaAPIC *apic = NULL; >> >> ID3v2ExtraMeta *extra_meta = NULL; >> >> int64_t timestamp = AV_NOPTS_VALUE; >> >> + // Only set audio_setup_info on first id3 chunk. >> >> + HLSAudioSetupInfo *audio_setup_info = pls->last_id3 ? NULL : >> >> &pls->audio_setup_info; >> >> >> >> - parse_id3(pls->ctx, pb, &metadata, ×tamp, >> >> &pls->audio_setup_info, &apic, &extra_meta); >> >> + parse_id3(pls->ctx, pb, &metadata, ×tamp, audio_setup_info, >> >> &apic, &extra_meta); >> >> >> >> - if (timestamp != AV_NOPTS_VALUE) { >> >> + if (pls->id3_mpegts_timestamp == AV_NOPTS_VALUE && timestamp != >> >> AV_NOPTS_VALUE) { >> >> pls->id3_mpegts_timestamp = timestamp; >> >> pls->id3_offset = 0; >> >> } >> >> >> >> - if (!pls->id3_found) { >> >> - /* initial ID3 tags */ >> >> - av_assert0(!pls->id3_deferred_extra); >> >> - pls->id3_found = 1; >> >> - >> >> + if (id3_has_changed_values(pls, metadata, apic)) { >> >> /* get picture attachment and set text metadata */ >> >> if (pls->ctx->nb_streams) >> >> ff_id3v2_parse_apic(pls->ctx, extra_meta); >> >> - else >> >> + else { >> >> + av_assert0(!pls->id3_deferred_extra); >> >> /* demuxer not yet opened, defer picture attachment */ >> >> pls->id3_deferred_extra = extra_meta; >> >> + } >> >> >> >> ff_id3v2_parse_priv_dict(&metadata, extra_meta); >> >> + >> >> + av_dict_set(&metadata, ID3v2_PRIV_OWNER_TS, NULL, 0); >> >> + av_dict_set(&metadata, ID3v2_PRIV_OWNER_AUDIO_SETUP, NULL, 0); >> >> + >> >> + av_dict_free(&pls->ctx->metadata); >> >> av_dict_copy(&pls->ctx->metadata, metadata, 0); >> >> - pls->id3_initial = metadata; >> >> >> >> + if (pls->n_main_streams) >> >> + av_dict_copy(&pls->main_streams[0]->metadata, metadata, >> 0); >> >> + >> >> + if (pls->last_id3) av_dict_free(&pls->last_id3); >> >> + pls->last_id3 = metadata; >> >> } else { >> >> - if (!pls->id3_changed && id3_has_changed_values(pls, metadata, >> >> apic)) { >> >> - avpriv_report_missing_feature(pls->parent, "Changing ID3 >> >> metadata in HLS audio elementary stream"); >> >> - pls->id3_changed = 1; >> >> - } >> >> av_dict_free(&metadata); >> >> } >> >> >> >> -- >> >> 2.39.3 (Apple Git-145) >> >> >> >> >> > _______________________________________________ >> > ffmpeg-devel mailing list >> > ffmpeg-devel@ffmpeg.org >> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > >> > To unsubscribe, visit link above, or email >> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >> > >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >> >
Romain Beauxis <toots@rastageeks.org> 于2024年3月26日周二 08:58写道： > > This patch adds support for updating HLS metadata passed as ID3 frames. > > This seems like a pretty straight-forward improvement. Updating the > metadaata of the first stream seems to be the mechanism is other places > in the code and works as expected. > > --- > libavformat/hls.c | 54 ++++++++++++++++++++++++++++------------------- > 1 file changed, 32 insertions(+), 22 deletions(-) > > diff --git a/libavformat/hls.c b/libavformat/hls.c > index f6b44c2e35..ba6634d57a 100644 > --- a/libavformat/hls.c > +++ b/libavformat/hls.c > @@ -93,6 +93,12 @@ enum PlaylistType { > PLS_TYPE_VOD > }; > > +#define ID3_PRIV_OWNER_TS "com.apple.streaming.transportStreamTimestamp" > +#define ID3_PRIV_OWNER_AUDIO_SETUP "com.apple.streaming.audioDescription" > + > +#define ID3v2_PRIV_OWNER_TS ID3v2_PRIV_METADATA_PREFIX ID3_PRIV_OWNER_TS > +#define ID3v2_PRIV_OWNER_AUDIO_SETUP ID3v2_PRIV_METADATA_PREFIX ID3_PRIV_OWNER_AUDIO_SETUP > + > /* > * Each playlist has its own demuxer. If it currently is active, > * it has an open AVIOContext too, and potentially an AVPacket > @@ -150,9 +156,7 @@ struct playlist { > int64_t id3_offset; /* in stream original tb */ > uint8_t* id3_buf; /* temp buffer for id3 parsing */ > unsigned int id3_buf_size; > - AVDictionary *id3_initial; /* data from first id3 tag */ > - int id3_found; /* ID3 tag found at some point */ > - int id3_changed; /* ID3 tag data has changed at some point */ > + AVDictionary *last_id3; /* data from the last id3 tag */ > ID3v2ExtraMeta *id3_deferred_extra; /* stored here until subdemuxer is opened */ > > HLSAudioSetupInfo audio_setup_info; > @@ -270,7 +274,7 @@ static void free_playlist_list(HLSContext *c) > av_freep(&pls->main_streams); > av_freep(&pls->renditions); > av_freep(&pls->id3_buf); > - av_dict_free(&pls->id3_initial); > + av_dict_free(&pls->last_id3); > ff_id3v2_free_extra_meta(&pls->id3_deferred_extra); > av_freep(&pls->init_sec_buf); > av_packet_free(&pls->pkt); > @@ -1083,15 +1087,13 @@ static void parse_id3(AVFormatContext *s, AVIOContext *pb, > AVDictionary **metadata, int64_t *dts, HLSAudioSetupInfo *audio_setup_info, > ID3v2ExtraMetaAPIC **apic, ID3v2ExtraMeta **extra_meta) > { > - static const char id3_priv_owner_ts[] = "com.apple.streaming.transportStreamTimestamp"; > - static const char id3_priv_owner_audio_setup[] = "com.apple.streaming.audioDescription"; > ID3v2ExtraMeta *meta; > > ff_id3v2_read_dict(pb, metadata, ID3v2_DEFAULT_MAGIC, extra_meta); > for (meta = *extra_meta; meta; meta = meta->next) { > if (!strcmp(meta->tag, "PRIV")) { > ID3v2ExtraMetaPRIV *priv = &meta->data.priv; > - if (priv->datasize == 8 && !av_strncasecmp(priv->owner, id3_priv_owner_ts, 44)) { > + if (priv->datasize == 8 && !av_strncasecmp(priv->owner, ID3_PRIV_OWNER_TS, strlen(ID3_PRIV_OWNER_TS))) { > /* 33-bit MPEG timestamp */ > int64_t ts = AV_RB64(priv->data); > av_log(s, AV_LOG_DEBUG, "HLS ID3 audio timestamp %"PRId64"\n", ts); > @@ -1099,7 +1101,9 @@ static void parse_id3(AVFormatContext *s, AVIOContext *pb, > *dts = ts; > else > av_log(s, AV_LOG_ERROR, "Invalid HLS ID3 audio timestamp %"PRId64"\n", ts); > - } else if (priv->datasize >= 8 && !av_strncasecmp(priv->owner, id3_priv_owner_audio_setup, 36)) { > + } else if (priv->datasize >= 8 && > + !av_strncasecmp(priv->owner, ID3_PRIV_OWNER_AUDIO_SETUP, 36) && > + audio_setup_info) { > ff_hls_senc_read_audio_setup_info(audio_setup_info, priv->data, priv->datasize); > } > } else if (!strcmp(meta->tag, "APIC") && apic) > @@ -1113,9 +1117,10 @@ static int id3_has_changed_values(struct playlist *pls, AVDictionary *metadata, > { > const AVDictionaryEntry *entry = NULL; > const AVDictionaryEntry *oldentry; > + > /* check that no keys have changed values */ > while ((entry = av_dict_iterate(metadata, entry))) { > - oldentry = av_dict_get(pls->id3_initial, entry->key, NULL, AV_DICT_MATCH_CASE); > + oldentry = av_dict_get(pls->last_id3, entry->key, NULL, AV_DICT_MATCH_CASE); > if (!oldentry || strcmp(oldentry->value, entry->value) != 0) > return 1; > } > @@ -1143,35 +1148,40 @@ static void handle_id3(AVIOContext *pb, struct playlist *pls) > ID3v2ExtraMetaAPIC *apic = NULL; > ID3v2ExtraMeta *extra_meta = NULL; > int64_t timestamp = AV_NOPTS_VALUE; > + // Only set audio_setup_info on first id3 chunk. > + HLSAudioSetupInfo *audio_setup_info = pls->last_id3 ? NULL : &pls->audio_setup_info; > > - parse_id3(pls->ctx, pb, &metadata, ×tamp, &pls->audio_setup_info, &apic, &extra_meta); > + parse_id3(pls->ctx, pb, &metadata, ×tamp, audio_setup_info, &apic, &extra_meta); > > - if (timestamp != AV_NOPTS_VALUE) { > + if (pls->id3_mpegts_timestamp == AV_NOPTS_VALUE && timestamp != AV_NOPTS_VALUE) { > pls->id3_mpegts_timestamp = timestamp; > pls->id3_offset = 0; > } > > - if (!pls->id3_found) { > - /* initial ID3 tags */ > - av_assert0(!pls->id3_deferred_extra); > - pls->id3_found = 1; > - > + if (id3_has_changed_values(pls, metadata, apic)) { > /* get picture attachment and set text metadata */ > if (pls->ctx->nb_streams) > ff_id3v2_parse_apic(pls->ctx, extra_meta); > - else > + else { > + av_assert0(!pls->id3_deferred_extra); > /* demuxer not yet opened, defer picture attachment */ > pls->id3_deferred_extra = extra_meta; > + } > > ff_id3v2_parse_priv_dict(&metadata, extra_meta); > + > + av_dict_set(&metadata, ID3v2_PRIV_OWNER_TS, NULL, 0); > + av_dict_set(&metadata, ID3v2_PRIV_OWNER_AUDIO_SETUP, NULL, 0); > + > + av_dict_free(&pls->ctx->metadata); > av_dict_copy(&pls->ctx->metadata, metadata, 0); > - pls->id3_initial = metadata; > > + if (pls->n_main_streams) > + av_dict_copy(&pls->main_streams[0]->metadata, metadata, 0); > + > + if (pls->last_id3) av_dict_free(&pls->last_id3); > + pls->last_id3 = metadata; > } else { > - if (!pls->id3_changed && id3_has_changed_values(pls, metadata, apic)) { > - avpriv_report_missing_feature(pls->parent, "Changing ID3 metadata in HLS audio elementary stream"); > - pls->id3_changed = 1; > - } > av_dict_free(&metadata); > } > > -- > 2.39.3 (Apple Git-145) > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". I look at the second patch result: make: *** [fate-hls-adts-meta-demux] Error 1 cpu_flags(raw) = 0x000813DB cpu_flags_str(raw) = mmx mmxext sse sse2 sse3 ssse3 sse4.1 sse4.2 cmov aesni cpu_flags(effective) = 0x000813DB cpu_flags_str(effective) = mmx mmxext sse sse2 sse3 ssse3 sse4.1 sse4.2 cmov aesni threads = 1 (cpu_count = 5) make: Target 'fate' not remade because of errors. https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240326005639.27000-2-toots@rastageeks.org/ Thanks Steven
Le dim. 7 avr. 2024 à 05:44, Steven Liu <lingjiujianke@gmail.com> a écrit : > Romain Beauxis <toots@rastageeks.org> 于2024年3月26日周二 08:58写道： > > > > This patch adds support for updating HLS metadata passed as ID3 frames. > > > > This seems like a pretty straight-forward improvement. Updating the > > metadaata of the first stream seems to be the mechanism is other places > > in the code and works as expected. > > > > --- > > libavformat/hls.c | 54 ++++++++++++++++++++++++++++------------------- > > 1 file changed, 32 insertions(+), 22 deletions(-) > > > > diff --git a/libavformat/hls.c b/libavformat/hls.c > > index f6b44c2e35..ba6634d57a 100644 > > --- a/libavformat/hls.c > > +++ b/libavformat/hls.c > > @@ -93,6 +93,12 @@ enum PlaylistType { > > PLS_TYPE_VOD > > }; > > > > +#define ID3_PRIV_OWNER_TS "com.apple.streaming.transportStreamTimestamp" > > +#define ID3_PRIV_OWNER_AUDIO_SETUP > "com.apple.streaming.audioDescription" > > + > > +#define ID3v2_PRIV_OWNER_TS ID3v2_PRIV_METADATA_PREFIX ID3_PRIV_OWNER_TS > > +#define ID3v2_PRIV_OWNER_AUDIO_SETUP ID3v2_PRIV_METADATA_PREFIX > ID3_PRIV_OWNER_AUDIO_SETUP > > + > > /* > > * Each playlist has its own demuxer. If it currently is active, > > * it has an open AVIOContext too, and potentially an AVPacket > > @@ -150,9 +156,7 @@ struct playlist { > > int64_t id3_offset; /* in stream original tb */ > > uint8_t* id3_buf; /* temp buffer for id3 parsing */ > > unsigned int id3_buf_size; > > - AVDictionary *id3_initial; /* data from first id3 tag */ > > - int id3_found; /* ID3 tag found at some point */ > > - int id3_changed; /* ID3 tag data has changed at some point */ > > + AVDictionary *last_id3; /* data from the last id3 tag */ > > ID3v2ExtraMeta *id3_deferred_extra; /* stored here until subdemuxer > is opened */ > > > > HLSAudioSetupInfo audio_setup_info; > > @@ -270,7 +274,7 @@ static void free_playlist_list(HLSContext *c) > > av_freep(&pls->main_streams); > > av_freep(&pls->renditions); > > av_freep(&pls->id3_buf); > > - av_dict_free(&pls->id3_initial); > > + av_dict_free(&pls->last_id3); > > ff_id3v2_free_extra_meta(&pls->id3_deferred_extra); > > av_freep(&pls->init_sec_buf); > > av_packet_free(&pls->pkt); > > @@ -1083,15 +1087,13 @@ static void parse_id3(AVFormatContext *s, > AVIOContext *pb, > > AVDictionary **metadata, int64_t *dts, > HLSAudioSetupInfo *audio_setup_info, > > ID3v2ExtraMetaAPIC **apic, ID3v2ExtraMeta > **extra_meta) > > { > > - static const char id3_priv_owner_ts[] = > "com.apple.streaming.transportStreamTimestamp"; > > - static const char id3_priv_owner_audio_setup[] = > "com.apple.streaming.audioDescription"; > > ID3v2ExtraMeta *meta; > > > > ff_id3v2_read_dict(pb, metadata, ID3v2_DEFAULT_MAGIC, extra_meta); > > for (meta = *extra_meta; meta; meta = meta->next) { > > if (!strcmp(meta->tag, "PRIV")) { > > ID3v2ExtraMetaPRIV *priv = &meta->data.priv; > > - if (priv->datasize == 8 && !av_strncasecmp(priv->owner, > id3_priv_owner_ts, 44)) { > > + if (priv->datasize == 8 && !av_strncasecmp(priv->owner, > ID3_PRIV_OWNER_TS, strlen(ID3_PRIV_OWNER_TS))) { > > /* 33-bit MPEG timestamp */ > > int64_t ts = AV_RB64(priv->data); > > av_log(s, AV_LOG_DEBUG, "HLS ID3 audio timestamp > %"PRId64"\n", ts); > > @@ -1099,7 +1101,9 @@ static void parse_id3(AVFormatContext *s, > AVIOContext *pb, > > *dts = ts; > > else > > av_log(s, AV_LOG_ERROR, "Invalid HLS ID3 audio > timestamp %"PRId64"\n", ts); > > - } else if (priv->datasize >= 8 && > !av_strncasecmp(priv->owner, id3_priv_owner_audio_setup, 36)) { > > + } else if (priv->datasize >= 8 && > > + !av_strncasecmp(priv->owner, > ID3_PRIV_OWNER_AUDIO_SETUP, 36) && > > + audio_setup_info) { > > ff_hls_senc_read_audio_setup_info(audio_setup_info, > priv->data, priv->datasize); > > } > > } else if (!strcmp(meta->tag, "APIC") && apic) > > @@ -1113,9 +1117,10 @@ static int id3_has_changed_values(struct playlist > *pls, AVDictionary *metadata, > > { > > const AVDictionaryEntry *entry = NULL; > > const AVDictionaryEntry *oldentry; > > + > > /* check that no keys have changed values */ > > while ((entry = av_dict_iterate(metadata, entry))) { > > - oldentry = av_dict_get(pls->id3_initial, entry->key, NULL, > AV_DICT_MATCH_CASE); > > + oldentry = av_dict_get(pls->last_id3, entry->key, NULL, > AV_DICT_MATCH_CASE); > > if (!oldentry || strcmp(oldentry->value, entry->value) != 0) > > return 1; > > } > > @@ -1143,35 +1148,40 @@ static void handle_id3(AVIOContext *pb, struct > playlist *pls) > > ID3v2ExtraMetaAPIC *apic = NULL; > > ID3v2ExtraMeta *extra_meta = NULL; > > int64_t timestamp = AV_NOPTS_VALUE; > > + // Only set audio_setup_info on first id3 chunk. > > + HLSAudioSetupInfo *audio_setup_info = pls->last_id3 ? NULL : > &pls->audio_setup_info; > > > > - parse_id3(pls->ctx, pb, &metadata, ×tamp, > &pls->audio_setup_info, &apic, &extra_meta); > > + parse_id3(pls->ctx, pb, &metadata, ×tamp, audio_setup_info, > &apic, &extra_meta); > > > > - if (timestamp != AV_NOPTS_VALUE) { > > + if (pls->id3_mpegts_timestamp == AV_NOPTS_VALUE && timestamp != > AV_NOPTS_VALUE) { > > pls->id3_mpegts_timestamp = timestamp; > > pls->id3_offset = 0; > > } > > > > - if (!pls->id3_found) { > > - /* initial ID3 tags */ > > - av_assert0(!pls->id3_deferred_extra); > > - pls->id3_found = 1; > > - > > + if (id3_has_changed_values(pls, metadata, apic)) { > > /* get picture attachment and set text metadata */ > > if (pls->ctx->nb_streams) > > ff_id3v2_parse_apic(pls->ctx, extra_meta); > > - else > > + else { > > + av_assert0(!pls->id3_deferred_extra); > > /* demuxer not yet opened, defer picture attachment */ > > pls->id3_deferred_extra = extra_meta; > > + } > > > > ff_id3v2_parse_priv_dict(&metadata, extra_meta); > > + > > + av_dict_set(&metadata, ID3v2_PRIV_OWNER_TS, NULL, 0); > > + av_dict_set(&metadata, ID3v2_PRIV_OWNER_AUDIO_SETUP, NULL, 0); > > + > > + av_dict_free(&pls->ctx->metadata); > > av_dict_copy(&pls->ctx->metadata, metadata, 0); > > - pls->id3_initial = metadata; > > > > + if (pls->n_main_streams) > > + av_dict_copy(&pls->main_streams[0]->metadata, metadata, 0); > > + > > + if (pls->last_id3) av_dict_free(&pls->last_id3); > > + pls->last_id3 = metadata; > > } else { > > - if (!pls->id3_changed && id3_has_changed_values(pls, metadata, > apic)) { > > - avpriv_report_missing_feature(pls->parent, "Changing ID3 > metadata in HLS audio elementary stream"); > > - pls->id3_changed = 1; > > - } > > av_dict_free(&metadata); > > } > > > > -- > > 2.39.3 (Apple Git-145) > > > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > I look at the second patch result: > > make: *** [fate-hls-adts-meta-demux] Error 1 > cpu_flags(raw) = 0x000813DB > cpu_flags_str(raw) = mmx mmxext sse sse2 sse3 ssse3 sse4.1 sse4.2 cmov > aesni > cpu_flags(effective) = 0x000813DB > cpu_flags_str(effective) = mmx mmxext sse sse2 sse3 ssse3 sse4.1 > sse4.2 cmov aesni > threads = 1 (cpu_count = 5) > make: Target 'fate' not remade because of errors. > > > > > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240326005639.27000-2-toots@rastageeks.org/ > > Ok. The samples are here: https://www.dropbox.com/scl/fo/1x74ztoa6yo9q49ignfnt/h?rlkey=xvg5nhgjr515gm6b375evm8n4&dl=0 If you place them under $FATE_SAMPLES/hls-adts-meta the test suite should pass. Are you the right person to also upload the samples? Thanks for your time on this! > > Thanks > Steven > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
Hi all, Le dim. 7 avr. 2024 à 09:46, Romain Beauxis <romain.beauxis@gmail.com> a écrit : > > > Le dim. 7 avr. 2024 à 05:44, Steven Liu <lingjiujianke@gmail.com> a > écrit : > >> Romain Beauxis <toots@rastageeks.org> 于2024年3月26日周二 08:58写道： >> > >> > This patch adds support for updating HLS metadata passed as ID3 frames. >> > >> > This seems like a pretty straight-forward improvement. Updating the >> > metadaata of the first stream seems to be the mechanism is other places >> > in the code and works as expected. >> >> Would it be possible to get this patch committed? My understanding is that it has been reviewed so this should be the next step? The second patch adding FATE tests could be committed separately if it causes issues. The samples for it are here: https://www.dropbox.com/scl/fo/1x74ztoa6yo9q49ignfnt/h?rlkey=xvg5nhgjr515gm6b375evm8n4&dl=0 and should be placed into a $FATE_SAMPLES/hls-adts-meta directory. Thanks! > > --- >> > libavformat/hls.c | 54 ++++++++++++++++++++++++++++------------------- >> > 1 file changed, 32 insertions(+), 22 deletions(-) >> > >> > diff --git a/libavformat/hls.c b/libavformat/hls.c >> > index f6b44c2e35..ba6634d57a 100644 >> > --- a/libavformat/hls.c >> > +++ b/libavformat/hls.c >> > @@ -93,6 +93,12 @@ enum PlaylistType { >> > PLS_TYPE_VOD >> > }; >> > >> > +#define ID3_PRIV_OWNER_TS >> "com.apple.streaming.transportStreamTimestamp" >> > +#define ID3_PRIV_OWNER_AUDIO_SETUP >> "com.apple.streaming.audioDescription" >> > + >> > +#define ID3v2_PRIV_OWNER_TS ID3v2_PRIV_METADATA_PREFIX >> ID3_PRIV_OWNER_TS >> > +#define ID3v2_PRIV_OWNER_AUDIO_SETUP ID3v2_PRIV_METADATA_PREFIX >> ID3_PRIV_OWNER_AUDIO_SETUP >> > + >> > /* >> > * Each playlist has its own demuxer. If it currently is active, >> > * it has an open AVIOContext too, and potentially an AVPacket >> > @@ -150,9 +156,7 @@ struct playlist { >> > int64_t id3_offset; /* in stream original tb */ >> > uint8_t* id3_buf; /* temp buffer for id3 parsing */ >> > unsigned int id3_buf_size; >> > - AVDictionary *id3_initial; /* data from first id3 tag */ >> > - int id3_found; /* ID3 tag found at some point */ >> > - int id3_changed; /* ID3 tag data has changed at some point */ >> > + AVDictionary *last_id3; /* data from the last id3 tag */ >> > ID3v2ExtraMeta *id3_deferred_extra; /* stored here until >> subdemuxer is opened */ >> > >> > HLSAudioSetupInfo audio_setup_info; >> > @@ -270,7 +274,7 @@ static void free_playlist_list(HLSContext *c) >> > av_freep(&pls->main_streams); >> > av_freep(&pls->renditions); >> > av_freep(&pls->id3_buf); >> > - av_dict_free(&pls->id3_initial); >> > + av_dict_free(&pls->last_id3); >> > ff_id3v2_free_extra_meta(&pls->id3_deferred_extra); >> > av_freep(&pls->init_sec_buf); >> > av_packet_free(&pls->pkt); >> > @@ -1083,15 +1087,13 @@ static void parse_id3(AVFormatContext *s, >> AVIOContext *pb, >> > AVDictionary **metadata, int64_t *dts, >> HLSAudioSetupInfo *audio_setup_info, >> > ID3v2ExtraMetaAPIC **apic, ID3v2ExtraMeta >> **extra_meta) >> > { >> > - static const char id3_priv_owner_ts[] = >> "com.apple.streaming.transportStreamTimestamp"; >> > - static const char id3_priv_owner_audio_setup[] = >> "com.apple.streaming.audioDescription"; >> > ID3v2ExtraMeta *meta; >> > >> > ff_id3v2_read_dict(pb, metadata, ID3v2_DEFAULT_MAGIC, extra_meta); >> > for (meta = *extra_meta; meta; meta = meta->next) { >> > if (!strcmp(meta->tag, "PRIV")) { >> > ID3v2ExtraMetaPRIV *priv = &meta->data.priv; >> > - if (priv->datasize == 8 && !av_strncasecmp(priv->owner, >> id3_priv_owner_ts, 44)) { >> > + if (priv->datasize == 8 && !av_strncasecmp(priv->owner, >> ID3_PRIV_OWNER_TS, strlen(ID3_PRIV_OWNER_TS))) { >> > /* 33-bit MPEG timestamp */ >> > int64_t ts = AV_RB64(priv->data); >> > av_log(s, AV_LOG_DEBUG, "HLS ID3 audio timestamp >> %"PRId64"\n", ts); >> > @@ -1099,7 +1101,9 @@ static void parse_id3(AVFormatContext *s, >> AVIOContext *pb, >> > *dts = ts; >> > else >> > av_log(s, AV_LOG_ERROR, "Invalid HLS ID3 audio >> timestamp %"PRId64"\n", ts); >> > - } else if (priv->datasize >= 8 && >> !av_strncasecmp(priv->owner, id3_priv_owner_audio_setup, 36)) { >> > + } else if (priv->datasize >= 8 && >> > + !av_strncasecmp(priv->owner, >> ID3_PRIV_OWNER_AUDIO_SETUP, 36) && >> > + audio_setup_info) { >> > ff_hls_senc_read_audio_setup_info(audio_setup_info, >> priv->data, priv->datasize); >> > } >> > } else if (!strcmp(meta->tag, "APIC") && apic) >> > @@ -1113,9 +1117,10 @@ static int id3_has_changed_values(struct >> playlist *pls, AVDictionary *metadata, >> > { >> > const AVDictionaryEntry *entry = NULL; >> > const AVDictionaryEntry *oldentry; >> > + >> > /* check that no keys have changed values */ >> > while ((entry = av_dict_iterate(metadata, entry))) { >> > - oldentry = av_dict_get(pls->id3_initial, entry->key, NULL, >> AV_DICT_MATCH_CASE); >> > + oldentry = av_dict_get(pls->last_id3, entry->key, NULL, >> AV_DICT_MATCH_CASE); >> > if (!oldentry || strcmp(oldentry->value, entry->value) != 0) >> > return 1; >> > } >> > @@ -1143,35 +1148,40 @@ static void handle_id3(AVIOContext *pb, struct >> playlist *pls) >> > ID3v2ExtraMetaAPIC *apic = NULL; >> > ID3v2ExtraMeta *extra_meta = NULL; >> > int64_t timestamp = AV_NOPTS_VALUE; >> > + // Only set audio_setup_info on first id3 chunk. >> > + HLSAudioSetupInfo *audio_setup_info = pls->last_id3 ? NULL : >> &pls->audio_setup_info; >> > >> > - parse_id3(pls->ctx, pb, &metadata, ×tamp, >> &pls->audio_setup_info, &apic, &extra_meta); >> > + parse_id3(pls->ctx, pb, &metadata, ×tamp, audio_setup_info, >> &apic, &extra_meta); >> > >> > - if (timestamp != AV_NOPTS_VALUE) { >> > + if (pls->id3_mpegts_timestamp == AV_NOPTS_VALUE && timestamp != >> AV_NOPTS_VALUE) { >> > pls->id3_mpegts_timestamp = timestamp; >> > pls->id3_offset = 0; >> > } >> > >> > - if (!pls->id3_found) { >> > - /* initial ID3 tags */ >> > - av_assert0(!pls->id3_deferred_extra); >> > - pls->id3_found = 1; >> > - >> > + if (id3_has_changed_values(pls, metadata, apic)) { >> > /* get picture attachment and set text metadata */ >> > if (pls->ctx->nb_streams) >> > ff_id3v2_parse_apic(pls->ctx, extra_meta); >> > - else >> > + else { >> > + av_assert0(!pls->id3_deferred_extra); >> > /* demuxer not yet opened, defer picture attachment */ >> > pls->id3_deferred_extra = extra_meta; >> > + } >> > >> > ff_id3v2_parse_priv_dict(&metadata, extra_meta); >> > + >> > + av_dict_set(&metadata, ID3v2_PRIV_OWNER_TS, NULL, 0); >> > + av_dict_set(&metadata, ID3v2_PRIV_OWNER_AUDIO_SETUP, NULL, 0); >> > + >> > + av_dict_free(&pls->ctx->metadata); >> > av_dict_copy(&pls->ctx->metadata, metadata, 0); >> > - pls->id3_initial = metadata; >> > >> > + if (pls->n_main_streams) >> > + av_dict_copy(&pls->main_streams[0]->metadata, metadata, 0); >> > + >> > + if (pls->last_id3) av_dict_free(&pls->last_id3); >> > + pls->last_id3 = metadata; >> > } else { >> > - if (!pls->id3_changed && id3_has_changed_values(pls, metadata, >> apic)) { >> > - avpriv_report_missing_feature(pls->parent, "Changing ID3 >> metadata in HLS audio elementary stream"); >> > - pls->id3_changed = 1; >> > - } >> > av_dict_free(&metadata); >> > } >> > >> > -- >> > 2.39.3 (Apple Git-145) >> > >> > _______________________________________________ >> > ffmpeg-devel mailing list >> > ffmpeg-devel@ffmpeg.org >> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > >> > To unsubscribe, visit link above, or email >> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >> >> I look at the second patch result: >> >> make: *** [fate-hls-adts-meta-demux] Error 1 >> cpu_flags(raw) = 0x000813DB >> cpu_flags_str(raw) = mmx mmxext sse sse2 sse3 ssse3 sse4.1 sse4.2 cmov >> aesni >> cpu_flags(effective) = 0x000813DB >> cpu_flags_str(effective) = mmx mmxext sse sse2 sse3 ssse3 sse4.1 >> sse4.2 cmov aesni >> threads = 1 (cpu_count = 5) >> make: Target 'fate' not remade because of errors. >> >> >> >> >> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240326005639.27000-2-toots@rastageeks.org/ >> >> > Ok. The samples are here: > https://www.dropbox.com/scl/fo/1x74ztoa6yo9q49ignfnt/h?rlkey=xvg5nhgjr515gm6b375evm8n4&dl=0 > > If you place them under $FATE_SAMPLES/hls-adts-meta the test suite should > pass. > > Are you the right person to also upload the samples? > > Thanks for your time on this! > > >> >> Thanks >> Steven >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >> >
diff --git a/libavformat/hls.c b/libavformat/hls.c index f6b44c2e35..ba6634d57a 100644 --- a/libavformat/hls.c +++ b/libavformat/hls.c @@ -93,6 +93,12 @@ enum PlaylistType { PLS_TYPE_VOD }; +#define ID3_PRIV_OWNER_TS "com.apple.streaming.transportStreamTimestamp" +#define ID3_PRIV_OWNER_AUDIO_SETUP "com.apple.streaming.audioDescription" + +#define ID3v2_PRIV_OWNER_TS ID3v2_PRIV_METADATA_PREFIX ID3_PRIV_OWNER_TS +#define ID3v2_PRIV_OWNER_AUDIO_SETUP ID3v2_PRIV_METADATA_PREFIX ID3_PRIV_OWNER_AUDIO_SETUP + /* * Each playlist has its own demuxer. If it currently is active, * it has an open AVIOContext too, and potentially an AVPacket @@ -150,9 +156,7 @@ struct playlist { int64_t id3_offset; /* in stream original tb */ uint8_t* id3_buf; /* temp buffer for id3 parsing */ unsigned int id3_buf_size; - AVDictionary *id3_initial; /* data from first id3 tag */ - int id3_found; /* ID3 tag found at some point */ - int id3_changed; /* ID3 tag data has changed at some point */ + AVDictionary *last_id3; /* data from the last id3 tag */ ID3v2ExtraMeta *id3_deferred_extra; /* stored here until subdemuxer is opened */ HLSAudioSetupInfo audio_setup_info; @@ -270,7 +274,7 @@ static void free_playlist_list(HLSContext *c) av_freep(&pls->main_streams); av_freep(&pls->renditions); av_freep(&pls->id3_buf); - av_dict_free(&pls->id3_initial); + av_dict_free(&pls->last_id3); ff_id3v2_free_extra_meta(&pls->id3_deferred_extra); av_freep(&pls->init_sec_buf); av_packet_free(&pls->pkt); @@ -1083,15 +1087,13 @@ static void parse_id3(AVFormatContext *s, AVIOContext *pb, AVDictionary **metadata, int64_t *dts, HLSAudioSetupInfo *audio_setup_info, ID3v2ExtraMetaAPIC **apic, ID3v2ExtraMeta **extra_meta) { - static const char id3_priv_owner_ts[] = "com.apple.streaming.transportStreamTimestamp"; - static const char id3_priv_owner_audio_setup[] = "com.apple.streaming.audioDescription"; ID3v2ExtraMeta *meta; ff_id3v2_read_dict(pb, metadata, ID3v2_DEFAULT_MAGIC, extra_meta); for (meta = *extra_meta; meta; meta = meta->next) { if (!strcmp(meta->tag, "PRIV")) { ID3v2ExtraMetaPRIV *priv = &meta->data.priv; - if (priv->datasize == 8 && !av_strncasecmp(priv->owner, id3_priv_owner_ts, 44)) { + if (priv->datasize == 8 && !av_strncasecmp(priv->owner, ID3_PRIV_OWNER_TS, strlen(ID3_PRIV_OWNER_TS))) { /* 33-bit MPEG timestamp */ int64_t ts = AV_RB64(priv->data); av_log(s, AV_LOG_DEBUG, "HLS ID3 audio timestamp %"PRId64"\n", ts); @@ -1099,7 +1101,9 @@ static void parse_id3(AVFormatContext *s, AVIOContext *pb, *dts = ts; else av_log(s, AV_LOG_ERROR, "Invalid HLS ID3 audio timestamp %"PRId64"\n", ts); - } else if (priv->datasize >= 8 && !av_strncasecmp(priv->owner, id3_priv_owner_audio_setup, 36)) { + } else if (priv->datasize >= 8 && + !av_strncasecmp(priv->owner, ID3_PRIV_OWNER_AUDIO_SETUP, 36) && + audio_setup_info) { ff_hls_senc_read_audio_setup_info(audio_setup_info, priv->data, priv->datasize); } } else if (!strcmp(meta->tag, "APIC") && apic) @@ -1113,9 +1117,10 @@ static int id3_has_changed_values(struct playlist *pls, AVDictionary *metadata, { const AVDictionaryEntry *entry = NULL; const AVDictionaryEntry *oldentry; + /* check that no keys have changed values */ while ((entry = av_dict_iterate(metadata, entry))) { - oldentry = av_dict_get(pls->id3_initial, entry->key, NULL, AV_DICT_MATCH_CASE); + oldentry = av_dict_get(pls->last_id3, entry->key, NULL, AV_DICT_MATCH_CASE); if (!oldentry || strcmp(oldentry->value, entry->value) != 0) return 1; } @@ -1143,35 +1148,40 @@ static void handle_id3(AVIOContext *pb, struct playlist *pls) ID3v2ExtraMetaAPIC *apic = NULL; ID3v2ExtraMeta *extra_meta = NULL; int64_t timestamp = AV_NOPTS_VALUE; + // Only set audio_setup_info on first id3 chunk. + HLSAudioSetupInfo *audio_setup_info = pls->last_id3 ? NULL : &pls->audio_setup_info; - parse_id3(pls->ctx, pb, &metadata, ×tamp, &pls->audio_setup_info, &apic, &extra_meta); + parse_id3(pls->ctx, pb, &metadata, ×tamp, audio_setup_info, &apic, &extra_meta); - if (timestamp != AV_NOPTS_VALUE) { + if (pls->id3_mpegts_timestamp == AV_NOPTS_VALUE && timestamp != AV_NOPTS_VALUE) { pls->id3_mpegts_timestamp = timestamp; pls->id3_offset = 0; } - if (!pls->id3_found) { - /* initial ID3 tags */ - av_assert0(!pls->id3_deferred_extra); - pls->id3_found = 1; - + if (id3_has_changed_values(pls, metadata, apic)) { /* get picture attachment and set text metadata */ if (pls->ctx->nb_streams) ff_id3v2_parse_apic(pls->ctx, extra_meta); - else + else { + av_assert0(!pls->id3_deferred_extra); /* demuxer not yet opened, defer picture attachment */ pls->id3_deferred_extra = extra_meta; + } ff_id3v2_parse_priv_dict(&metadata, extra_meta); + + av_dict_set(&metadata, ID3v2_PRIV_OWNER_TS, NULL, 0); + av_dict_set(&metadata, ID3v2_PRIV_OWNER_AUDIO_SETUP, NULL, 0); + + av_dict_free(&pls->ctx->metadata); av_dict_copy(&pls->ctx->metadata, metadata, 0); - pls->id3_initial = metadata; + if (pls->n_main_streams) + av_dict_copy(&pls->main_streams[0]->metadata, metadata, 0); + + if (pls->last_id3) av_dict_free(&pls->last_id3); + pls->last_id3 = metadata; } else { - if (!pls->id3_changed && id3_has_changed_values(pls, metadata, apic)) { - avpriv_report_missing_feature(pls->parent, "Changing ID3 metadata in HLS audio elementary stream"); - pls->id3_changed = 1; - } av_dict_free(&metadata); }