diff mbox

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

Message ID 20180617042117.3844-2-ramiro.polla@gmail.com
State New
Headers show

Commit Message

Ramiro Polla June 17, 2018, 4:21 a.m. UTC 
---
 libavcodec/mpegutils.c | 115 +++++++++++++++++++++++++++++--------------------
 libavcodec/mpegutils.h |   7 +++
 2 files changed, 76 insertions(+), 46 deletions(-)

Comments

Carl Eugen Hoyos June 17, 2018, 10:58 a.m. UTC | #1 
2018-06-17 6:21 GMT+02:00, Ramiro Polla <ramiro.polla@gmail.com>:

> +int ff_mb_type_str(char *str, int size, int mb_type)

I don't think you can use "ff_" functions from a tool, you
have to make this part of the API afaict.

Carl Eugen
Michael Niedermayer June 17, 2018, 8:52 p.m. UTC | #2 
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

[...]
diff mbox

Patch

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);
                 }
             }
             av_log(avctx, AV_LOG_DEBUG, "\n");
diff --git a/libavcodec/mpegutils.h b/libavcodec/mpegutils.h
index 1ed21c19be..11856f9edb 100644
--- a/libavcodec/mpegutils.h
+++ b/libavcodec/mpegutils.h
@@ -138,6 +138,13 @@  void ff_draw_horiz_band(AVCodecContext *avctx, AVFrame *cur, AVFrame *last,
                         int low_delay);
 
 /**
+ * Fill buffer with a string representing the macroblock type.
+ *
+ * @return number of characters written to str
+ */
+int ff_mb_type_str(char *str, int size, int mb_type);
+
+/**
  * Print debugging info for the given picture.
  */
 void ff_print_debug_info2(AVCodecContext *avctx, AVFrame *pict, uint8_t *mbskip_table,