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 ESMTPS id 1E34A4CF57
for <ffmpegdev@gitmailbox.com>; Tue, 15 Apr 2025 18:33:09 +0000 (UTC)
Received: from [127.0.1.1] (localhost [127.0.0.1])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 1881A687D1E;
Tue, 15 Apr 2025 21:33:05 +0300 (EEST)
Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net
[217.70.183.194])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 5D095687CFB
for <ffmpeg-devel@ffmpeg.org>; Tue, 15 Apr 2025 21:32:58 +0300 (EEST)
Received: by mail.gandi.net (Postfix) with ESMTPSA id 962164315F
for <ffmpeg-devel@ffmpeg.org>; Tue, 15 Apr 2025 18:32:57 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc;
s=gm1; t=1744741977;
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=VncR1p7rinT7AYBmKmd/DE6Yejj6Tm906WpiOs2A9AM=;
b=LG1/2rnDcZQvXx88ladVuJUVaWx8msn7yTA7yUqBTik6C6hrAIKKYhRv2L0r1oIkgAHsRG
f8mIL8WWSt1Sg/QNGOaf+qU51D0KT13fq0FZls2Xx8mAtBy/2g22oAPuulFoB+AjTwitrD
XYC5aScUqiTSld+SZKoAAOhdqUXb9tXegvKgVxL7oSmqarKXfnUFRaBt+zreQ9MO0kykj9
am0R38zUTJ8rk7MTcnXDB6i5oHsr01agyCMF+vg6ghgl8eDy2BUJq9JXIuLBMdwar2WzH9
2pHZxZ1AgCzuuck/6QB2ATGBV3fLWd9V5Wn0viYsG3VEs0A/gWC9AUr17dr+tw==
Date: Tue, 15 Apr 2025 20:32:56 +0200
From: Michael Niedermayer <michael@niedermayer.cc>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Message-ID: <20250415183256.GQ4991@pb2>
References: <20250411222719.2779176-1-michael@niedermayer.cc>
<20250411222719.2779176-2-michael@niedermayer.cc>
<DM8P223MB0365A90C10FBC9EF6512F9E1BAB12@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM>
<20250414231950.GN4991@pb2>
<DM8P223MB0365928500AB53069869DC99BAB32@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM>
<3D7D11CA-0A71-41DB-BD95-3EF31E990BCF@rcombs.me>
<DM8P223MB0365E771783D7093DC2A7645BAB22@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM>
MIME-Version: 1.0
In-Reply-To: <DM8P223MB0365E771783D7093DC2A7645BAB22@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM>
X-GND-State: clean
X-GND-Score: -93
X-GND-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgddvvdegvddvucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuifetpfffkfdpucggtfgfnhhsuhgsshgtrhhisggvnecuuegrihhlohhuthemuceftddunecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenfghrlhcuvffnffculdejmdenucfjughrpeffhffvuffkfhggtggujgesghdtreertddtjeenucfhrhhomhepofhitghhrggvlhcupfhivgguvghrmhgrhigvrhcuoehmihgthhgrvghlsehnihgvuggvrhhmrgihvghrrdgttgeqnecuggftrfgrthhtvghrnhepffehvefhvddvhfdtgfethfdtueelfedtveekffeljeethfegtdfhfefggfeufedtnecuffhomhgrihhnpehffhhmphgvghdrohhrghenucfkphepgedurdeiiedrieejrdduudefnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehinhgvthepgedurdeiiedrieejrdduudefpdhhvghloheplhhotggrlhhhohhsthdpmhgrihhlfhhrohhmpehmihgthhgrvghlsehnihgvuggvrhhmrgihvghrrdgttgdpnhgspghrtghpthhtohepuddprhgtphhtthhopehffhhmphgvghdquggvvhgvlhesfhhfmhhpvghgrdhorhhg
X-GND-Sasl: michael@niedermayer.cc
Subject: Re: [FFmpeg-devel] [PATCH 2/2] avformat/id3v2: Check that
decode_str() did advance
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="===============1644852669430401197=="
Errors-To: ffmpeg-devel-bounces@ffmpeg.org
Sender: "ffmpeg-devel" <ffmpeg-devel-bounces@ffmpeg.org>
Archived-At: <https://master.gitmailbox.com/ffmpegdev/20250415183256.GQ4991@pb2/>
List-Archive: <https://master.gitmailbox.com/ffmpegdev/>
List-Post: <mailto:ffmpegdev@gitmailbox.com>
--===============1644852669430401197==
Content-Type: multipart/signed; micalg=pgp-sha512;
protocol="application/pgp-signature"; boundary="BZ8Mbs+radZV19pw"
Content-Disposition: inline
--BZ8Mbs+radZV19pw
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Tue, Apr 15, 2025 at 12:17:32AM +0000, softworkz . wrote:
>=20
>=20
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Ridley Combs via ffmpeg-devel
> > Sent: Dienstag, 15. April 2025 02:03
> > To: ffmpeg-devel <ffmpeg-devel@ffmpeg.org>
> > Cc: Ridley Combs <rcombs@rcombs.me>
> > Subject: Re: [FFmpeg-devel] [PATCH 2/2] avformat/id3v2: Check that
> > decode_str() did advance
> >=20
> >=20
> >=20
> > > On Apr 15, 2025, at 08:59, softworkz . <softworkz-at-
> > hotmail.com@ffmpeg.org> wrote:
> > >
> > >
> > >
> > >> -----Original Message-----
> > >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org <mailto:ffmpeg-
> > devel-bounces@ffmpeg.org>> On Behalf Of
> > >> Michael Niedermayer
> > >> Sent: Dienstag, 15. April 2025 01:20
> > >> To: FFmpeg development discussions and patches <ffmpeg-
> > >> devel@ffmpeg.org <mailto:devel@ffmpeg.org>>
> > >> Subject: Re: [FFmpeg-devel] [PATCH 2/2] avformat/id3v2: Check that
> > >> decode_str() did advance
> > >>
> > >> On Sat, Apr 12, 2025 at 01:49:53AM +0000, softworkz . wrote:
> > >>>
> > >>>
> > >>>> -----Original Message-----
> > >>>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > >>>> Michael Niedermayer
> > >>>> Sent: Samstag, 12. April 2025 00:27
> > >>>> To: FFmpeg development discussions and patches <ffmpeg-
> > >> devel@ffmpeg.org>
> > >>>> Subject: [FFmpeg-devel] [PATCH 2/2] avformat/id3v2: Check that
> > >>>> decode_str() did advance
> > >>>>
> > >>>> Fixes infinite loop with unknown encodings
> > >>>>
> > >>>> We could alternatively error out from decode_str() or consume all
> > >> of
> > >>>> taglen
> > >>>> this would affect other callers though.
> > >>>>
> > >>>> Fixes: 409819224/clusterfuzz-testcase-minimized-
> > >> ffmpeg_dem_H261_fuzzer-
> > >>>> 6003527535362048
> > >>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > >>>> ---
> > >>>> libavformat/id3v2.c | 3 +++
> > >>>> 1 file changed, 3 insertions(+)
> > >>>>
> > >>>> diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c
> > >>>> index 90314583a74..e3f7f9e2a90 100644
> > >>>> --- a/libavformat/id3v2.c
> > >>>> +++ b/libavformat/id3v2.c
> > >>>> @@ -341,10 +341,13 @@ static void read_ttag(AVFormatContext *s,
> > >>>> AVIOContext *pb, int taglen,
> > >>>> taglen--; /* account for encoding type byte */
> > >>>>
> > >>>> while (taglen > 1) {
> > >>>> + int current_taglen =3D taglen;
> > >>>> if (decode_str(s, pb, encoding, &dst, &taglen) < 0) {
> > >>>> av_log(s, AV_LOG_ERROR, "Error reading frame %s,
> > >>>> skipped\n", key);
> > >>>> return;
> > >>>> }
> > >>>> + if (current_taglen =3D=3D taglen)
> > >>>> + return;
> > >>>>
> > >>>> count++;
> > >>>>
> > >>>> --
> > >>>> 2.49.0
> > >>>>
> > >>>> _______________________________________________
> > >>>
> > >>> Hi Michael,
> > >>>
> > >>> this kind of conflicts with this patch that I had submitted
> > >> recently:
> > >>>
> > >>>
> > >>
> > https://patchwork.ffmpeg.org/project/ffmpeg/patch/pull.54.ffstaging.FF
> > >> mpeg.1740873449247.ffmpegagent@gmail.com/
> > >>>
> > >>>
> > >>> I wonder whether my patch would still be prone to the issue your
> > >> patch is addressing -
> > >>
> > >> This already conflicts with rcombs patch in git master, i think
> > >> Applying: Fixes Trac ticket https://trac.ffmpeg.org/ticket/6949
> > >> Using index info to reconstruct a base tree...
> > >> M libavformat/id3v2.c
> > >> Falling back to patching base and 3-way merge...
> > >> Auto-merging libavformat/id3v2.c
> > >> CONFLICT (content): Merge conflict in libavformat/id3v2.c
> > >> error: Failed to merge in the changes.
> > >> Patch failed at 0001 Fixes Trac ticket
> > >> https://trac.ffmpeg.org/ticket/6949
> > >>
> > >>
> > >>> do you have a test file perhaps?
> > >>
> > >> Will email you one, but the loop with a function that doesnt
> > advance
> > >> is an issue even if the specific file doesnt trigger it in a
> > different
> > >> implementation
> > >>
> > >> also probaly a good idea if you contact rcombs as you seemed to
> > work
> > >> on
> > >> the same code
> > >>
> > >> I was looking at teh ticket and saw a link to rcombs patch, looked
> > at
> > >> the patch and applied it. I did not realize there where 2 patches
> > >
> > >
> > > Hi Michael,
> > >
> > > I know the rcombs patch, but it has a - let's say - different
> > behavior.
> > > Let's look at an example where artist and genre have multiple
> > values:
> > >
> > >
> > > This was ffmpeg output unpatched:
> > >
> > > Metadata:
> > > title : Infinite (Original Mix)
> > > artist : B-Front
> > > track : 1
> > > album : Infinite
> > > date : 2017
> > > genre : Hardstyle
> > > TBPM : 150
> > > compilation : 0
> > > album_artist : B-Front
> > > publisher : Roughstate
> > >
> > >
> > > This is what the rcombs patch does:
> > >
> > > Metadata:
> > > title : Infinite (Original Mix)
> > > artist : B-Front
> > > artist : Second Artist Example
> > > track : 1
> > > album : Infinite
> > > date : 2017
> > > genre : Hardstyle
> > > genre : Test
> > > genre : Example
> > > genre : Hard Dance
> > > TBPM : 150
> > > compilation : 0
> > > album_artist : B-Front
> > > publisher : Roughstate
> > >
> > >
> > >
> > > My path does that:
> > >
> > > Metadata:
> > > title : Infinite (Original Mix)
> > > artist : B-Front;Second Artist Example
> > > track : 1
> > > album : Infinite
> > > date : 2017
> > > genre : Hardstyle;Test;Example;Hard Dance
> > > TBPM : 150
> > > compilation : 0
> > > album_artist : B-Front
> > > publisher : Roughstate
> > >
> > >
> > >
> > > I'm not sure whether it is even allowed or intended that there are
> > > multiple metadata entries with the same key?
> >=20
> > It is indeed an intended feature of the AVDictionary system, and the
> > metadata feature in particular.
>=20
> Hi,
>=20
> you meant to say that it is an intended change introduced by your patch? =
=F0=9F=98=89=20
>=20
> Because we haven't had duplicate metadata keys before. I'm afraid but=20
> I think these changes should be reverted, because it also creates invalid
> FFprobe output - like in case of JSON for example:
>=20
> {
> "format": {
> "filename": "multiple_id3v2_4_values.mp3",
> "nb_streams": 1,
> "nb_programs": 0,
> "nb_stream_groups": 0,
> "format_name": "mp3",
> "format_long_name": "MP2/3 (MPEG audio layer 2/3)",
> "start_time": "0.011995",
> "duration": "20.035918",
> "size": "804936",
> "bit_rate": "321397",
> "probe_score": 51,
> "tags": {
> "title": "Infinite (Original Mix)",
> "artist": "B-Front",
> "artist": "Second Artist Example",
> "track": "1",
> "album": "Infinite",
> "date": "2017",
> "genre": "Hardstyle",
> "genre": "Test",
> "genre": "Example",
> "genre": "Hard Dance",
> "TBPM": "150",
> "compilation": "0",
> "album_artist": "B-Front",
> "publisher": "Roughstate",
> "encoder": "Lavf57.83.100",
> }
> }
> }
I think ffprobe should not generate invalid output if metadata given to it =
has
2 entries with the same key, independant of that being the correct way to e=
xport
metadata.
thx
[...]
--=20
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Nations do behave wisely once they have exhausted all other alternatives.=
=20
-- Abba Eban
--BZ8Mbs+radZV19pw
Content-Type: application/pgp-signature; name="signature.asc"
-----BEGIN PGP SIGNATURE-----
iF0EABEKAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZ/6mWAAKCRBhHseHBAsP
qzC6AKCOOQ5zr28VdSy0xIyDTeFPJ/G6vQCgmH5swmKFuMvWD5i8s3/LpWRHmOk=
=wPTZ
-----END PGP SIGNATURE-----
--BZ8Mbs+radZV19pw--
--===============1644852669430401197==
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".
--===============1644852669430401197==--