Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH] avcodec/hevcdec: Output MD5-message in one piece
@ 2022-07-22 19:32 Andreas Rheinhardt
  2022-07-23 20:48 ` Andreas Rheinhardt
  0 siblings, 1 reply; 2+ messages in thread
From: Andreas Rheinhardt @ 2022-07-22 19:32 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Andreas Rheinhardt

Otherwise, there is no guarantee that the various av_log-messages
are not interrupted by another log statement. The latter may originate
from anywhere else, even the HEVC decoder itself, as happens when
one uses frame-threading to decode the BUMPING_A_ericsson_1.bit
sample from the FATE-suite.

Furthermore, the earlier approach suffered from the fact that
various parts of the logmsg were output with different loglevels
and that checking stopped after having encountered the first
plane with MD5 mismatch, although it is probably interesting to
know whether other planes are incorrect, too.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/hevcdec.c | 39 ++++++++++++++++++---------------------
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index 2d06d09b75..3c95398b96 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -26,6 +26,7 @@
 #include "config_components.h"
 
 #include "libavutil/attributes.h"
+#include "libavutil/avstring.h"
 #include "libavutil/common.h"
 #include "libavutil/display.h"
 #include "libavutil/film_grain_params.h"
@@ -3374,17 +3375,12 @@ fail:
     return ret;
 }
 
-static void print_md5(void *log_ctx, int level, uint8_t md5[16])
-{
-    int i;
-    for (i = 0; i < 16; i++)
-        av_log(log_ctx, level, "%02"PRIx8, md5[i]);
-}
-
 static int verify_md5(HEVCContext *s, AVFrame *frame)
 {
     const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(frame->format);
+    char msg_buf[4 * (50 + 2 * 2 * 16 /* MD5-size */)];
     int pixel_shift;
+    int err = 0;
     int i, j;
 
     if (!desc)
@@ -3392,9 +3388,6 @@ static int verify_md5(HEVCContext *s, AVFrame *frame)
 
     pixel_shift = desc->comp[0].depth > 8;
 
-    av_log(s->avctx, AV_LOG_DEBUG, "Verifying checksum for frame with POC %d: ",
-           s->poc);
-
     /* the checksums are LE, so we have to byteswap for >8bpp formats
      * on BE arches */
 #if HAVE_BIGENDIAN
@@ -3407,6 +3400,7 @@ static int verify_md5(HEVCContext *s, AVFrame *frame)
     }
 #endif
 
+    msg_buf[0] = '\0';
     for (i = 0; frame->data[i]; i++) {
         int width  = s->avctx->coded_width;
         int height = s->avctx->coded_height;
@@ -3428,23 +3422,26 @@ static int verify_md5(HEVCContext *s, AVFrame *frame)
         }
         av_md5_final(s->md5_ctx, md5);
 
+#define MD5_PRI "%016" PRIx64 "%016" PRIx64
+#define MD5_PRI_ARG(buf) AV_RB64(buf), AV_RB64((const uint8_t*)(buf) + 8)
+
         if (!memcmp(md5, s->sei.picture_hash.md5[i], 16)) {
-            av_log   (s->avctx, AV_LOG_DEBUG, "plane %d - correct ", i);
-            print_md5(s->avctx, AV_LOG_DEBUG, md5);
-            av_log   (s->avctx, AV_LOG_DEBUG, "; ");
+            av_strlcatf(msg_buf, sizeof(msg_buf),
+                        "plane %d - correct " MD5_PRI "; ",
+                        i, MD5_PRI_ARG(md5));
         } else {
-            av_log   (s->avctx, AV_LOG_ERROR, "mismatching checksum of plane %d - ", i);
-            print_md5(s->avctx, AV_LOG_ERROR, md5);
-            av_log   (s->avctx, AV_LOG_ERROR, " != ");
-            print_md5(s->avctx, AV_LOG_ERROR, s->sei.picture_hash.md5[i]);
-            av_log   (s->avctx, AV_LOG_ERROR, "\n");
-            return AVERROR_INVALIDDATA;
+            av_strlcatf(msg_buf, sizeof(msg_buf),
+                       "mismatching checksum of plane %d - " MD5_PRI " != " MD5_PRI "; ",
+                        i, MD5_PRI_ARG(md5), MD5_PRI_ARG(s->sei.picture_hash.md5[i]));
+            err = AVERROR_INVALIDDATA;
         }
     }
 
-    av_log(s->avctx, AV_LOG_DEBUG, "\n");
+    av_log(s->avctx, err < 0 ? AV_LOG_ERROR : AV_LOG_DEBUG,
+           "Verifying checksum for frame with POC %d: %s\n",
+           s->poc, msg_buf);
 
-    return 0;
+    return err;
 }
 
 static int hevc_decode_extradata(HEVCContext *s, uint8_t *buf, int length, int first)
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avcodec/hevcdec: Output MD5-message in one piece
  2022-07-22 19:32 [FFmpeg-devel] [PATCH] avcodec/hevcdec: Output MD5-message in one piece Andreas Rheinhardt
@ 2022-07-23 20:48 ` Andreas Rheinhardt
  0 siblings, 0 replies; 2+ messages in thread
From: Andreas Rheinhardt @ 2022-07-23 20:48 UTC (permalink / raw)
  To: ffmpeg-devel

