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 A358F449E0 for ; Wed, 28 Sep 2022 19:54:24 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id EE6C568BC1B; Wed, 28 Sep 2022 22:54:21 +0300 (EEST) Received: from iq.passwd.hu (iq.passwd.hu [217.27.212.140]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id E80D468BB14 for ; Wed, 28 Sep 2022 22:54:15 +0300 (EEST) Received: from localhost (localhost [127.0.0.1]) by iq.passwd.hu (Postfix) with ESMTP id 7D449E7175 for ; Wed, 28 Sep 2022 21:54:14 +0200 (CEST) X-Virus-Scanned: amavisd-new at passwd.hu Received: from iq.passwd.hu ([127.0.0.1]) by localhost (iq.passwd.hu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id dAaiMvWKmUaS for ; Wed, 28 Sep 2022 21:54:11 +0200 (CEST) Received: from iq (iq [217.27.212.140]) by iq.passwd.hu (Postfix) with ESMTPS id 28A55E6FCE for ; Wed, 28 Sep 2022 21:54:11 +0200 (CEST) Date: Wed, 28 Sep 2022 21:54:11 +0200 (CEST) From: Marton Balint To: FFmpeg development discussions and patches In-Reply-To: <20220928093213.947-1-anton@khirnov.net> Message-ID: <5f92999c-2097-5016-daff-7ecddbfac1f3@passwd.hu> References: <20220928093213.947-1-anton@khirnov.net> MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH 1/3] lavc/encode: make sure frame timebase matches encoder, when set 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Wed, 28 Sep 2022, Anton Khirnov wrote: > AVFrame.time_base has been added recently, but is currently not used for > anything. Prepare for its use in encoders by rejecting frames where > time_base is set, but differs from the AVCodecContext one. How is that not an API break? Users can encode AVFrames with anything in the AVFrame->time_base right now, if you change that behaviour, that will surely break some code. That is why it was explicitly documented that it will be ignored by encoders by default. It is the second time people make this mistake, and it is exactly this confusion why I was hesitant to accept adding the time base to AVFrame. Regards, Marton > --- > libavcodec/avcodec.h | 4 ++++ > libavcodec/encode.c | 8 ++++++++ > libavutil/frame.h | 5 +++-- > 3 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h > index 7365eb5cc0..4030675d4f 100644 > --- a/libavcodec/avcodec.h > +++ b/libavcodec/avcodec.h > @@ -2645,6 +2645,10 @@ int avcodec_receive_frame(AVCodecContext *avctx, AVFrame *frame); > * packets are ignored, and sending frames will return > * AVERROR_EOF. > * > + * frame->time_base should be set to the same value as > + * avctx->time_base. This is not required yet, but may be in > + * the future. > + * > * For audio: > * If AV_CODEC_CAP_VARIABLE_FRAME_SIZE is set, then each frame > * can have any number of samples. > diff --git a/libavcodec/encode.c b/libavcodec/encode.c > index b275344bd1..0f78012747 100644 > --- a/libavcodec/encode.c > +++ b/libavcodec/encode.c > @@ -386,6 +386,14 @@ static int encode_send_frame_internal(AVCodecContext *avctx, const AVFrame *src) > AVFrame *dst = avci->buffer_frame; > int ret; > > + /* make sure the frame's timebase (if set) matches the encoder one */ > + if (src->time_base.num && > + (src->time_base.num != avctx->time_base.num || > + src->time_base.den != avctx->time_base.den)) { > + av_log(avctx, AV_LOG_ERROR, "Mismatching frame/encoder time base\n"); > + return AVERROR(EINVAL); > + } > + > if (avctx->codec->type == AVMEDIA_TYPE_AUDIO) { > /* extract audio service type metadata */ > AVFrameSideData *sd = av_frame_get_side_data(src, AV_FRAME_DATA_AUDIO_SERVICE_TYPE); > diff --git a/libavutil/frame.h b/libavutil/frame.h > index 6d9563bc5d..c5e2de85b3 100644 > --- a/libavutil/frame.h > +++ b/libavutil/frame.h > @@ -441,8 +441,9 @@ typedef struct AVFrame { > /** > * Time base for the timestamps in this frame. > * In the future, this field may be set on frames output by decoders or > - * filters, but its value will be by default ignored on input to encoders > - * or filters. > + * filters; its value will be by default ignored on input to filters. > + * For frames sent to encoders, it should be set by the user to the same > + * value as AVCodecContext.time_base. > */ > AVRational time_base; > > -- > 2.35.1 > > _______________________________________________ > 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".