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 75146449B4 for ; Sat, 4 Feb 2023 10:33:02 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 3BF4268B88C; Sat, 4 Feb 2023 12:32:59 +0200 (EET) Received: from mail-vs1-f50.google.com (mail-vs1-f50.google.com [209.85.217.50]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 959CB68B526 for ; Sat, 4 Feb 2023 12:32:52 +0200 (EET) Received: by mail-vs1-f50.google.com with SMTP id k4so7913656vsc.4 for ; Sat, 04 Feb 2023 02:32:52 -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=74jlZkBqtuu1jhK1XQ+gLHwxOuoSr10pC1f3TmPyNPQ=; b=LXbjz5LKKJXDg9YTxaaC5NmsqKjDIyx1xZa4G3aQrj9D9UQZDHT/B7bxVZ0tPusCBy 5ypDJpyIXjOLLPa6cKxV9EpZzzqMOMn+KL6EigGYVQ/bc86z/12O2b4wqGyE0T9maqwZ 2ZCMlrDPIxAo6KdPNE5aLHbVsGo+p5me7+37J2AMztSSFtRL2+D9cl/EqwLfpHitf0th ceObEaGurw530dYG3iQ9VV5fbbA5ceJNqfa63FQNiAMTayiWjdMmdM7qa22R89K7B4nn JXgg6SG1mWxk/4Rs1fu6fe776xiTt5tKLPizSL5NmHEKYmUZz2tCkpRqOLxTEsXalA8L kUXQ== 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=74jlZkBqtuu1jhK1XQ+gLHwxOuoSr10pC1f3TmPyNPQ=; b=W1j29HNeDZkEj2WHIDYKKu4yN5pzL14tgarA3t9n3IdvB5ZcuU7Y44t61kbHqiVxyZ dXreXnpO0Cw/J72Mg9Vf9XvKFkw+TmLnLNREEsXPiNOVl9BbdtQVdDObrXNq4yuSn2nw To1exnaRimgTIwsvS+R7LX0UGg1/T0RRTr0u4+4YEKZe+Xn5YagmmIKaPn8oSTjCYStV Zf6WzoG2LrgjdNWnMrOuUY0/jI4WOWmXhsZpSR2k/38WuU4CBM9LpJzJHDO9OCdAm/EG sFFVBo4NTPqmcKtzt5OcMQwtMyO6z1IPRDexAiJyqHUye/hdnyDjcbcE08Z36sWn1JYi iu1g== X-Gm-Message-State: AO0yUKWKRxEAScUNl1V2DcjfpXwgjaHjTUIYmiU2nD7shOBwXLYcGTRP VEFvVXkBZQipGWThsiYYx/LtESQgfLnw2VYh7EbSgNak X-Google-Smtp-Source: AK7set8VQ6VqaqECx6OJ6xngJd127VXu757wjQyk+WnOvgkM7X7dQ3VjVXyP42I1pM7lmBaRbIDu+wMoqCqf+tVBUCs= X-Received: by 2002:a67:f984:0:b0:3f1:32b7:7185 with SMTP id b4-20020a67f984000000b003f132b77185mr2183186vsq.61.1675506770992; Sat, 04 Feb 2023 02:32:50 -0800 (PST) MIME-Version: 1.0 Received: by 2002:a05:612c:2428:b0:32a:5eaa:92e4 with HTTP; Sat, 4 Feb 2023 02:32:49 -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: Sat, 4 Feb 2023 11:32:49 +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/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. Also those strcmp() spaghetti lines are unacceptable. _______________________________________________ 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".