Andreas Rheinhardt:
> Otherwise, there is no guarantee that the various av_log-messages
> are not interrupted by another log statement. The latter may originate
> from anywhere else, even the HEVC decoder itself, as happens when
> one uses frame-threading to decode the BUMPING_A_ericsson_1.bit
> sample from the FATE-suite.
> 
> Furthermore, the earlier approach suffered from the fact that
> various parts of the logmsg were output with different loglevels
> and that checking stopped after having encountered the first
> plane with MD5 mismatch, although it is probably interesting to
> know whether other planes are incorrect, too.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/hevcdec.c | 39 ++++++++++++++++++---------------------
>  1 file changed, 18 insertions(+), 21 deletions(-)
> 
> diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
> index 2d06d09b75..3c95398b96 100644
> --- a/libavcodec/hevcdec.c
> +++ b/libavcodec/hevcdec.c
> @@ -26,6 +26,7 @@
>  #include "config_components.h"
>  
>  #include "libavutil/attributes.h"
> +#include "libavutil/avstring.h"
>  #include "libavutil/common.h"
>  #include "libavutil/display.h"
>  #include "libavutil/film_grain_params.h"
> @@ -3374,17 +3375,12 @@ fail:
>      return ret;
>  }
>  
> -static void print_md5(void *log_ctx, int level, uint8_t md5[16])
> -{
> -    int i;
> -    for (i = 0; i < 16; i++)
> -        av_log(log_ctx, level, "%02"PRIx8, md5[i]);
> -}
> -
>  static int verify_md5(HEVCContext *s, AVFrame *frame)
>  {
>      const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(frame->format);
> +    char msg_buf[4 * (50 + 2 * 2 * 16 /* MD5-size */)];
>      int pixel_shift;
> +    int err = 0;
>      int i, j;
>  
>      if (!desc)
> @@ -3392,9 +3388,6 @@ static int verify_md5(HEVCContext *s, AVFrame *frame)
>  
>      pixel_shift = desc->comp[0].depth > 8;
>  
> -    av_log(s->avctx, AV_LOG_DEBUG, "Verifying checksum for frame with POC %d: ",
> -           s->poc);
> -
>      /* the checksums are LE, so we have to byteswap for >8bpp formats
>       * on BE arches */
>  #if HAVE_BIGENDIAN
> @@ -3407,6 +3400,7 @@ static int verify_md5(HEVCContext *s, AVFrame *frame)
>      }
>  #endif
>  
> +    msg_buf[0] = '\0';
>      for (i = 0; frame->data[i]; i++) {
>          int width  = s->avctx->coded_width;
>          int height = s->avctx->coded_height;
> @@ -3428,23 +3422,26 @@ static int verify_md5(HEVCContext *s, AVFrame *frame)
>          }
>          av_md5_final(s->md5_ctx, md5);
>  
> +#define MD5_PRI "%016" PRIx64 "%016" PRIx64
> +#define MD5_PRI_ARG(buf) AV_RB64(buf), AV_RB64((const uint8_t*)(buf) + 8)
> +
>          if (!memcmp(md5, s->sei.picture_hash.md5[i], 16)) {
> -            av_log   (s->avctx, AV_LOG_DEBUG, "plane %d - correct ", i);
> -            print_md5(s->avctx, AV_LOG_DEBUG, md5);
> -            av_log   (s->avctx, AV_LOG_DEBUG, "; ");
> +            av_strlcatf(msg_buf, sizeof(msg_buf),
> +                        "plane %d - correct " MD5_PRI "; ",
> +                        i, MD5_PRI_ARG(md5));
>          } else {
> -            av_log   (s->avctx, AV_LOG_ERROR, "mismatching checksum of plane %d - ", i);
> -            print_md5(s->avctx, AV_LOG_ERROR, md5);
> -            av_log   (s->avctx, AV_LOG_ERROR, " != ");
> -            print_md5(s->avctx, AV_LOG_ERROR, s->sei.picture_hash.md5[i]);
> -            av_log   (s->avctx, AV_LOG_ERROR, "\n");
> -            return AVERROR_INVALIDDATA;
> +            av_strlcatf(msg_buf, sizeof(msg_buf),
> +                       "mismatching checksum of plane %d - " MD5_PRI " != " MD5_PRI "; ",
> +                        i, MD5_PRI_ARG(md5), MD5_PRI_ARG(s->sei.picture_hash.md5[i]));
> +            err = AVERROR_INVALIDDATA;
>          }
>      }
>  
> -    av_log(s->avctx, AV_LOG_DEBUG, "\n");
> +    av_log(s->avctx, err < 0 ? AV_LOG_ERROR : AV_LOG_DEBUG,
> +           "Verifying checksum for frame with POC %d: %s\n",
> +           s->poc, msg_buf);
>  
> -    return 0;
> +    return err;
>  }
>  
>  static int hevc_decode_extradata(HEVCContext *s, uint8_t *buf, int length, int first)

Will apply this tomorrow unless there are objections.

- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-07-23 20:48 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-22 19:32 [FFmpeg-devel] [PATCH] avcodec/hevcdec: Output MD5-message in one piece Andreas Rheinhardt
2022-07-23 20:48 ` Andreas Rheinhardt

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git