Re: [FFmpeg-devel] [PATCH 1/2] libavformat/hls.c: support in-stream ID3 metadata update.

[Romain Beauxis <romain.beauxis@gmail.com>]

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, &timestamp,
> &pls->audio_setup_info, &apic, &extra_meta);
> > +    parse_id3(pls->ctx, pb, &metadata, &timestamp, 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".
>
_______________________________________________
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".