From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.ffmpeg.org (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTPS id 277EB48276 for ; Thu, 19 Jun 2025 00:17:40 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTP id 3264E68D639; Thu, 19 Jun 2025 03:17:35 +0300 (EEST) Received: from relay15.mail.gandi.net (relay15.mail.gandi.net [217.70.178.235]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTPS id 31D6668C88D for ; Thu, 19 Jun 2025 03:17:28 +0300 (EEST) Received: by mail.gandi.net (Postfix) with ESMTPSA id 890944427B for ; Thu, 19 Jun 2025 00:17:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc; s=gm1; t=1750292246; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=VmbKjxgC33KUfLlSVZdaDOE8zXFrZGagbtf9ASSuZso=; b=fqemgITZsankP8pn1+G0KbACgStsF7IVejhVDbamHYEMlcFvA8xsTHzTGguuMQprWV2UbG R1uKkDTngT05FA5KJ4a0vw+GJTwmTrEXUNbn3VE0v0ytxvu0e/9vq3gMTdRsmwzzl+2+FU gx5KF+8K02hnvdlMGA0fXr7fwqoIYy4XEbuxXUlkjAZoHhvgwW5Jz+IIU3Tsq0W0OozQn7 kmf5cEFqZnGkprIvXiHrMTQ4wimBmGqrme0zdjOuBhKFGyRinA3aKp792/KrvCUQ3PAajy bJoW6BLAoPDxq9fjE6+cZjaRnewNHh4RzhOWORdLQN6pQsZjSvkLgYwaASz2/g== Date: Thu, 19 Jun 2025 02:17:25 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20250619001725.GQ29660@pb2> References: <2dc54cad-932f-4c30-9f9d-0a943e0a7be3@regaud-chapuy.fr> <639a031a-06c7-4855-8d97-de61626ba194@regaud-chapuy.fr> <20250617235625.GL29660@pb2> MIME-Version: 1.0 In-Reply-To: X-GND-State: clean X-GND-Score: -85 X-GND-Cause: gggruggvucftvghtrhhoucdtuddrgeeffedrtddvgdegtdeiucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuifetpfffkfdpucggtfgfnhhsuhgsshgtrhhisggvnecuuegrihhlohhuthemuceftddunecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenfghrlhcuvffnffculdduhedmnecujfgurhepfffhvffukfhfgggtuggjsehgtderredttddunecuhfhrohhmpefoihgthhgrvghlucfpihgvuggvrhhmrgihvghruceomhhitghhrggvlhesnhhivgguvghrmhgrhigvrhdrtggtqeenucggtffrrghtthgvrhhnpeehkefhleeigfevleeijedvieelfeehuefghfejtdfggeeigeekuefhleelfffgieenucffohhmrghinhepvdeigedrihhtnecukfhppeeguddrieeirdeijedruddufeenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepihhnvghtpeeguddrieeirdeijedruddufedphhgvlhhopehlohgtrghlhhhoshhtpdhmrghilhhfrhhomhepmhhitghhrggvlhesnhhivgguvghrmhgrhigvrhdrtggtpdhnsggprhgtphhtthhopedupdhrtghpthhtohepfhhfmhhpvghgqdguvghvvghlsehffhhmphgvghdrohhrgh Subject: Re: [FFmpeg-devel] [PATCH v3] avcodec/h264: fixed qp table attach for h264 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="===============5174902049037284062==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============5174902049037284062== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="e3f9oIT5Uykbfm+Q" Content-Disposition: inline --e3f9oIT5Uykbfm+Q Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jun 18, 2025 at 01:04:04PM +0200, Timoth=E9e wrote: > On 18/06/2025 at 01:56, Michael Niedermayer wrote : > > On Tue, Jun 17, 2025 at 09:29:01AM +0200, Timothee wrote: > > > Context from the first version : > > >=20 > > > > Here is a patch where I fixed the attach of per-macroblock qp table= s for > > > > H.264. It was implemented for MPEG2 so I have only extended it. > > > >=20 > > > > I tested the functionality with the codecview filter using the foll= owing > > > > command: `./ffmpeg -export_side_data 4 -i input.mp4 -vf codecview= =3Dqp=3D1 > > > > output.mp4` > > > Andreas : > > > > 1. Commits should be small atomic units; changes to different libra= ries > > > > in the same commit are almost always not of this type. > > > > 2. Both ff_h264_decode_mb_cabac() and ff_h264_decode_mb_cavlc() alr= eady > > > > set qscale_table on their own (on success), so that all the changes= to > > > > h264_slice.c seem completely redundant. > > > >=20 > > > > - Andreas > > > Here is a new version of the patch without the redundant lines. > > >=20 > > > Thanks, > > >=20 > > > Timoth=E9e > > > qp_table.c | 3 ++- > > > qp_table.h | 1 + > > > 2 files changed, 3 insertions(+), 1 deletion(-) > > > f5478a074261026e13cd6ec745b80aee4a0720b5 0001-avcodec-h264-fixed-qp-= table-attach-for-h264.patch > > > From 422e8dbdc3d79b24c6ccb11b7f384fc08406ee74 Mon Sep 17 00:00:00 20= 01 > > > From: Timothee > > > Date: Fri, 13 Jun 2025 14:21:28 +0200 > > > Subject: [PATCH] avcodec/h264: fixed qp table attach for h264 > > >=20 > > > Signed-off-by: Timothee > > > --- > > > libavcodec/h264_slice.c | 16 ++++++++++++---- > > > libavfilter/qp_table.c | 3 ++- > > > libavfilter/qp_table.h | 1 + > > > 3 files changed, 15 insertions(+), 5 deletions(-) > > >=20 > > > diff --git a/libavfilter/qp_table.c b/libavfilter/qp_table.c > > > index 8137dc019f..a99b99e77a 100644 > > > --- a/libavfilter/qp_table.c > > > +++ b/libavfilter/qp_table.c > > > @@ -40,7 +40,8 @@ int ff_qp_table_extract(AVFrame *frame, int8_t **ta= ble, int *table_w, int *table > > > if (!sd) > > > return 0; > > > par =3D (AVVideoEncParams*)sd->data; > > > - if (par->type !=3D AV_VIDEO_ENC_PARAMS_MPEG2 || > > > + if ((par->type !=3D AV_VIDEO_ENC_PARAMS_MPEG2 > > > + && par->type !=3D AV_VIDEO_ENC_PARAMS_H264) || > > > (par->nb_blocks !=3D 0 && par->nb_blocks !=3D nb_mb)) > > > return AVERROR(ENOSYS); > > The commit message should be a bit more verbose >=20 > How about this for the commit message? >=20 > ``` > [PATCH] avfilter/codecview: Enable QP visualization for H.264 >=20 > The codecviewfilter, when used with qp=3D1, did not display quantization > parameter values for H.264 streams because the QP table extraction was > restricted to MPEG-2 video. > > This patch enables H.264 support by updating ff_qp_table_extractto accept > AV_VIDEO_ENC_PARAMS_H264. An explicit case is also added to ff_norm_qscal= eto > handle H.264 qscale values directly, clarifying intent. This allows for > correct QP overlay on H.264 video >=20 > ``` >=20 > > why these are unequal, and why teh later copy of nb_mb is correct > >=20 > > If its not always correct, that should at least be documented >=20 > My understanding is that this is a sanity check: the nb_mbcalculated from > the frame's geometry is considered the ground truth. The check ensures th= at > if the encoder provides its own macroblock count in the side data, it must > match. That validation was already in place for MPEG2, so I extended the > same logic to H.264. yes, i misread the code, your change to this hunk is ok >=20 > > > diff --git a/libavfilter/qp_table.h b/libavfilter/qp_table.h > > > index 4407bacb0e..c1a80d1830 100644 > > > --- a/libavfilter/qp_table.h > > > +++ b/libavfilter/qp_table.h > > > @@ -40,6 +40,7 @@ static inline int ff_norm_qscale(int qscale, enum A= VVideoEncParamsType type) > > > { > > > switch (type) { > > > case AV_VIDEO_ENC_PARAMS_MPEG2: return qscale >> 1; > > > + case AV_VIDEO_ENC_PARAMS_H264: return qscale; > > > } > > > return qscale; > > This does nothing, it returns qscale already >=20 > You're correct, but I added it to make the handling of H.264 explicit. Th= is > improves clarity and protects it from any future changes to the default > case. That said, if you still feel it's unnecessary, I'm happy to remove = it. please remove it, mpeg1, mmspeg4, h263 also arent listed thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I know you won't believe me, but the highest form of Human Excellence is to question oneself and others. -- Socrates --e3f9oIT5Uykbfm+Q Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEKAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCaFNXEQAKCRBhHseHBAsP q+oUAJwLopZWYjPjFONhTW95BUx69m1AxgCgkQJG7TZXUDFPI+aOrXFxNYy/Gno= =cOZ9 -----END PGP SIGNATURE----- --e3f9oIT5Uykbfm+Q-- --===============5174902049037284062== 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". --===============5174902049037284062==--