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 A9E5940C5F for ; Wed, 5 Apr 2023 12:43:01 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id BE38868BA58; Wed, 5 Apr 2023 15:42:58 +0300 (EEST) Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id D16A1689961 for ; Wed, 5 Apr 2023 15:42:51 +0300 (EEST) Received: (Authenticated sender: michael@niedermayer.cc) by mail.gandi.net (Postfix) with ESMTPSA id F39131C0002 for ; Wed, 5 Apr 2023 12:42:50 +0000 (UTC) Date: Wed, 5 Apr 2023 14:42:49 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20230405124249.GY1164690@pb2> References: <85c1e58e-cc90-5814-cba5-d91201dec993@mediaarea.net> <20230401154301.GF1164690@pb2> <71eaedd7-d771-50a7-ed13-3733b8c2187f@mediaarea.net> <20230402200740.GK1164690@pb2> <7291a984-856c-3cd4-483f-f55f13af6be2@mediaarea.net> <20230404144355.GU1164690@pb2> <4bcc6a3e-d0c8-f7ce-0620-7e9e9e745a5f@mediaarea.net> <20230404173242.GW1164690@pb2> <05797ebe-97fe-148f-03b9-a74ccc7e5931@mediaarea.net> <20230405123116.GX1164690@pb2> MIME-Version: 1.0 In-Reply-To: <20230405123116.GX1164690@pb2> Subject: Re: [FFmpeg-devel] avformat/mxfenc: SMPTE RDD 48:2018 Amd 1:2022 (FFV1 in MXF) support 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: multipart/mixed; boundary="===============8415498244678859941==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============8415498244678859941== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="j+D14l8Ki1YJdzYp" Content-Disposition: inline --j+D14l8Ki1YJdzYp Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Apr 05, 2023 at 02:31:16PM +0200, Michael Niedermayer wrote: > On Tue, Apr 04, 2023 at 11:37:54PM +0200, Jerome Martinez wrote: > > On 04/04/2023 19:32, Michael Niedermayer wrote: > > > On Tue, Apr 04, 2023 at 04:57:03PM +0200, Jerome Martinez wrote: > > > > On 04/04/2023 16:43, Michael Niedermayer wrote: > > > > > On Mon, Apr 03, 2023 at 12:07:06AM +0200, Jerome Martinez wrote: > > > > > > On 02/04/2023 22:07, Michael Niedermayer wrote: > > > > > >=20 > > > > > > + if (f->version =3D=3D 4 && f->micro_version > 2) { > > > > > > + av_log(f->avctx, AV_LOG_ERROR, "unsupported versio= n 4 micro_version %d\n", > > > > > > + f->micro_version); > > > > > > + return AVERROR_PATCHWELCOME; > > > > > > + } > > > > > > } > > > > > > f->ac =3D get_symbol(c, state, 0); > > > > > you do not know if the decoder will have any problem with these f= iles > > > >=20 > > > > But you don't don't if the decoder will have no problem, it seems s= afer to > > > > me to reject something we are not sure to support. > > > "each new micro-version after this first stable variant is compatible= with the previous micro-version: decoders **SHOULD NOT** reject FFV1 bitst= reams due to an unknown micro-version equal or above the micro-version cons= idered as stable." > >=20 > > This sentence is about a stable version, and version 4 is not stable, so > > FFV1 version 4 micro_version 3 is not necessarily compatible with FFV1 > > version 4 micro_version 2, and so on, until we freeze the micro-version > > considered as stable. It is actually same for FFV1 version 3 e.g. "if > > (f->version =3D=3D 3 && f->micro_version > 1 || f->version > 3) get_rac= (&fs->c, > > (uint8_t[]) { 129 });" seems to say that version 3 micro_version 2 is n= ot > > compatible with version 3 micro_version 1 (and acceptable, only version= 3 > > micro_version 5 and above must be compatible with version 3 micro_versi= on 4 > > because micro_version 4 is the one flagged as stable). > > Reason I suggested this micro_version check for version 4 (I didn't add= it > > for version 3 because version 3 has a micro-version considered as stabl= e). > > Anyway, it is not important to me, just apply the patch you prefer. >=20 > Theres a practical problem with rejecting increased micro versions. > That problem is that we would not be able to introduce compatible changes > as even if they are compatible the micro_version itself would break all > decoders >=20 >=20 > >=20 > >=20 > > >=20 > > >=20 > > > [...] > > > > libavcodec/ffv1dec.c | 5 +++++ > > > > libavformat/mxfenc.c | 4 ++++ > > > > 2 files changed, 9 insertions(+) > > > > 9b094eb0bd0888725a4a3fac925ef1fa733a48c3 0001-avcodec-ffv1dec-reje= ct-unsupported-ffv1-versions.patch > > > > From dc0382709e548ef2514198bc866028066134d33e Mon Sep 17 00:00:00 = 2001 > > > > From: Jerome Martinez > > > > Date: Mon, 3 Apr 2023 00:04:53 +0200 > > > > Subject: [PATCH] avcodec/ffv1dec: reject unsupported ffv1 versions > > > >=20 > > > > And add similar check in libavformat/mxfenc > > > > --- > > > > libavcodec/ffv1dec.c | 5 +++++ > > > > libavformat/mxfenc.c | 4 ++++ > > > > 2 files changed, 9 insertions(+) > > > the patch is mostly ok > > > iam a bit undecided if a decoder change and a muxer bugfix belong in = the > > > same patch, so do as you prefer > >=20 > > I think that both parts are about the same thing (and in the future both > > should be changed at the same time) but it is really subjective, let me= know > > if you prefer that I send 2 separated patches and I'll do it, else plea= se > > apply one of the already proposed patch. >=20 > The problem is "same" "time" > In the world of libavcodec, "time" is specified in libavcodec/version.h > In the world of libavformat, "time" is specified in libavformat/version.h >=20 > Now if neither version is increased then you certainly can not assume > anything and any mixture can occurr. You can have before and after > libs mixed in any way on a system. For many changes this is ok > but sometimes the libavformat change needs the libavcodec feature. > So doing it in the same commit can be done here but it cannot be the > default (future) way to do it >=20 > If you bump the minor versions of both libs with a change then you can ha= ve > libavcodec libavformat > before before > after before > after after >=20 > If you bump the major versions of both libs with a change then you can ha= ve > before before > after after >=20 > the last variant is hard as major isnt bumped often. >=20 > so the way changes are introduced to 2 libs is to do it > first to libavcodec (this tests the after before case) > then subsequently to libavformat (this tests the after after case) > And with such seperate changes you just tested all legal combinations > without any need to think about it :) >=20 > OTOH if a commit does both libs together it requires an actual review > of what happens if not both libs are upgraded together on the user side. >=20 > The idea is simple, split it in 2 and things get naturally tested > and no need to spend any time on odd lib mixes as the only case is natura= lly > tested between the 2 commits. >=20 > Ill apply one of the patches as its ok here, but it might be "not ok" for > future changes to ffv1 in both libs, it depends on the exact changes Changed my mind, ill push it splited in 2. Its more consistent with how such changes have been done, even when its not needed here thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB He who knows, does not speak. He who speaks, does not know. -- Lao Tsu --j+D14l8Ki1YJdzYp Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZC1sxAAKCRBhHseHBAsP q0vlAJ0b7d7G9CUHBCFR2jxpN3MnJ7zj0wCfapzXU3XB7dy9CEsDLzQR5eRYi7Q= =s/zr -----END PGP SIGNATURE----- --j+D14l8Ki1YJdzYp-- --===============8415498244678859941== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ 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". --===============8415498244678859941==--