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 AC7B440C8E for ; Sun, 31 Mar 2024 17:46:44 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id D079B68CCAA; Sun, 31 Mar 2024 20:46:41 +0300 (EEST) Received: from mail-pf1-f179.google.com (mail-pf1-f179.google.com [209.85.210.179]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 27FC268C09C for ; Sun, 31 Mar 2024 20:46:36 +0300 (EEST) Received: by mail-pf1-f179.google.com with SMTP id d2e1a72fcca58-6ea7eff5f3cso469996b3a.0 for ; Sun, 31 Mar 2024 10:46:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1711907194; x=1712511994; darn=ffmpeg.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=kXOntdUYymmzxw6AmfN3D/Gw8Mb4c4tzO+6akyGwOWo=; b=ZV2iVMALZh36DT3UOQTc71/3iKjAXvVWchGGL84MXcCg5vcs8D910BiB56bJHaykHe zttvefkIbQ9sTOiwftBpefOuRgRvK8lyTwYn6plXu9LxeQixx4HsI/sf7nxeFr4m1u2j CX64zteX7zQdqXwvmdWTwvvaO0d0jA99GIIUusKEqsy9Zt6YvAkSWZ9qTRqd9HjpWLHd TOfUeqWSwq/JfrsVZvdQ5rV3v7e7qukz3XZlSixb9gt2XKYpqdX6PNoYHWycKMJyRMNi OqzbAwrUOWfa52mJAU2HN6OPo2JgsOEX9UcRTkLd/wBNNWpqtpxrQcGweyP7I62ZhIKk MeLw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711907194; x=1712511994; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=kXOntdUYymmzxw6AmfN3D/Gw8Mb4c4tzO+6akyGwOWo=; b=pI+PakfOptuDTolT6LTJwgcaaFOfr3yauEoxxIutyZWi0B8strF7soVhSVhVDRdi9X KNWeA2YMZVBTBP7EuZZIdMI3UWcgLeXNcB4AlrYQaPWFz9ycgtk+6aVCDCqsmDwzQ83Y 7KpxelKNciKhcew8Et05r1hnXe9UUPVSnD8E1eXf245chsGCa6mITlQVoy+BvD8IkVWz KWBCr2QiXXJVTqtwPw+C+nPaGejJTa0sJh3kFbLo8hTB4KC0B/cKLn2nly6nonJS8MEs dB4gMfSChNmmbyQwqNPp9IvEwFIzAighHPXX6U+QMAWLWWExrLh6AzAY+vOatDFQYj5H /BhQ== X-Gm-Message-State: AOJu0Yztf1ODZt6xFpGaPDmeMYSPx3IdfpIbyTrwy34jgn7wJgMXlGvt PTMvMyUP8AErTS9yO5fqZyMEBaK24ik4HrQ4hQt/7WTIMMKMYLpu5I072hADfUOEs7rjF6GZ0cm RkmW6IjGvRNiCBiq8cHFxA3YOjdka8+VZ X-Google-Smtp-Source: AGHT+IHdFLSyF4YEwtC/IqBX4z1OcpHWWO1x2DPhQlnZecPAodS+cr2NBdrb6OO3t0AX75lZ/d91/LRBM3Z2AXKp1S0= X-Received: by 2002:a05:6a21:998d:b0:1a3:6c0a:4f9b with SMTP id ve13-20020a056a21998d00b001a36c0a4f9bmr10715521pzb.2.1711907193663; Sun, 31 Mar 2024 10:46:33 -0700 (PDT) MIME-Version: 1.0 References: <20240326005639.27000-1-toots@rastageeks.org> <7E56E890-A099-4833-AAE6-5F3916157848@chinaffmpeg.org> In-Reply-To: <7E56E890-A099-4833-AAE6-5F3916157848@chinaffmpeg.org> From: Romain Beauxis Date: Sun, 31 Mar 2024 12:46:16 -0500 Message-ID: To: FFmpeg development discussions and patches X-Content-Filtered-By: Mailman/MimeDel 2.1.29 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 Sun, Mar 31, 2024, 05:52 Liu Steven wrote: > > > > 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. > 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". > _______________________________________________ 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".