From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <ffmpeg-devel-bounces@ffmpeg.org>
Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100])
by master.gitmailbox.com (Postfix) with ESMTP id C20B245C9E
for <ffmpegdev@gitmailbox.com>; Sun, 2 Apr 2023 20:07:52 +0000 (UTC)
Received: from [127.0.1.1] (localhost [127.0.0.1])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 3D1ED68BEA6;
Sun, 2 Apr 2023 23:07:50 +0300 (EEST)
Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net
[217.70.183.196])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 9A53168BEA0
for <ffmpeg-devel@ffmpeg.org>; Sun, 2 Apr 2023 23:07:43 +0300 (EEST)
Received: (Authenticated sender: michael@niedermayer.cc)
by mail.gandi.net (Postfix) with ESMTPSA id A830FE0004
for <ffmpeg-devel@ffmpeg.org>; Sun, 2 Apr 2023 20:07:42 +0000 (UTC)
Date: Sun, 2 Apr 2023 22:07:40 +0200
From: Michael Niedermayer <michael@niedermayer.cc>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Message-ID: <20230402200740.GK1164690@pb2>
References: <a8992de53a0b8d6144a443b26242f760ff4e4526.camel@haerdin.se>
<068476d4-1430-27bb-83c4-3a160d022596@mediaarea.net>
<c8e361229d926590436681c72cd10d18247fe113.camel@haerdin.se>
<F76F7FE1-61A9-4A17-B0F7-D885F31FEEE5@dericed.com>
<09775b73ab48e425f3b557aaa8d5847643ebe9b7.camel@haerdin.se>
<0b6c3856-082c-cb33-e07c-3adf0502b93c@mediaarea.net>
<20230401143739.GD1164690@pb2>
<85c1e58e-cc90-5814-cba5-d91201dec993@mediaarea.net>
<20230401154301.GF1164690@pb2>
<71eaedd7-d771-50a7-ed13-3733b8c2187f@mediaarea.net>
MIME-Version: 1.0
In-Reply-To: <71eaedd7-d771-50a7-ed13-3733b8c2187f@mediaarea.net>
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 <ffmpeg-devel.ffmpeg.org>
List-Unsubscribe: <https://ffmpeg.org/mailman/options/ffmpeg-devel>,
<mailto:ffmpeg-devel-request@ffmpeg.org?subject=unsubscribe>
List-Archive: <https://ffmpeg.org/pipermail/ffmpeg-devel>
List-Post: <mailto:ffmpeg-devel@ffmpeg.org>
List-Help: <mailto:ffmpeg-devel-request@ffmpeg.org?subject=help>
List-Subscribe: <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>,
<mailto:ffmpeg-devel-request@ffmpeg.org?subject=subscribe>
Reply-To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Content-Type: multipart/mixed; boundary="===============3871747371580138744=="
Errors-To: ffmpeg-devel-bounces@ffmpeg.org
Sender: "ffmpeg-devel" <ffmpeg-devel-bounces@ffmpeg.org>
Archived-At: <https://master.gitmailbox.com/ffmpegdev/20230402200740.GK1164690@pb2/>
List-Archive: <https://master.gitmailbox.com/ffmpegdev/>
List-Post: <mailto:ffmpegdev@gitmailbox.com>
--===============3871747371580138744==
Content-Type: multipart/signed; micalg=pgp-sha256;
protocol="application/pgp-signature"; boundary="cEobB2knsyc5ebfU"
Content-Disposition: inline
--cEobB2knsyc5ebfU
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Sat, Apr 01, 2023 at 07:11:45PM +0200, Jerome Martinez wrote:
> On 01/04/2023 17:43, Michael Niedermayer wrote:
> > On Sat, Apr 01, 2023 at 05:20:50PM +0200, Jerome Martinez wrote:
> > > On 01/04/2023 16:37, Michael Niedermayer wrote:
> > > > On Tue, Mar 14, 2023 at 10:52:15AM +0100, Jerome Martinez wrote:
> > > > [...]
> > > > > +
> > > > > + memset(state, 128, sizeof(state));
> > > > > + if (st->codecpar->extradata) {
> > > > > + ff_init_range_decoder(&c, st->codecpar->extradata, st->c=
odecpar->extradata_size);
> > > > > + ff_build_rac_states(&c, 0.05 * (1LL << 32), 256 - 8);
> > > > > + v =3D get_ffv1_unsigned_symbol(&c, state);
> > > > > + av_assert0(v >=3D 2);
> > > > > + if (v > 4) {
> > > > > + return 0;
> > > > > + }
> > > > > + sc->micro_version =3D get_ffv1_unsigned_symbol(&c, state=
);
> > > > > + } else {
> > > > > + uint8_t keystate =3D 128;
> > > > > + ff_init_range_decoder(&c, pkt->data, pkt->size);
> > > > > + ff_build_rac_states(&c, 0.05 * (1LL << 32), 256 - 8);
> > > > > + get_rac(&c, &keystate); // keyframe
> > > > > + v =3D get_ffv1_unsigned_symbol(&c, state);
> > > > > + av_assert0(v < 2);
> > > > Are these asserts testing muxer input ?
> > > > if so what ensures that the values are within the asserted range ?
> > >=20
> > > My understanding of the code and workflow is that the input is curren=
tly
> > > rejected (AV_LOG_ERROR, "invalid version %d in ver01 header\n" and
> > > AV_LOG_ERROR, "Invalid version in global header\n") in ffv1dec.c duri=
ng the
> > > analysis of this input so before this piece of code is reached.
> > > Could be an AV_LOG_ERROR if preferred.
> > if the encoder writes 5 teh muxer crashes here
> >=20
> > V 5me=3D 0 fps=3D0.0 q=3D-0.0 size=3D 0kB time=3D00:00:00.00 b=
itrate=3DN/A speed=3D 0x
> > Assertion v < 2 failed at libavformat/mxfenc.c:2505
>=20
>=20
> I hardcoded version 5 in the encoder based on version 4 (so with extra
> metadata), and I have:
> [mxf @ 0x7fffd52f3100] could not get ffv1 version
>=20
> My understanding is that you hardcoded version 5 in the encoder based on
> version 0 or 1 (so without extra metadata), in that case I have the asser=
t,
> and it was on purpose on my side because I understand from ffv1 encoder c=
ode
> that any version > 2 should have extra metadata, and if it changes in the
> future this line in MXF muxer should be changed at the same time as FFV1
> encoder code
>=20
> But I didn't test and didn't realize that the FFV1 parsing is not done wi=
th
> -c:v copy, and if that case I get the assert with an (fake) input with
> version 5 and no extra metadata.
>=20
> What about the attached patch? I reused the ffv1 decoder error messages +
> "ffv1 " tip.
> Note: I moved, so less coherency in the code, the error message in
> mxf_parse_ffv1_frame for more precise message depending on the presence or
> not of extra metadata, it could stay as is if we don't care about the
> precision.
> mxfenc.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
> 2b3a2cc619fcaf2dd1da7aa4b566b390eced62a3 0001-avformat-mxfenc-replace-ff=
v1-version-related-assert-.patch
> From 65f6fba9434fa9ab4b5d2b91c3f930c1e3cb4bef Mon Sep 17 00:00:00 2001
> From: Jerome Martinez <jerome@mediaarea.net>
> Date: Sat, 1 Apr 2023 19:04:25 +0200
> Subject: [PATCH] avformat/mxfenc: replace ffv1 version related assert by =
error
> message
>=20
> ---
> libavformat/mxfenc.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>=20
> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> index 9eba208ffb..0a1ae5353c 100644
> --- a/libavformat/mxfenc.c
> +++ b/libavformat/mxfenc.c
> @@ -2489,8 +2489,8 @@ static int mxf_parse_ffv1_frame(AVFormatContext *s,=
AVStream *st, AVPacket *pkt)
> ff_init_range_decoder(&c, st->codecpar->extradata, st->codecpar-=
>extradata_size);
> ff_build_rac_states(&c, 0.05 * (1LL << 32), 256 - 8);
> v =3D get_ffv1_unsigned_symbol(&c, state);
> - av_assert0(v >=3D 2);
> - if (v > 4) {
> + if (v < 2) {
> + av_log(s, AV_LOG_ERROR, "Invalid version in ffv1 global head=
er\n");
> return 0;
> }
should this not also fail with v > 4 as before ?
thx
[...]
--=20
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.
--cEobB2knsyc5ebfU
Content-Type: application/pgp-signature; name="signature.asc"
-----BEGIN PGP SIGNATURE-----
iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZCnghgAKCRBhHseHBAsP
q9MUAKCcLtd2UhZ4Dk6KU7j6pV6FZAvPCwCgg+eG+MKQ41812T+vC7815EYOPLk=
=V8N6
-----END PGP SIGNATURE-----
--cEobB2knsyc5ebfU--
--===============3871747371580138744==
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".
--===============3871747371580138744==--