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

[Michael Niedermayer]

On Fri, Sep 03, 2021 at 03:45:55PM -0300, James Almer wrote:
> On 9/3/2021 3:39 PM, Michael Niedermayer wrote:
> > 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);
> >       }
> > -
> 
> Stray change.
> 
> >       /* 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);
> > +        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;
> 
> This is ugly and not very obvious at first glance. Can you use lavu's bprint
> or something like that instead?

the whole point of the patch is to eliminate the string formating overhead.
(which surprisingly is significant)
so as long as we have a "%d" style format in there, it at least feels like we
arent doing a good job at eliminating it

would moving this in a seperate function resolve this concern too ?

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

"Nothing to hide" only works if the folks in power share the values of
you and everyone you know entirely and always will -- Tom Scott

-------------- 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/0849225e/attachment.sig>