[FFmpeg-devel] [PATCH 3/4] avcodec/mpegutils: consolidate single byte av_log()

[Michael Niedermayer]

On Fri, Sep 03, 2021 at 09:00:22PM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Fixes: Timeout (56sec -> 15sec)
> > Fixes: 37141/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MPEG4_fuzzer-6192122867875840
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > ---
> >  libavcodec/mpegutils.c | 56 ++++++++++++++++++++++++------------------
> >  1 file changed, 32 insertions(+), 24 deletions(-)
> > 
> > diff --git a/libavcodec/mpegutils.c b/libavcodec/mpegutils.c
> > index e5105ecc58..e91c554781 100644
> > --- a/libavcodec/mpegutils.c
> > +++ b/libavcodec/mpegutils.c
> > @@ -187,7 +187,6 @@ void ff_print_debug_info2(AVCodecContext *avctx, AVFrame *pict, uint8_t *mbskip_
> >  
> >          av_freep(&mvs);
> >      }
> > -
> >      /* TODO: export all the following to make them accessible for users (and filters) */
> >      if (avctx->hwaccel || !mbtype_table)
> >          return;
> > @@ -195,71 +194,80 @@ void ff_print_debug_info2(AVCodecContext *avctx, AVFrame *pict, uint8_t *mbskip_
> >  
> >      if (avctx->debug & (FF_DEBUG_SKIP | FF_DEBUG_QP | FF_DEBUG_MB_TYPE)) {
> >          int x,y;
> > +#define MB_STRING_SIZE 6
> > +        char *mbstring = av_malloc(MB_STRING_SIZE * mb_width + 1);
> 
> Is it guaranteed that mb_width can't be huge? (I wouldn't be surprised
> if there were a compile-time bound for it; this could be used for a
> stack allocation.)

i had no major problem creating a file with a mb_width above 40k, the failure
was from allocating the image if the size was increased not anything inherent
to mb_width
not every codec and container allows this but some do, i picked 
msmpeg4v3 in nut but that is not a unique choice at all


> 
> > +        if (!mbstring)
> > +            return;
> >  
> >          av_log(avctx, AV_LOG_DEBUG, "New frame, type: %c\n",
> >                 av_get_picture_type_char(pict->pict_type));
> >          for (y = 0; y < mb_height; y++) {
> > +            char *mbs = mbstring;
> >              for (x = 0; x < mb_width; x++) {
> > +                av_assert0(mbs - mbstring <= MB_STRING_SIZE * x);
> >                  if (avctx->debug & FF_DEBUG_SKIP) {
> >                      int count = mbskip_table ? mbskip_table[x + y * mb_stride] : 0;
> >                      if (count > 9)
> >                          count = 9;
> > -                    av_log(avctx, AV_LOG_DEBUG, "%1d", count);
> > +                    *mbs++ = '0' + count;
> >                  }
> >                  if (avctx->debug & FF_DEBUG_QP) {
> > -                    av_log(avctx, AV_LOG_DEBUG, "%2d",
> > -                           qscale_table[x + y * mb_stride]);
> > +                    int q = qscale_table[x + y * mb_stride];
> > +                    *mbs++ = '0' + q/10;
> > +                    *mbs++ = '0' + q%10;
> 
> This is only equivalent to the old code if the value is in the range
> 0-99. Is this guaranteed?

the code prior to this assumed so, if it exceeded it, the output would
become difficult to read. So if this assumtation is wrong we would need to
already fix that, Do you know of a case where this is wrong ?

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210904/b7c69627/attachment.sig>