From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 8AA204A5D3 for ; Sun, 31 Mar 2024 10:52:43 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 5F79B68CB04; Sun, 31 Mar 2024 13:52:40 +0300 (EEST) Received: from bg4.exmail.qq.com (bg4.exmail.qq.com [43.154.54.12]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id A3DD368C0F8 for ; Sun, 31 Mar 2024 13:52:30 +0300 (EEST) X-QQ-mid: bizesmtp79t1711882343t6v3uj0j X-QQ-Originating-IP: K+iTDmiM8zmE4pruaPoTFUag79xzKix7STI3UCrpat8= Received: from smtpclient.apple ( [111.196.213.76]) by bizesmtp.qq.com (ESMTP) with id ; Sun, 31 Mar 2024 18:52:21 +0800 (CST) X-QQ-SSF: 01100000000000Z0Z000000A0000000 X-QQ-FEAT: znfcQSa1hKaSOwMcQEtUMNtCB7AGrm14Q2DDLFzNy4BKC9uuOKMlVBz/JOIkH 59vS+kgWGHyAtte1Sf7yad4tud9kagfECu7rFfUhaX54BEWVZpuxE1YV2+JFq7oJ5BsUrjO Nnt9wJH/rJHM2J8VauzndXLzj2m2Ck3j9KzLdTzZJAVt5xdVEm9b4KNBd4k4Ar71o7YoIqq uducyCblUs+74pmar7YhatO+wSyAQqX6X4qHAoWARMTtM+mKfl9LP5wyP+4bl4ttZ1upMxA O+foXQToIDOcOKOr+JnIYM3BrrBnBfgWJRrinpeehRFXSXqsW2AzNPVdaFgV3/WhDlW+l/a Ph+Ai8M/Z1y6UycsXGTulu6VqlxwnnIsg4apNXyEoWXnujUv4togwkTF2RlMqVFbq9ccIfq X-QQ-GoodBg: 0 X-BIZMAIL-ID: 5805975619234331279 Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3774.400.31\)) From: Liu Steven In-Reply-To: Date: Sun, 31 Mar 2024 18:52:11 +0800 Message-Id: <7E56E890-A099-4833-AAE6-5F3916157848@chinaffmpeg.org> References: <20240326005639.27000-1-toots@rastageeks.org> To: FFmpeg development discussions and patches X-Mailer: Apple Mail (2.3774.400.31) X-QQ-SENDSIZE: 520 Feedback-ID: bizesmtp:chinaffmpeg.org:qybglogicsvrsz:qybglogicsvrsz3a-0 Subject: Re: [FFmpeg-devel] [PATCH 1/2] libavformat/hls.c: support in-stream ID3 metadata update. X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Liu Steven Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: > On Mar 29, 2024, at 06:51, Romain Beauxis wrote: > > On Mon, Mar 25, 2024, 19:58 Romain Beauxis 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". > _______________________________________________ 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".