[FFmpeg-devel] [PATCH 2/4] mpegutils: split debug function that prints mb_type so it may be used by ffprobe

[Michael Niedermayer]

On Sun, Jun 17, 2018 at 06:21:15AM +0200, Ramiro Polla wrote:
> ---
>  libavcodec/mpegutils.c | 115 +++++++++++++++++++++++++++++--------------------
>  libavcodec/mpegutils.h |   7 +++
>  2 files changed, 76 insertions(+), 46 deletions(-)
> 
> diff --git a/libavcodec/mpegutils.c b/libavcodec/mpegutils.c
> index 0fbe5f8c9d..12c2468797 100644
> --- a/libavcodec/mpegutils.c
> +++ b/libavcodec/mpegutils.c
> @@ -100,6 +100,72 @@ void ff_draw_horiz_band(AVCodecContext *avctx,
>      }
>  }
>  
> +int ff_mb_type_str(char *str, int size, int mb_type)
> +{
> +    char *ptr = str;
> +
> +    if (size <= 0)
> +        return 0;
> +
> +    if (--size <= 0)
> +        goto end;
> +
> +    // Type & MV direction
> +    if (IS_PCM(mb_type))
> +        *ptr++ = 'P';
> +    else if (IS_INTRA(mb_type) && IS_ACPRED(mb_type))
> +        *ptr++ = 'A';
> +    else if (IS_INTRA4x4(mb_type))
> +        *ptr++ = 'i';
> +    else if (IS_INTRA16x16(mb_type))
> +        *ptr++ = 'I';
> +    else if (IS_DIRECT(mb_type) && IS_SKIP(mb_type))
> +        *ptr++ = 'd';
> +    else if (IS_DIRECT(mb_type))
> +        *ptr++ = 'D';
> +    else if (IS_GMC(mb_type) && IS_SKIP(mb_type))
> +        *ptr++ = 'g';
> +    else if (IS_GMC(mb_type))
> +        *ptr++ = 'G';
> +    else if (IS_SKIP(mb_type))
> +        *ptr++ = 'S';
> +    else if (!USES_LIST(mb_type, 1))
> +        *ptr++ = '>';
> +    else if (!USES_LIST(mb_type, 0))
> +        *ptr++ = '<';
> +    else {
> +        av_assert2(USES_LIST(mb_type, 0) && USES_LIST(mb_type, 1));
> +        *ptr++ = 'X';
> +    }
> +
> +    if (--size <= 0)
> +        goto end;
> +
> +    // segmentation
> +    if (IS_8X8(mb_type))
> +        *ptr++ = '+';
> +    else if (IS_16X8(mb_type))
> +        *ptr++ = '-';
> +    else if (IS_8X16(mb_type))
> +        *ptr++ = '|';
> +    else if (IS_INTRA(mb_type) || IS_16X16(mb_type))
> +        *ptr++ = ' ';
> +    else
> +        *ptr++ = '?';
> +
> +    if (--size <= 0)
> +        goto end;
> +
> +    if (IS_INTERLACED(mb_type))
> +        *ptr++ = '=';
> +    else
> +        *ptr++ = ' ';
> +
> +end:
> +    *ptr = '\0';
> +    return ptr - str;
> +}
> +
>  void ff_print_debug_info2(AVCodecContext *avctx, AVFrame *pict, uint8_t *mbskip_table,
>                           uint32_t *mbtype_table, int8_t *qscale_table, int16_t (*motion_val[2])[2],
>                           int *low_delay,
> @@ -231,52 +297,9 @@ void ff_print_debug_info2(AVCodecContext *avctx, AVFrame *pict, uint8_t *mbskip_
>                             qscale_table[x + y * mb_stride]);
>                  }
>                  if (avctx->debug & FF_DEBUG_MB_TYPE) {
> -                    int mb_type = mbtype_table[x + y * mb_stride];
> -                    // Type & MV direction
> -                    if (IS_PCM(mb_type))
> -                        av_log(avctx, AV_LOG_DEBUG, "P");
> -                    else if (IS_INTRA(mb_type) && IS_ACPRED(mb_type))
> -                        av_log(avctx, AV_LOG_DEBUG, "A");
> -                    else if (IS_INTRA4x4(mb_type))
> -                        av_log(avctx, AV_LOG_DEBUG, "i");
> -                    else if (IS_INTRA16x16(mb_type))
> -                        av_log(avctx, AV_LOG_DEBUG, "I");
> -                    else if (IS_DIRECT(mb_type) && IS_SKIP(mb_type))
> -                        av_log(avctx, AV_LOG_DEBUG, "d");
> -                    else if (IS_DIRECT(mb_type))
> -                        av_log(avctx, AV_LOG_DEBUG, "D");
> -                    else if (IS_GMC(mb_type) && IS_SKIP(mb_type))
> -                        av_log(avctx, AV_LOG_DEBUG, "g");
> -                    else if (IS_GMC(mb_type))
> -                        av_log(avctx, AV_LOG_DEBUG, "G");
> -                    else if (IS_SKIP(mb_type))
> -                        av_log(avctx, AV_LOG_DEBUG, "S");
> -                    else if (!USES_LIST(mb_type, 1))
> -                        av_log(avctx, AV_LOG_DEBUG, ">");
> -                    else if (!USES_LIST(mb_type, 0))
> -                        av_log(avctx, AV_LOG_DEBUG, "<");
> -                    else {
> -                        av_assert2(USES_LIST(mb_type, 0) && USES_LIST(mb_type, 1));
> -                        av_log(avctx, AV_LOG_DEBUG, "X");
> -                    }
> -
> -                    // segmentation
> -                    if (IS_8X8(mb_type))
> -                        av_log(avctx, AV_LOG_DEBUG, "+");
> -                    else if (IS_16X8(mb_type))
> -                        av_log(avctx, AV_LOG_DEBUG, "-");
> -                    else if (IS_8X16(mb_type))
> -                        av_log(avctx, AV_LOG_DEBUG, "|");
> -                    else if (IS_INTRA(mb_type) || IS_16X16(mb_type))
> -                        av_log(avctx, AV_LOG_DEBUG, " ");
> -                    else
> -                        av_log(avctx, AV_LOG_DEBUG, "?");
> -
> -
> -                    if (IS_INTERLACED(mb_type))
> -                        av_log(avctx, AV_LOG_DEBUG, "=");
> -                    else
> -                        av_log(avctx, AV_LOG_DEBUG, " ");
> +                    char str[4];
> +                    ff_mb_type_str(str, sizeof(str), mbtype_table[x + y * mb_stride]);

> +                    av_log(avctx, AV_LOG_DEBUG, str);

This style of code is risky and gcc refuses to build it here:
libavcodec/mpegutils.c: In function ‘ff_print_debug_info2’:
libavcodec/mpegutils.c:302:21: error: format not a string literal and no format arguments [-Werror=format-security]
                     av_log(avctx, AV_LOG_DEBUG, str);
                     ^

This should use probably something like
"%s", str
that way funny symbols in str can never cause issues

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180617/b19405ed/attachment.sig>