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 686A941283 for ; Tue, 11 Oct 2022 09:24:44 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 8EF2D68BC3E; Tue, 11 Oct 2022 12:24:41 +0300 (EEST) Received: from mail0.khirnov.net (red.khirnov.net [176.97.15.12]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 29D0168BA39 for ; Tue, 11 Oct 2022 12:24:35 +0300 (EEST) Received: from localhost (localhost [IPv6:::1]) by mail0.khirnov.net (Postfix) with ESMTP id 4123A2404E4 for ; Tue, 11 Oct 2022 11:24:34 +0200 (CEST) Received: from mail0.khirnov.net ([IPv6:::1]) by localhost (mail0.khirnov.net [IPv6:::1]) (amavisd-new, port 10024) with ESMTP id 7utNgrE6lQ1R for ; Tue, 11 Oct 2022 11:24:33 +0200 (CEST) Received: from lain.khirnov.net (lain.khirnov.net [IPv6:2001:67c:1138:4306::3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "lain.khirnov.net", Issuer "smtp.khirnov.net SMTP CA" (verified OK)) by mail0.khirnov.net (Postfix) with ESMTPS id 9EB2E2400F4 for ; Tue, 11 Oct 2022 11:24:33 +0200 (CEST) Received: by lain.khirnov.net (Postfix, from userid 1000) id A0B9D1601B2; Tue, 11 Oct 2022 11:24:33 +0200 (CEST) From: Anton Khirnov To: FFmpeg development discussions and patches In-Reply-To: References: <20220928093213.947-1-anton@khirnov.net> <5f92999c-2097-5016-daff-7ecddbfac1f3@passwd.hu> <166487620906.5794.5456148015950663376@lain.khirnov.net> Mail-Followup-To: FFmpeg development discussions and patches Date: Tue, 11 Oct 2022 11:24:33 +0200 Message-ID: <166548027362.5794.9052977797531434972@lain.khirnov.net> User-Agent: alot/0.8.1 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-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: Quoting Marton Balint (2022-10-05 20:54:46) > > > On Tue, 4 Oct 2022, Anton Khirnov wrote: > > > Quoting Marton Balint (2022-09-28 21:54:11) > >> > >> > >> 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. > > > > Why would there be anything in that field? No code we have currently > > sets that field or does anything with it. > > It is a public field which was explicitly documented to be ignored by > filters or encoders. The user could store any data in it, because the > documentation of the field ensured it will not be a problem. > > If you read back the old threads which added AVFrame->time_base > you will find the reasoning behind the original comments, in fact, > you suggested the actual wording for the documentation of the field, and > now you want now to change the semantics of the field which contradicts > the existing documentation... Usually we introduce a new field and > deprecate the old if we want to do something like this. Honestly I do not remember why I wrote it that way and it now seems like a mistake to me. We did not add this field as random-number storage for our callers, we added it to use it in the libraries. > One could argue that this break is "small" enough, to not dance around it, > but I don't really see the benefit of the change in the first place. So > the real question is why do you want to start using AVFrame->time_base in > encoders, and what is the feature which is undoable with the current > AVCodecContext->time_base? Where I'm going with this is duration handling in encoders. libavcodec currently disregards AVFrame.duration completely, while properly it should set output AVPacket.duration from its corresponding AVFrame.duration. The problem with that is that we cannot just use AVFrame.duration, because there is no guarantee that the caller is aware of it and set in the correct timebase. E.g. ffmpeg.c currently does not touch AVFrame.duration, so whatever value it acquired along the way (from the decoder or some filter) will linger there in whatever timebase it was originally in, which is not necessarily equal to the encoder timebase. So my idea was to use the timebase being set as an indicator of the caller being duration-aware. But on more thought, this seems rather obscure and an explicit option is probably better. -- Anton Khirnov _______________________________________________ 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".