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 41D2840DB1 for ; Sun, 5 Feb 2023 10:09:14 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id B15D868BE97; Sun, 5 Feb 2023 12:09:11 +0200 (EET) Received: from mail-vs1-f53.google.com (mail-vs1-f53.google.com [209.85.217.53]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id A18DF68BE03 for ; Sun, 5 Feb 2023 12:09:05 +0200 (EET) Received: by mail-vs1-f53.google.com with SMTP id m1so7962983vst.7 for ; Sun, 05 Feb 2023 02:09:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=to:subject:message-id:date:from:references:in-reply-to:mime-version :from:to:cc:subject:date:message-id:reply-to; bh=vBin7NNkT6LiJy+5KGjdwjEd0QRv8WckST/tt4IPhzE=; b=P8+bMJ95+m/ImwB3Qf0B28wWQFhdh1T784U2NM/gNmFRSnhcvXMI4DwtMiIjzjnxw2 oVZ1ZzeU+rbuvyd+Fdzsq+AEfj/rlDqfGONJX/DmV6QW/ZZL1xH1r1nYwIceRhRS5Zb9 Ms8zmIENjtaGmaMamBziBSiBlQnDNySLrwof8sDHmSpJMnWy/BgUR6+MoR7ToQ7utxkW 2UswXuAMJkaPt/UVlIG61Yg2OR1L1jM+VhbGPNbT3d3ISBhglVVeS62OJVAF3iK9X7BC k7YK7glsS2Q3DXjat3sY0BKD3717eXkv2WEVyEtKXGYm84t1gQIcQDgTXlidLenwk+Ct I37Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:subject:message-id:date:from:references:in-reply-to:mime-version :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=vBin7NNkT6LiJy+5KGjdwjEd0QRv8WckST/tt4IPhzE=; b=1/QmuPjo9nJUXyOhOXpvBlWojK+GChMGETGQ/kjjDQrym4LaJSIRHzXmDx+f12k5u5 UhYUZTnco44vzgw34RAWhWUVbGXsvYd1EZ1P0W+OpKc1/xNCwxxmqHe/TwhbMt+T6XNO RGCFEZdEQdE/viNJDgiO/JVwQGTZoJD1GyFtSQNG/jslOtE4moWOwkzTLEfMkVtNjSji kUcyntCAbjx+9ugbpSvCYD85fxVGQQc09g8D2MnrFEsN2O/V1pxQ5jJDraWaXtiWrASL vTn2clgYXsrdTIjW3yudt++Bud/sHrjmS/Z5QKPqqzlGD4mrEpe0XIifj859ytNnHmHm O0LA== X-Gm-Message-State: AO0yUKUiF3zx+IUvmUZTWuhmezt1US1QnP5ao8E77NFToZiIlU/ysgLV pOIk7+Ha5e/XSzj3QFOGinMgCG1nhbH56F873BjsQ3S7 X-Google-Smtp-Source: AK7set/E33BnQyCI+nGmvZKa92+nTVFVzmAnV2NXjafMTDgof0X4lPJXdFnUiZ74XzN2zuxATPiZLgbklqObeCDKLno= X-Received: by 2002:a67:c01a:0:b0:3f2:a71c:fb8f with SMTP id v26-20020a67c01a000000b003f2a71cfb8fmr2251775vsi.57.1675591743916; Sun, 05 Feb 2023 02:09:03 -0800 (PST) MIME-Version: 1.0 Received: by 2002:a05:612c:2428:b0:32a:5eaa:92e4 with HTTP; Sun, 5 Feb 2023 02:09:02 -0800 (PST) In-Reply-To: References: <20230202160708.29461-1-ffmpeg@gyani.pro> <20230202160708.29461-2-ffmpeg@gyani.pro> <6a0bb352-0971-42a4-8f34-8637dda46ccc@gyani.pro> From: Paul B Mahol Date: Sun, 5 Feb 2023 11:09:02 +0100 Message-ID: To: FFmpeg development discussions and patches Subject: Re: [FFmpeg-devel] [PATCH v2 2/2] avformat/flvenc: add option to read metadata from file 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 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 2/5/23, Gyan Doshi wrote: > > > On 2023-02-04 04:12 pm, Gyan Doshi wrote: >> >> >> On 2023-02-04 04:02 pm, Paul B Mahol wrote: >>> On 2/4/23, Gyan Doshi wrote: >>>> >>>> On 2023-02-04 03:46 pm, Paul B Mahol wrote: >>>>> On 2/4/23, Gyan Doshi wrote: >>>>>> On 2023-02-03 09:04 pm, Andreas Rheinhardt wrote: >>>>>>> Gyan Doshi: >>>>>>>> Useful, in conjuntion with option meta_period, to vary metadata >>>>>>>> during >>>>>>>> stream. >>>>>>>> >>>>>>>> File format is ffmetadata. >>>>>>>> --- >>>>>>>> configure | 2 +- >>>>>>>> doc/muxers.texi | 3 + >>>>>>>> libavformat/flvenc.c | 133 >>>>>>>> +++++++++++++++++++++++++++++++++---------- >>>>>>>> 3 files changed, 107 insertions(+), 31 deletions(-) >>>>>>>> >>>>>>>> diff --git a/configure b/configure >>>>>>>> index 9d78a244a3..de371632c4 100755 >>>>>>>> --- a/configure >>>>>>>> +++ b/configure >>>>>>>> @@ -3433,7 +3433,7 @@ eac3_demuxer_select="ac3_parser" >>>>>>>> f4v_muxer_select="mov_muxer" >>>>>>>> fifo_muxer_deps="threads" >>>>>>>> flac_demuxer_select="flac_parser" >>>>>>>> -flv_muxer_select="aac_adtstoasc_bsf" >>>>>>>> +flv_muxer_select="aac_adtstoasc_bsf ffmetadata_demuxer" >>>>>>>> gxf_muxer_select="pcm_rechunk_bsf" >>>>>>>> hds_muxer_select="flv_muxer" >>>>>>>> hls_demuxer_select="adts_header ac3_parser" >>>>>>>> diff --git a/doc/muxers.texi b/doc/muxers.texi >>>>>>>> index 02ecddf186..000c92b2a7 100644 >>>>>>>> --- a/doc/muxers.texi >>>>>>>> +++ b/doc/muxers.texi >>>>>>>> @@ -555,6 +555,9 @@ With every video packet. >>>>>>>> @end table >>>>>>>> Note that metadata will always be re-emitted if a metadata >>>>>>>> update >>>>>>>> event >>>>>>>> is signalled. >>>>>>>> >>>>>>>> +@item meta_filename >>>>>>>> +Specify a ffmetadata file from which to load metadata. >>>>>>>> + >>>>>>>> @end table >>>>>>>> >>>>>>>> @anchor{framecrc} >>>>>>>> diff --git a/libavformat/flvenc.c b/libavformat/flvenc.c >>>>>>>> index d1c7a493d1..1332b18b41 100644 >>>>>>>> --- a/libavformat/flvenc.c >>>>>>>> +++ b/libavformat/flvenc.c >>>>>>>> @@ -122,6 +122,10 @@ typedef struct FLVContext { >>>>>>>> double framerate; >>>>>>>> AVCodecParameters *data_par; >>>>>>>> >>>>>>>> + char *meta_filename; >>>>>>>> + AVFormatContext *meta_ctx; >>>>>>>> + AVDictionary *meta_dict; >>>>>>>> + >>>>>>>> int flags; >>>>>>>> int meta_period; >>>>>>>> } FLVContext; >>>>>>>> @@ -277,6 +281,92 @@ static void put_amf_bool(AVIOContext *pb, >>>>>>>> int b) >>>>>>>> avio_w8(pb, !!b); >>>>>>>> } >>>>>>>> >>>>>>>> +static int read_metadata_from_file(AVFormatContext *s, unsigned >>>>>>>> int >>>>>>>> ts, >>>>>>>> int header) >>>>>>>> +{ >>>>>>>> + FLVContext *flv = s->priv_data; >>>>>>>> + const AVInputFormat *meta_format = NULL; >>>>>>>> + AVDictionary *current_meta_dict = NULL; >>>>>>>> + float timestamp = ts/1000.f; >>>>>>>> + int ret; >>>>>>>> + >>>>>>>> + if (!flv->meta_filename) >>>>>>>> + return 0; >>>>>>>> + >>>>>>>> + meta_format = av_find_input_format("ffmetadata"); >>>>>>>> + if (!meta_format) { >>>>>>>> + av_log(s, AV_LOG_ERROR, "ffmetadata demuxer not >>>>>>>> found.\n"); >>>>>>>> + return AVERROR(ENOSYS); >>>>>>>> + } >>>>>>>> + >>>>>>>> + avformat_close_input(&flv->meta_ctx); >>>>>>>> + >>>>>>>> + ret = avformat_open_input(&flv->meta_ctx, flv->meta_filename, >>>>>>>> meta_format, NULL); >>>>>>>> + if (ret < 0) { >>>>>>>> + av_log(s, AV_LOG_ERROR, "Failed to read metadata from >>>>>>>> file %s >>>>>>>> t: >>>>>>>> %f.", flv->meta_filename, timestamp); >>>>>>>> + if (flv->meta_dict) >>>>>>>> + av_log(s, AV_LOG_ERROR, " Continuing with old >>>>>>>> metadata."); >>>>>>>> + av_log(s, AV_LOG_ERROR, "\n"); >>>>>>>> + return ret; >>>>>>>> + } >>>>>>>> + >>>>>>>> + if (flv->meta_dict) { >>>>>>>> + av_dict_copy(¤t_meta_dict, flv->meta_dict, 0); >>>>>>>> + av_dict_free(&flv->meta_dict); >>>>>>>> + } >>>>>>>> + >>>>>>>> + ret = av_dict_copy(&flv->meta_dict, >>>>>>>> flv->meta_ctx->metadata, 0); >>>>>>>> + if (ret < 0) { >>>>>>>> + av_log(s, AV_LOG_ERROR, "Could not transfer metadata >>>>>>>> from %s >>>>>>>> at >>>>>>>> %f seconds. Continuing with old metadata.\n", flv->meta_filename, >>>>>>>> timestamp); >>>>>>>> + av_dict_free(&flv->meta_dict); >>>>>>>> + av_dict_copy(&flv->meta_dict, current_meta_dict, 0); >>>>>>>> + av_dict_free(¤t_meta_dict); >>>>>>>> + return ret; >>>>>>>> + } >>>>>>>> + >>>>>>>> + av_log(s, AV_LOG_VERBOSE, "Metadata from file %s updated %s >>>>>>>> at %f >>>>>>>> seconds.\n", flv->meta_filename, header ? "in header" : "in video >>>>>>>> packet", timestamp); >>>>>>>> + av_dict_free(¤t_meta_dict); >>>>>>>> + avformat_close_input(&flv->meta_ctx); >>>>>>>> + >>>>>>>> + return 0; >>>>>>>> +} >>>>>>>> + >>>>>>>> +static int write_user_metadata_tag(AVFormatContext *s, >>>>>>>> AVDictionaryEntry >>>>>>>> *tag, AVIOContext *pb, int *metadata_count) >>>>>>>> +{ >>>>>>>> + >>>>>>>> + av_log(s, AV_LOG_DEBUG, "Writing tag %s with value %s count: >>>>>>>> %d\n", >>>>>>>> tag->key, tag->value, *metadata_count); >>>>>>>> + >>>>>>>> + if( !strcmp(tag->key, "width") >>>>>>>> + ||!strcmp(tag->key, "height") >>>>>>>> + ||!strcmp(tag->key, "videodatarate") >>>>>>>> + ||!strcmp(tag->key, "framerate") >>>>>>>> + ||!strcmp(tag->key, "videocodecid") >>>>>>>> + ||!strcmp(tag->key, "audiodatarate") >>>>>>>> + ||!strcmp(tag->key, "audiosamplerate") >>>>>>>> + ||!strcmp(tag->key, "audiosamplesize") >>>>>>>> + ||!strcmp(tag->key, "stereo") >>>>>>>> + ||!strcmp(tag->key, "audiocodecid") >>>>>>>> + ||!strcmp(tag->key, "duration") >>>>>>>> + ||!strcmp(tag->key, "onMetaData") >>>>>>>> + ||!strcmp(tag->key, "datasize") >>>>>>>> + ||!strcmp(tag->key, "lasttimestamp") >>>>>>>> + ||!strcmp(tag->key, "totalframes") >>>>>>>> + ||!strcmp(tag->key, "hasAudio") >>>>>>>> + ||!strcmp(tag->key, "hasVideo") >>>>>>>> + ||!strcmp(tag->key, "hasCuePoints") >>>>>>>> + ||!strcmp(tag->key, "hasMetadata") >>>>>>>> + ||!strcmp(tag->key, "hasKeyframes") >>>>>>>> + ){ >>>>>>>> + av_log(s, AV_LOG_DEBUG, "Ignoring metadata for %s\n", >>>>>>>> tag->key); >>>>>>>> + return AVERROR(EINVAL); >>>>>>>> + } >>>>>>>> + put_amf_string(pb, tag->key); >>>>>>>> + avio_w8(pb, AMF_DATA_TYPE_STRING); >>>>>>>> + put_amf_string(pb, tag->value); >>>>>>>> + (*metadata_count)++; >>>>>>>> + >>>>>>>> + return 0; >>>>>>>> +} >>>>>>>> + >>>>>>>> static void write_metadata(AVFormatContext *s, unsigned int ts) >>>>>>>> { >>>>>>>> AVIOContext *pb = s->pb; >>>>>>>> @@ -360,36 +450,11 @@ static void write_metadata(AVFormatContext >>>>>>>> *s, >>>>>>>> unsigned int ts) >>>>>>>> } >>>>>>>> >>>>>>>> ff_standardize_creation_time(s); >>>>>>>> - while ((tag = av_dict_iterate(s->metadata, tag))) { >>>>>>>> - if( !strcmp(tag->key, "width") >>>>>>>> - ||!strcmp(tag->key, "height") >>>>>>>> - ||!strcmp(tag->key, "videodatarate") >>>>>>>> - ||!strcmp(tag->key, "framerate") >>>>>>>> - ||!strcmp(tag->key, "videocodecid") >>>>>>>> - ||!strcmp(tag->key, "audiodatarate") >>>>>>>> - ||!strcmp(tag->key, "audiosamplerate") >>>>>>>> - ||!strcmp(tag->key, "audiosamplesize") >>>>>>>> - ||!strcmp(tag->key, "stereo") >>>>>>>> - ||!strcmp(tag->key, "audiocodecid") >>>>>>>> - ||!strcmp(tag->key, "duration") >>>>>>>> - ||!strcmp(tag->key, "onMetaData") >>>>>>>> - ||!strcmp(tag->key, "datasize") >>>>>>>> - ||!strcmp(tag->key, "lasttimestamp") >>>>>>>> - ||!strcmp(tag->key, "totalframes") >>>>>>>> - ||!strcmp(tag->key, "hasAudio") >>>>>>>> - ||!strcmp(tag->key, "hasVideo") >>>>>>>> - ||!strcmp(tag->key, "hasCuePoints") >>>>>>>> - ||!strcmp(tag->key, "hasMetadata") >>>>>>>> - ||!strcmp(tag->key, "hasKeyframes") >>>>>>>> - ){ >>>>>>>> - av_log(s, AV_LOG_DEBUG, "Ignoring metadata for %s\n", >>>>>>>> tag->key); >>>>>>>> - continue; >>>>>>>> - } >>>>>>>> - put_amf_string(pb, tag->key); >>>>>>>> - avio_w8(pb, AMF_DATA_TYPE_STRING); >>>>>>>> - put_amf_string(pb, tag->value); >>>>>>>> - metadata_count++; >>>>>>>> - } >>>>>>>> + >>>>>>>> + while (tag = av_dict_get(s->metadata, "", tag, >>>>>>>> AV_DICT_IGNORE_SUFFIX)) >>>>>>>> + write_user_metadata_tag(s, tag, pb, &metadata_count); >>>>>>>> + while (tag = av_dict_get(flv->meta_dict, "", tag, >>>>>>>> AV_DICT_IGNORE_SUFFIX)) >>>>>>>> + write_user_metadata_tag(s, tag, pb, &metadata_count); >>>>>>>> >>>>>>>> if (write_duration_filesize) { >>>>>>>> put_amf_string(pb, "filesize"); >>>>>>>> @@ -718,6 +783,7 @@ static int flv_write_header(AVFormatContext *s) >>>>>>>> if (flv->flags & FLV_NO_METADATA) { >>>>>>>> pb->seekable = 0; >>>>>>>> } else { >>>>>>>> + read_metadata_from_file(s, 0, 1); >>>>>>>> write_metadata(s, 0); >>>>>>>> } >>>>>>>> >>>>>>>> @@ -880,6 +946,7 @@ static int flv_write_packet(AVFormatContext *s, >>>>>>>> AVPacket *pkt) >>>>>>>> if (meta_upd_flag || >>>>>>>> (flv->meta_period == FLV_META_AT_KF && >>>>>>>> par->codec_type >>>>>>>> == AVMEDIA_TYPE_VIDEO && (pkt->flags & AV_PKT_FLAG_KEY)) || >>>>>>>> (flv->meta_period == FLV_META_EVERY_PACKET && >>>>>>>> par->codec_type >>>>>>>> == AVMEDIA_TYPE_VIDEO )) { >>>>>>>> + read_metadata_from_file(s, ts, 0); >>>>>>>> write_metadata(s, ts); >>>>>>>> if (meta_upd_flag) >>>>>>>> s->event_flags &= >>>>>>>> ~AVSTREAM_EVENT_FLAG_METADATA_UPDATED; >>>>>>>> @@ -1053,6 +1120,11 @@ static void flv_deinit(AVFormatContext *s) >>>>>>>> } >>>>>>>> flv->filepositions = flv->head_filepositions = NULL; >>>>>>>> flv->filepositions_count = 0; >>>>>>>> + >>>>>>>> + if (flv->meta_dict) >>>>>>>> + av_dict_free(&flv->meta_dict); >>>>>>>> + >>>>>>>> + avformat_close_input(&flv->meta_ctx); >>>>>>>> } >>>>>>>> >>>>>>>> static const AVOption options[] = { >>>>>>>> @@ -1066,6 +1138,7 @@ static const AVOption options[] = { >>>>>>>> { "at_start", "only once at start", 0, >>>>>>>> AV_OPT_TYPE_CONST, {.i64 = FLV_META_ONCE_AT_START}, INT_MIN, >>>>>>>> INT_MAX, >>>>>>>> AV_OPT_FLAG_ENCODING_PARAM, "meta_period" }, >>>>>>>> { "at_keyframes", "with every video keyframe", 0, >>>>>>>> AV_OPT_TYPE_CONST, {.i64 = FLV_META_AT_KF}, INT_MIN, INT_MAX, >>>>>>>> AV_OPT_FLAG_ENCODING_PARAM, "meta_period" }, >>>>>>>> { "every_packet", "with every video packet", 0, >>>>>>>> AV_OPT_TYPE_CONST, {.i64 = FLV_META_EVERY_PACKET}, INT_MIN, >>>>>>>> INT_MAX, >>>>>>>> AV_OPT_FLAG_ENCODING_PARAM, "meta_period" }, >>>>>>>> + { "meta_filename", "ffmetadata file to import metadata", >>>>>>>> offsetof(FLVContext, meta_filename), AV_OPT_TYPE_STRING, {.str = >>>>>>>> NULL}, >>>>>>>> 0, 0, AV_OPT_FLAG_ENCODING_PARAM }, >>>>>>>> { NULL }, >>>>>>>> }; >>>>>>>> >>>>>>> What does this achieve that can't be achieved by other means >>>>>>> (namely by >>>>>>> an API-user updating the relevant metadata dictionaries)? >>>>>> This option together with meta_period is for CLI users. Similar to >>>>>> drawtext's textfile + reload. >>>>>> It's been used in production for over a year at a client. Think it >>>>>> will >>>>>> be broadly useful. >>>>> Not everything that is useful now is correct on long term. >>>>> >>>>> Muxer specific metadata handling by reading from files is big hack. >>>> What would you suggest? >>> There should be way to handle this generically see Anton's patch that >>> adds new AVOption type. >> Link? > > So, I only found his patch to load filter options from files, but that's > only inside filtergraphs. > > Anyway, this option is for dynamic metadata. Which means that even > meta_period would have to be a generic lavf option. > But most muxers don't emit metadata post-header. In fact, flvenc is the > only muxer that makes use of > AV***_EVENT_FLAG_METADATA_UPDATED. So, we will need a new AVFMT flag to > avoid reloading of file metadata in all other cases. > Seems overkill for something useful in one muxer. > Please ask Anton for opinion about this one. > Regards, > Gyan > > _______________________________________________ > 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".