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 5649A47FED for ; Sat, 6 Apr 2024 17:48:49 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 296D268CF9D; Sat, 6 Apr 2024 20:48:47 +0300 (EEST) Received: from mail-pg1-f177.google.com (mail-pg1-f177.google.com [209.85.215.177]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 8A04068CB28 for ; Sat, 6 Apr 2024 20:48:41 +0300 (EEST) Received: by mail-pg1-f177.google.com with SMTP id 41be03b00d2f7-55b5a37acb6so754980a12.0 for ; Sat, 06 Apr 2024 10:48:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1712425719; x=1713030519; 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=CelTNcpHqCRyMGXx29fk0kpUiGACn2lkAssXLWVVf+U=; b=ePUsLW6sxyb81Mmt7p9Mu10ETIrfKVGm8+wNW0a20lbU2eU2AbHYMzYWSW3q76QdCM uY1lkSh9OIQN9P43WOhJY+Mg5K06RMe74X0JBAG3TMTy/SxW0O84fHVyOrDP8gEjsKCh OtNqgGh4U9ZqDk/XoLoCzo2NuTgquagilRxZPzUD74T7/985RI1A3ZZgrDhdIYElS0rr GWR+ofMrwF6UHbXVklBU/Ihi3dlTvsYc+jKVGvUBeGWQu2GkEDRaxQ2fufpRW26qesvk WnooYaPwSAvG56U6CA72+xkKvsIC36JYsrlh1JOcyeNrQT+g9ljxzy9XtSNGX/3ssXtu C/Ew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712425719; x=1713030519; 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=CelTNcpHqCRyMGXx29fk0kpUiGACn2lkAssXLWVVf+U=; b=aVWxrLYNcAm7xUgWMlg5q2Fj8nPDCEL+7iYbogK9YoVEyKJHeAgl0q68YpgMVJAIlr CNoSRiBHwYg5Rze8c4J8TdLJuH5RYP3wNeFns5fRrWFwN2xMGHiXtche/l8li5jztls3 tgip8/Iy5SD3J/9o5YlvWVRX9E21UT/H9nBQZ/y5tKQyMwU7+IgsY+GPj0UkD3g6PTVi Yfbe1MxXMu6yaJ+8L3Jnvtm+KrDRN7rdNkGhrEBe4klb6bDXiWl+WzS04fmC1ev413ug VaB3wt1ZzepF5CwuO0Supwh9KYDdfEQTrhqeXgvC+i8xYA6xQPYp3P0mAWdM2GtWjoNp iHoQ== X-Gm-Message-State: AOJu0YyLLgpLAuoyJlLIbbSGeaLu7YQFGY208L8klQ86DdPs8vccdWrh kgPaYAHeRuJtq7BNV1k2+5g687kjef2wgVM3Y4SfCIf68/hrQNkaSrrjFH0fPhqm5vRA0ED7rVm PgzQRJ45s3s9DIT+vjPlfYXRcVsaqyHPe X-Google-Smtp-Source: AGHT+IG1GyxOC4LVND4CWjWtEoWOWSQH4zu6r/W4mqfv/oZNHXDQgapG2T7a0ZdOkrOawayzj8WIwpxlec//IALh4S0= X-Received: by 2002:a05:6a20:2452:b0:1a7:17d:8863 with SMTP id t18-20020a056a20245200b001a7017d8863mr5922975pzc.4.1712425718902; Sat, 06 Apr 2024 10:48:38 -0700 (PDT) MIME-Version: 1.0 References: <20240326005639.27000-1-toots@rastageeks.org> <7E56E890-A099-4833-AAE6-5F3916157848@chinaffmpeg.org> In-Reply-To: From: Romain Beauxis Date: Sat, 6 Apr 2024 12:48:27 -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, 12:46 Romain Beauxis wrote: > > > 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. > > 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". >> > _______________________________________________ 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".