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 0BB0A47754 for ; Sun, 22 Oct 2023 07:06:28 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 8B90468CA51; Sun, 22 Oct 2023 10:06:25 +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 4D81268C94A; Sun, 22 Oct 2023 10:06:14 +0300 (EEST) Received: by mail.gandi.net (Postfix) with ESMTPSA id 980BFE0002; Sun, 22 Oct 2023 07:06:13 +0000 (UTC) Date: Sun, 22 Oct 2023 09:06:12 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20231022070612.GV3543730@pb2> References: <20230906221928.9C116410B55@natalya.videolan.org> <20231021001255.GD2105706@pb2> <20231021225640.GS3543730@pb2> MIME-Version: 1.0 In-Reply-To: <20231021225640.GS3543730@pb2> X-GND-Sasl: michael@niedermayer.cc Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/magicyuv: add vlc multi 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 Cc: cc@ffmpeg.org Content-Type: multipart/mixed; boundary="===============4975580513077878212==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============4975580513077878212== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="K2szJh/f7LS5Tjzd" Content-Disposition: inline --K2szJh/f7LS5Tjzd Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Oct 22, 2023 at 12:56:40AM +0200, Michael Niedermayer wrote: > On Sat, Oct 21, 2023 at 11:00:19AM +0200, Paul B Mahol wrote: > > On Sat, Oct 21, 2023 at 2:13=E2=80=AFAM Michael Niedermayer > > wrote: > >=20 > > > On Wed, Sep 06, 2023 at 10:19:27PM +0000, Paul B Mahol wrote: > > > > ffmpeg | branch: master | Paul B Mahol | Mon Aug= 28 > > > 12:20:15 2023 +0200| [8b7391cb5ff94ce94612fda69392a95d7ab1ffd0] | > > > committer: Paul B Mahol > > > > > > > > avcodec/magicyuv: add vlc multi support > > > > > > > > Gives nice speed boost, depending on encoded content it goes from > > > > 30% to 60% faster. > > > > > > > > > > > > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=3Dcommit;h=3D8b7391c= b5ff94ce94612fda69392a95d7ab1ffd0 > > > > --- > > > > > > > > libavcodec/magicyuv.c | 65 > > > +++++++++++++++++++++++++++------------------------ > > > > 1 file changed, 34 insertions(+), 31 deletions(-) > > > > > > > > diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c > > > > index 7898cd5be4..bbaf14d0e0 100644 > > > > --- a/libavcodec/magicyuv.c > > > > +++ b/libavcodec/magicyuv.c > > > > @@ -34,6 +34,8 @@ > > > > #include "lossless_videodsp.h" > > > > #include "thread.h" > > > > > > > > +#define VLC_BITS 12 > > > > + > > > > typedef struct Slice { > > > > uint32_t start; > > > > uint32_t size; > > > > @@ -67,13 +69,14 @@ typedef struct MagicYUVContext { > > > > Slice *slices[4]; // slice bitstream positions= for > > > each plane > > > > unsigned int slices_size[4]; // slice sizes for each plane > > > > VLC vlc[4]; // VLC for each plane > > > > + VLC_MULTI multi[4]; // Buffer for joint VLC data > > > > int (*magy_decode_slice)(AVCodecContext *avctx, void *tdata, > > > > int j, int threadnr); > > > > LLVidDSPContext llviddsp; > > > > } MagicYUVContext; > > > > > > > > static int huff_build(const uint8_t len[], uint16_t codes_pos[33], > > > > - VLC *vlc, int nb_elems, void *logctx) > > > > + VLC *vlc, VLC_MULTI *multi, int nb_elems, vo= id > > > *logctx) > > > > { > > > > HuffEntry he[4096]; > > > > > > > > @@ -84,7 +87,8 @@ static int huff_build(const uint8_t len[], uint16= _t > > > codes_pos[33], > > > > he[--codes_pos[len[i]]] =3D (HuffEntry){ len[i], i }; > > > > > > > > ff_free_vlc(vlc); > > > > - return ff_init_vlc_from_lengths(vlc, FFMIN(he[0].len, 12), nb_= elems, > > > > + ff_free_vlc_multi(multi); > > > > + return ff_init_vlc_multi_from_lengths(vlc, multi, FFMIN(he[0].= len, > > > VLC_BITS), nb_elems, nb_elems, > > > > &he[0].len, sizeof(he[0]), > > > > &he[0].sym, sizeof(he[0]), > > > sizeof(he[0].sym), > > > > 0, 0, logctx); > > > > @@ -111,6 +115,22 @@ static void magicyuv_median_pred16(uint16_t *d= st, > > > const uint16_t *src1, > > > > *left_top =3D lt; > > > > } > > > > > > > > +#define READ_PLANE(dst, plane, b, c) \ > > > > +{ \ > > > > + x =3D 0; \ > > > > + for (; CACHED_BITSTREAM_READER && x < width-c && get_bits_left= (&gb) > > > > 0;) {\ > > > > + ret =3D get_vlc_multi(&gb, (uint8_t *)dst + x * b, multi, \ > > > > + vlc, vlc_bits, 3); \ > > > > + if (ret > 0) \ > > > > + x +=3D ret; \ > > > > + if (ret <=3D 0) \ > > > > + return AVERROR_INVALIDDATA; \ > > > > + } \ > > > > + for (; x < width && get_bits_left(&gb) > 0; x++) \ > > > > + dst[x] =3D get_vlc2(&gb, vlc, vlc_bits, 3); \ > > > > + dst +=3D stride; \ > > > > +} > > > > + > > > > static int magy_decode_slice10(AVCodecContext *avctx, void *tdata, > > > > int j, int threadnr) > > > > { > > > > @@ -130,6 +150,9 @@ static int magy_decode_slice10(AVCodecContext > > > *avctx, void *tdata, > > > > int sheight =3D AV_CEIL_RSHIFT(s->slice_height, s->vshift[= i]); > > > > ptrdiff_t fake_stride =3D (p->linesize[i] / 2) * (1 + inte= rlaced); > > > > ptrdiff_t stride =3D p->linesize[i] / 2; > > > > + const VLC_MULTI_ELEM *const multi =3D s->multi[i].table; > > > > + const VLCElem *const vlc =3D s->vlc[i].table; > > > > + const int vlc_bits =3D s->vlc[i].bits; > > > > int flags, pred; > > > > int ret =3D init_get_bits8(&gb, s->buf + s->slices[i][j].s= tart, > > > > s->slices[i][j].size); > > > > @@ -151,20 +174,8 @@ static int magy_decode_slice10(AVCodecContext > > > *avctx, void *tdata, > > > > dst +=3D stride; > > > > } > > > > } else { > > > > - for (k =3D 0; k < height; k++) { > > > > - for (x =3D 0; x < width; x++) { > > > > - int pix; > > > > - if (get_bits_left(&gb) <=3D 0) > > > > - return AVERROR_INVALIDDATA; > > > > - > > > > - pix =3D get_vlc2(&gb, s->vlc[i].table, > > > s->vlc[i].bits, 3); > > > > - if (pix < 0) > > > > - return AVERROR_INVALIDDATA; > > > > - > > > > - dst[x] =3D pix; > > > > - } > > > > - dst +=3D stride; > > > > - } > > > > + for (k =3D 0; k < height; k++) > > > > + READ_PLANE(dst, i, 2, 3) > > > > } > > > > > > > > switch (pred) { > > > > @@ -261,6 +272,9 @@ static int magy_decode_slice(AVCodecContext *av= ctx, > > > void *tdata, > > > > ptrdiff_t fake_stride =3D p->linesize[i] * (1 + interlaced= ); > > > > ptrdiff_t stride =3D p->linesize[i]; > > > > const uint8_t *slice =3D s->buf + s->slices[i][j].start; > > > > + const VLC_MULTI_ELEM *const multi =3D s->multi[i].table; > > > > + const VLCElem *const vlc =3D s->vlc[i].table; > > > > + const int vlc_bits =3D s->vlc[i].bits; > > > > int flags, pred; > > > > > > > > flags =3D bytestream_get_byte(&slice); > > > > @@ -280,20 +294,8 @@ static int magy_decode_slice(AVCodecContext *a= vctx, > > > void *tdata, > > > > if (ret < 0) > > > > return ret; > > > > > > > > - for (k =3D 0; k < height; k++) { > > > > - for (x =3D 0; x < width; x++) { > > > > - int pix; > > > > - if (get_bits_left(&gb) <=3D 0) > > > > - return AVERROR_INVALIDDATA; > > > > - > > > > - pix =3D get_vlc2(&gb, s->vlc[i].table, > > > s->vlc[i].bits, 3); > > > > - if (pix < 0) > > > > - return AVERROR_INVALIDDATA; > > > > - > > > > - dst[x] =3D pix; > > > > - } > > > > - dst +=3D stride; > > > > - } > > > > + for (k =3D 0; k < height; k++) > > > > + READ_PLANE(dst, i, 1, 5) > > > > } > > > > > > > > switch (pred) { > > > > > > Who reviewed this ? > > > > > > This is a straight out of array write > > > writing 8 bytes while the check assumes its max 5 > > > > >=20 > > Why are you becoming so aggressive? >=20 > I objected to this patchset because of copy and pasted (duplicated) code, as it was disputed, heres the link: https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2023-September/314414.html > you ignored this objection and pushed it anyway. Now its found out that > it contains a out of array write. After a DOS vulnerability was found > previosuly in it. [...] thx --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Many that live deserve death. And some that die deserve life. Can you give it to them? Then do not be too eager to deal out death in judgement. For even the very wise cannot see all ends. -- Gandalf --K2szJh/f7LS5Tjzd Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iFwEABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZTTJ4AAKCRBhHseHBAsP q2/PAJ9pwhUQ6FI8vvrEqtK6SDdEsLzuMQCY9ZJEbW4luxR1/ktfUuiGl/IO5Q== =DdVm -----END PGP SIGNATURE----- --K2szJh/f7LS5Tjzd-- --===============4975580513077878212== 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". --===============4975580513077878212==--