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 87C76465C2 for ; Sat, 21 Oct 2023 00:13:08 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 8666368CAEF; Sat, 21 Oct 2023 03:13:04 +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 B9E8368C620 for ; Sat, 21 Oct 2023 03:12:57 +0300 (EEST) Received: by mail.gandi.net (Postfix) with ESMTPSA id 61DC31C0003 for ; Sat, 21 Oct 2023 00:12:56 +0000 (UTC) Date: Sat, 21 Oct 2023 02:12:55 +0200 From: Michael Niedermayer To: ffmpeg-devel@ffmpeg.org Message-ID: <20231021001255.GD2105706@pb2> References: <20230906221928.9C116410B55@natalya.videolan.org> MIME-Version: 1.0 In-Reply-To: <20230906221928.9C116410B55@natalya.videolan.org> 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 Content-Type: multipart/mixed; boundary="===============1993906961339663649==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============1993906961339663649== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="r7U+bLA8boMOj+mD" Content-Disposition: inline --r7U+bLA8boMOj+mD Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 >=20 > avcodec/magicyuv: add vlc multi support >=20 > Gives nice speed boost, depending on encoded content it goes from > 30% to 60% faster. >=20 > > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=3Dcommit;h=3D8b7391cb5= ff94ce94612fda69392a95d7ab1ffd0 > --- >=20 > libavcodec/magicyuv.c | 65 +++++++++++++++++++++++++++------------------= ------ > 1 file changed, 34 insertions(+), 31 deletions(-) >=20 > 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" > =20 > +#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 e= ach 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; > =20 > 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, void *lo= gctx) > { > HuffEntry he[4096]; > =20 > @@ -84,7 +87,8 @@ static int huff_build(const uint8_t len[], uint16_t cod= es_pos[33], > he[--codes_pos[len[i]]] =3D (HuffEntry){ len[i], i }; > =20 > 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, V= LC_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 *dst, co= nst uint16_t *src1, > *left_top =3D lt; > } > =20 > +#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 + interlaced= ); > 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].start, > 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].bit= s, 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) > } > =20 > switch (pred) { > @@ -261,6 +272,9 @@ static int magy_decode_slice(AVCodecContext *avctx, v= oid *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; > =20 > flags =3D bytestream_get_byte(&slice); > @@ -280,20 +294,8 @@ static int magy_decode_slice(AVCodecContext *avctx, = void *tdata, > if (ret < 0) > return ret; > =20 > - 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].bit= s, 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) > } > =20 > switch (pred) { Who reviewed this ? This is a straight out of array write writing 8 bytes while the check assumes its max 5 =3D=3D16861=3D=3DERROR: AddressSanitizer: heap-buffer-overflow on address 0= x631000024800 at pc 0x000001552992 bp 0x7ffd8d34bf10 sp 0x7ffd8d34bf08 WRITE of size 8 at 0x631000024800 thread T0 #0 0x1552991 in bits_read_vlc_multi_be ffmpeg/libavcodec/bitstream_temp= late.h:531:9 #1 0x15453d0 in magy_decode_slice ffmpeg/libavcodec/magicyuv.c:302:17 #2 0x13f3738 in avcodec_default_execute2 ffmpeg/libavcodec/avcodec.c:76= :17 #3 0x153fba5 in magy_decode_frame ffmpeg/libavcodec/magicyuv.c:663:5 #4 0x142d60e in decode_simple_internal ffmpeg/libavcodec/decode.c:430:20 #5 0x142d60e in decode_simple_receive_frame ffmpeg/libavcodec/decode.c:= 609 #6 0x142d60e in decode_receive_frame_internal ffmpeg/libavcodec/decode.= c:637 #7 0x142c42f in avcodec_send_packet ffmpeg/libavcodec/decode.c:734:15 #8 0x4ce9df in LLVMFuzzerTestOneInput ffmpeg/tools/target_dec_fuzzer.c:= 579:25 #9 0x29f5f0d in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, u= nsigned long) Fuzzer/build/../FuzzerLoop.cpp:495:13 #10 0x29eaae2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsig= ned long) Fuzzer/build/../FuzzerDriver.cpp:273:6 #11 0x29efce1 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned c= har const*, unsigned long)) Fuzzer/build/../FuzzerDriver.cpp:690:9 #12 0x29ea7c0 in main Fuzzer/build/../FuzzerMain.cpp:20:10 #13 0x7f15a835ec86 in __libc_start_main /build/glibc-CVJwZb/glibc-2.27/= csu/../csu/libc-start.c:310 #14 0x420339 in _start (ffmpeg/tools/target_dec_magicyuv_fuzzer+0x42033= 9) the multi vlc code is not documented, teh function writing out of array is one of the very few functions in bitstream_template.h that is undocument= ed why do we in 2023 add undocumented non trivial functions into files where b= asically every function prior is documented ? [...] --=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 --r7U+bLA8boMOj+mD Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZTMXhAAKCRBhHseHBAsP q74GAJwJOdOBWoZKnBRR3xjspqTTyBiq6wCeNFeD3D+NjYTXUb5poZ8N+Y4fWBg= =7CPW -----END PGP SIGNATURE----- --r7U+bLA8boMOj+mD-- --===============1993906961339663649== 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". --===============1993906961339663649==--