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 9186243A0D for ; Thu, 4 Aug 2022 23:08:54 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 7613768B9F3; Fri, 5 Aug 2022 02:08:52 +0300 (EEST) Received: from mail-pg1-f174.google.com (mail-pg1-f174.google.com [209.85.215.174]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 8879368B6D6 for ; Fri, 5 Aug 2022 02:08:45 +0300 (EEST) Received: by mail-pg1-f174.google.com with SMTP id h132so1163618pgc.10 for ; Thu, 04 Aug 2022 16:08:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sandflow-com.20210112.gappssmtp.com; s=20210112; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :from:to:cc; bh=Gyp4Kght1XNBUUDQp9olVrrDa4IZ9bC4dgLhh8y94FI=; b=P4GHVdwAC0+qkVPuauRFHQ9wWlDIomqKlhxpdeqmcgknEsD4aKll+4hq4nquWAHiOQ 5rkZwMyEcjFScDDY7S7Y6DNtGjaqYLqjujgb3R919Th02cB0JjR1+FiGhwQjN2JTmJtB WJkxbInr464NFKBAgUWRc919qyGQfeAwUz7PKG2d8SpFNvCWO9OQMmMq7nqH/dxSu24Q JbBRfk/7J0JihdAFp6fHChy22mQgKBTpQ9Qoj0B11wvjvwUQf9bRjY9js9Fntmw1KQ0I MCq8Z7uKd8MRbVkTH4geY0ZEcMTLgce/yfUP6oiVBlD3Tjlv8bF4uiO2Y4hTeKTFKuus hUYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :x-gm-message-state:from:to:cc; bh=Gyp4Kght1XNBUUDQp9olVrrDa4IZ9bC4dgLhh8y94FI=; b=uWkMI3dhk/6Bs6T/oo003VWVVZNPMlAXskjgcB5H5EEDQaFjtGLkBrrnohnQkV/xwS QIHHCY4Pe7DQE5v3OuADS30LGSvv+/lQgkG2agBR2avoGxeKxU593IGLxQulfhGURLzy wHbzbh1w/VkRxyKjoEZsbum5AE3nolhCb+Sb8kAi/K5wy72uqXdjxhUtpEp+80xsv01D eRz3eKAPVXzqNNVvd9oDabiGnW2daTLYcbV6WiSxInrxDDPTjQgaRdZhRbWGrFTqgKoQ jgwEvw1cCsUImhgfxJ00sP607dniq+IUbtvxyYpA1JIWxiK0ezPiabT+SZZ046ohw/Y2 DotA== X-Gm-Message-State: ACgBeo2NhdRT0ZwIOrAP549zXllLlhaWKyLJfrlp0J24sp0lj70m3sH5 Br6Dxyt8bfZT8F0Xr2uIxK9Fsbbdb2U9wQ== X-Google-Smtp-Source: AA6agR6KzE3lQ1IuS00qlcwVoqdJeUaT+h3ndRfRCEFkLHC4XuFWOFp/HOTQJTc/RViuywvrC7VF2A== X-Received: by 2002:a63:85c8:0:b0:41b:f27f:5a7e with SMTP id u191-20020a6385c8000000b0041bf27f5a7emr3435944pgd.590.1659654522433; Thu, 04 Aug 2022 16:08:42 -0700 (PDT) Received: from mail-pj1-f49.google.com (mail-pj1-f49.google.com. [209.85.216.49]) by smtp.gmail.com with ESMTPSA id v6-20020aa799c6000000b0052db0ea0462sm1495731pfi.113.2022.08.04.16.08.41 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 04 Aug 2022 16:08:41 -0700 (PDT) Received: by mail-pj1-f49.google.com with SMTP id o5-20020a17090a3d4500b001ef76490983so1202511pjf.2 for ; Thu, 04 Aug 2022 16:08:41 -0700 (PDT) X-Received: by 2002:a17:903:2589:b0:16d:c26c:d641 with SMTP id jb9-20020a170903258900b0016dc26cd641mr3867904plb.8.1659654521017; Thu, 04 Aug 2022 16:08:41 -0700 (PDT) MIME-Version: 1.0 References: <20220731215157.22022-1-pal@sandflow.com> In-Reply-To: From: Pierre-Anthony Lemieux Date: Thu, 4 Aug 2022 16:08:28 -0700 X-Gmail-Original-Message-ID: Message-ID: To: FFmpeg development discussions and patches Subject: Re: [FFmpeg-devel] [PATCH v5 1/2] avformat: refactor ff_stream_encode_params_copy() to ff_stream_params_copy() 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 Thu, Aug 4, 2022 at 3:22 PM Andreas Rheinhardt wrote: > > Pierre-Anthony Lemieux: > > On Thu, Aug 4, 2022 at 10:15 AM Andreas Rheinhardt > > wrote: > >> > >> Pierre-Anthony Lemieux: > >>> On Thu, Aug 4, 2022 at 9:53 AM Andreas Rheinhardt > >>> wrote: > >>>> > >>>> pal@sandflow.com: > >>>>> From: Pierre-Anthony Lemieux > >>>>> > >>>>> As discussed at https://ffmpeg.org/pipermail/ffmpeg-devel/2022-July/298491.html. > >>>>> Note that ff_stream_params_copy() does not copy: > >>>>> * the index field > >>>>> * the attached_pic if its size is 0 > >>>>> > >>>>> Addresses http://ffmpeg.org/pipermail/ffmpeg-devel/2022-August/299514.html > >>>>> > >>>>> --- > >>>>> libavformat/avformat.c | 40 ++++++++++++++++++++++++++++++++++++++++ > >>>>> libavformat/fifo.c | 2 +- > >>>>> libavformat/internal.h | 12 ++++++++++++ > >>>>> libavformat/mux.h | 9 --------- > >>>>> libavformat/mux_utils.c | 28 ---------------------------- > >>>>> libavformat/segment.c | 2 +- > >>>>> libavformat/tee.c | 2 +- > >>>>> libavformat/webm_chunk.c | 2 +- > >>>>> 8 files changed, 56 insertions(+), 41 deletions(-) > >>>>> > >>>>> diff --git a/libavformat/avformat.c b/libavformat/avformat.c > >>>>> index 30d6ea6a49..242187c359 100644 > >>>>> --- a/libavformat/avformat.c > >>>>> +++ b/libavformat/avformat.c > >>>>> @@ -235,6 +235,46 @@ int ff_stream_side_data_copy(AVStream *dst, const AVStream *src) > >>>>> return 0; > >>>>> } > >>>>> > >>>>> +int ff_stream_params_copy(AVStream *dst, const AVStream *src) > >>>>> +{ > >>>>> + int ret; > >>>>> + > >>>>> + dst->id = src->id; > >>>>> + dst->time_base = src->time_base; > >>>>> + dst->start_time = src->start_time; > >>>>> + dst->duration = src->duration; > >>>>> + dst->nb_frames = src->nb_frames; > >>>>> + dst->disposition = src->disposition; > >>>>> + dst->discard = src->discard; > >>>>> + dst->sample_aspect_ratio = src->sample_aspect_ratio; > >>>>> + dst->avg_frame_rate = src->avg_frame_rate; > >>>>> + dst->event_flags = src->event_flags; > >>>>> + dst->r_frame_rate = src->r_frame_rate; > >>>>> + dst->pts_wrap_bits = src->pts_wrap_bits; > >>>>> + > >>>>> + av_dict_free(&dst->metadata); > >>>>> + ret = av_dict_copy(&dst->metadata, src->metadata, 0); > >>>>> + if (ret < 0) > >>>>> + return ret; > >>>>> + > >>>>> + ret = avcodec_parameters_copy(dst->codecpar, src->codecpar); > >>>>> + if (ret < 0) > >>>>> + return ret; > >>>>> + > >>>>> + ret = ff_stream_side_data_copy(dst, src); > >>>>> + if (ret < 0) > >>>>> + return ret; > >>>>> + > >>>>> + av_packet_unref(&dst->attached_pic); > >>>>> + if (src->attached_pic.size > 0) { > >>>>> + ret = av_packet_ref(&dst->attached_pic, &src->attached_pic); > >>>>> + if (ret < 0) > >>>>> + return ret; > >>>>> + } > >>>>> + > >>>>> + return 0; > >>>>> +} > >>>>> + > >>>>> AVProgram *av_new_program(AVFormatContext *ac, int id) > >>>>> { > >>>>> AVProgram *program = NULL; > >>>>> diff --git a/libavformat/fifo.c b/libavformat/fifo.c > >>>>> index ead2bdc5cf..fef116d040 100644 > >>>>> --- a/libavformat/fifo.c > >>>>> +++ b/libavformat/fifo.c > >>>>> @@ -509,7 +509,7 @@ static int fifo_mux_init(AVFormatContext *avf, const AVOutputFormat *oformat, > >>>>> if (!st) > >>>>> return AVERROR(ENOMEM); > >>>>> > >>>>> - ret = ff_stream_encode_params_copy(st, avf->streams[i]); > >>>>> + ret = ff_stream_params_copy(st, avf->streams[i]); > >>>>> if (ret < 0) > >>>>> return ret; > >>>>> } > >>>>> diff --git a/libavformat/internal.h b/libavformat/internal.h > >>>>> index b6b8fbf56f..87a00bbae3 100644 > >>>>> --- a/libavformat/internal.h > >>>>> +++ b/libavformat/internal.h > >>>>> @@ -625,6 +625,18 @@ enum AVCodecID ff_get_pcm_codec_id(int bps, int flt, int be, int sflags); > >>>>> */ > >>>>> int ff_stream_side_data_copy(AVStream *dst, const AVStream *src); > >>>>> > >>>>> +/** > >>>>> + * Copy all stream parameters from source to destination stream, with the > >>>>> + * exception of: > >>>>> + * * the index field, which is usually set by avformat_new_stream() > >>>>> + * * the attached_pic field, if attached_pic.size is 0 or less > >>>>> + * > >>>>> + * @param dst pointer to destination AVStream > >>>>> + * @param src pointer to source AVStream > >>>>> + * @return >=0 on success, AVERROR code on error > >>>>> + */ > >>>>> +int ff_stream_params_copy(AVStream *dst, const AVStream *src); > >>>>> + > >>>>> /** > >>>>> * Wrap ffurl_move() and log if error happens. > >>>>> * > >>>>> diff --git a/libavformat/mux.h b/libavformat/mux.h > >>>>> index c01da82194..1bfcaf795f 100644 > >>>>> --- a/libavformat/mux.h > >>>>> +++ b/libavformat/mux.h > >>>>> @@ -113,15 +113,6 @@ int ff_format_shift_data(AVFormatContext *s, int64_t read_start, int shift_size) > >>>>> */ > >>>>> int ff_format_output_open(AVFormatContext *s, const char *url, AVDictionary **options); > >>>>> > >>>>> -/** > >>>>> - * Copy encoding parameters from source to destination stream > >>>>> - * > >>>>> - * @param dst pointer to destination AVStream > >>>>> - * @param src pointer to source AVStream > >>>>> - * @return >=0 on success, AVERROR code on error > >>>>> - */ > >>>>> -int ff_stream_encode_params_copy(AVStream *dst, const AVStream *src); > >>>>> - > >>>>> /** > >>>>> * Parse creation_time in AVFormatContext metadata if exists and warn if the > >>>>> * parsing fails. > >>>>> diff --git a/libavformat/mux_utils.c b/libavformat/mux_utils.c > >>>>> index eb8ea3d560..2fa2ab5b0f 100644 > >>>>> --- a/libavformat/mux_utils.c > >>>>> +++ b/libavformat/mux_utils.c > >>>>> @@ -121,34 +121,6 @@ int ff_format_output_open(AVFormatContext *s, const char *url, AVDictionary **op > >>>>> return 0; > >>>>> } > >>>>> > >>>>> -int ff_stream_encode_params_copy(AVStream *dst, const AVStream *src) > >>>>> -{ > >>>>> - int ret; > >>>>> - > >>>>> - dst->id = src->id; > >>>>> - dst->time_base = src->time_base; > >>>>> - dst->nb_frames = src->nb_frames; > >>>>> - dst->disposition = src->disposition; > >>>>> - dst->sample_aspect_ratio = src->sample_aspect_ratio; > >>>>> - dst->avg_frame_rate = src->avg_frame_rate; > >>>>> - dst->r_frame_rate = src->r_frame_rate; > >>>>> - > >>>>> - av_dict_free(&dst->metadata); > >>>>> - ret = av_dict_copy(&dst->metadata, src->metadata, 0); > >>>>> - if (ret < 0) > >>>>> - return ret; > >>>>> - > >>>>> - ret = avcodec_parameters_copy(dst->codecpar, src->codecpar); > >>>>> - if (ret < 0) > >>>>> - return ret; > >>>>> - > >>>>> - ret = ff_stream_side_data_copy(dst, src); > >>>>> - if (ret < 0) > >>>>> - return ret; > >>>>> - > >>>>> - return 0; > >>>>> -} > >>>>> - > >>>>> int ff_parse_creation_time_metadata(AVFormatContext *s, int64_t *timestamp, int return_seconds) > >>>>> { > >>>>> AVDictionaryEntry *entry; > >>>>> diff --git a/libavformat/segment.c b/libavformat/segment.c > >>>>> index fa435d9f32..a8f3e94714 100644 > >>>>> --- a/libavformat/segment.c > >>>>> +++ b/libavformat/segment.c > >>>>> @@ -169,7 +169,7 @@ static int segment_mux_init(AVFormatContext *s) > >>>>> > >>>>> if (!(st = avformat_new_stream(oc, NULL))) > >>>>> return AVERROR(ENOMEM); > >>>>> - ret = ff_stream_encode_params_copy(st, ist); > >>>>> + ret = ff_stream_params_copy(st, ist); > >>>>> if (ret < 0) > >>>>> return ret; > >>>>> opar = st->codecpar; > >>>>> diff --git a/libavformat/tee.c b/libavformat/tee.c > >>>>> index f1f2a9d2a5..dbfad604d0 100644 > >>>>> --- a/libavformat/tee.c > >>>>> +++ b/libavformat/tee.c > >>>>> @@ -289,7 +289,7 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave) > >>>>> goto end; > >>>>> } > >>>>> > >>>>> - ret = ff_stream_encode_params_copy(st2, st); > >>>>> + ret = ff_stream_params_copy(st2, st); > >>>>> if (ret < 0) > >>>>> goto end; > >>>>> } > >>>>> diff --git a/libavformat/webm_chunk.c b/libavformat/webm_chunk.c > >>>>> index d69db3a004..39f21fce7a 100644 > >>>>> --- a/libavformat/webm_chunk.c > >>>>> +++ b/libavformat/webm_chunk.c > >>>>> @@ -94,7 +94,7 @@ static int webm_chunk_init(AVFormatContext *s) > >>>>> if (!(st = avformat_new_stream(oc, NULL))) > >>>>> return AVERROR(ENOMEM); > >>>>> > >>>>> - if ((ret = ff_stream_encode_params_copy(st, ost)) < 0) > >>>>> + if ((ret = ff_stream_params_copy(st, ost)) < 0) > >>>>> return ret; > >>>>> > >>>>> if (wc->http_method) > >>>> > >>>> Looking at these callers shows that they all have one thing in common: > >>>> They create a stream and immediately afterwards copy stream parameters. > >>>> The caller that you intend to add in #2 does the same. How about we make > >>>> a function that is equivalent to > >>>> avformat_new_stream+ff_stream_params_copy above and use that? (But > >>>> please leave ff_stream_params_copy as it is in the form of a static > >>>> function.) > >>> > >>> avformat_clone_stream()? > >>> > >> > >> I was not thinking about a public function. But clone_stream sounds good. > > > > Something like this? > > > > int ff_stream_clone(AVFormatContext *s, AVStream **dst, const AVStream *src) > > { > > AVStream *st; > > int ret; > > > > st = avformat_new_stream(s, NULL); > > if (!st) > > return AVERROR(ENOMEM); > > > > ret = stream_params_copy(st, src); > > if (ret < 0) > > return ret; > > > > *dst = st; > > > > return 0; > > } > > > > I'd use AVStream *ff_stream_clone(AVFormatContext *dst_ctx, const > AVStream *src); How does one recover a pointer to the stream that was just created? Isn't there a potential race condition with dst_ctx[nb_streams - 1]? > > - Andreas > _______________________________________________ > 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".