On Sat, Oct 21, 2023 at 2:13 AM Michael Niedermayer <mich...@niedermayer.cc> wrote:
> On Wed, Sep 06, 2023 at 10:19:27PM +0000, Paul B Mahol wrote: > > ffmpeg | branch: master | Paul B Mahol <one...@gmail.com> | 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=commit;h=8b7391cb5ff94ce94612fda69392a95d7ab1ffd0 > > --- > > > > 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, void > *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]]] = (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 *dst, > const uint16_t *src1, > > *left_top = lt; > > } > > > > +#define READ_PLANE(dst, plane, b, c) \ > > +{ \ > > + x = 0; \ > > + for (; CACHED_BITSTREAM_READER && x < width-c && get_bits_left(&gb) > > 0;) {\ > > + ret = get_vlc_multi(&gb, (uint8_t *)dst + x * b, multi, \ > > + vlc, vlc_bits, 3); \ > > + if (ret > 0) \ > > + x += ret; \ > > + if (ret <= 0) \ > > + return AVERROR_INVALIDDATA; \ > > + } \ > > + for (; x < width && get_bits_left(&gb) > 0; x++) \ > > + dst[x] = get_vlc2(&gb, vlc, vlc_bits, 3); \ > > + dst += 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 = AV_CEIL_RSHIFT(s->slice_height, s->vshift[i]); > > ptrdiff_t fake_stride = (p->linesize[i] / 2) * (1 + interlaced); > > ptrdiff_t stride = p->linesize[i] / 2; > > + const VLC_MULTI_ELEM *const multi = s->multi[i].table; > > + const VLCElem *const vlc = s->vlc[i].table; > > + const int vlc_bits = s->vlc[i].bits; > > int flags, pred; > > int ret = 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 += stride; > > } > > } else { > > - for (k = 0; k < height; k++) { > > - for (x = 0; x < width; x++) { > > - int pix; > > - if (get_bits_left(&gb) <= 0) > > - return AVERROR_INVALIDDATA; > > - > > - pix = get_vlc2(&gb, s->vlc[i].table, > s->vlc[i].bits, 3); > > - if (pix < 0) > > - return AVERROR_INVALIDDATA; > > - > > - dst[x] = pix; > > - } > > - dst += stride; > > - } > > + for (k = 0; k < height; k++) > > + READ_PLANE(dst, i, 2, 3) > > } > > > > switch (pred) { > > @@ -261,6 +272,9 @@ static int magy_decode_slice(AVCodecContext *avctx, > void *tdata, > > ptrdiff_t fake_stride = p->linesize[i] * (1 + interlaced); > > ptrdiff_t stride = p->linesize[i]; > > const uint8_t *slice = s->buf + s->slices[i][j].start; > > + const VLC_MULTI_ELEM *const multi = s->multi[i].table; > > + const VLCElem *const vlc = s->vlc[i].table; > > + const int vlc_bits = s->vlc[i].bits; > > int flags, pred; > > > > flags = bytestream_get_byte(&slice); > > @@ -280,20 +294,8 @@ static int magy_decode_slice(AVCodecContext *avctx, > void *tdata, > > if (ret < 0) > > return ret; > > > > - for (k = 0; k < height; k++) { > > - for (x = 0; x < width; x++) { > > - int pix; > > - if (get_bits_left(&gb) <= 0) > > - return AVERROR_INVALIDDATA; > > - > > - pix = get_vlc2(&gb, s->vlc[i].table, > s->vlc[i].bits, 3); > > - if (pix < 0) > > - return AVERROR_INVALIDDATA; > > - > > - dst[x] = pix; > > - } > > - dst += stride; > > - } > > + for (k = 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 > Why are you becoming so aggressive? The function writes at once 8 bytes, even though only 6 values (in 8bit case) are kept. Can you share sample? > ==16861==ERROR: AddressSanitizer: heap-buffer-overflow on address > 0x631000024800 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_template.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*, > unsigned long) Fuzzer/build/../FuzzerLoop.cpp:495:13 > #10 0x29eaae2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, > unsigned long) Fuzzer/build/../FuzzerDriver.cpp:273:6 > #11 0x29efce1 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned > char 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+0x420339) > > > 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 > undocumented > > why do we in 2023 add undocumented non trivial functions into files where > basically > every function prior is documented ? > > [...] > -- > 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 > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
