diff mbox

[FFmpeg-devel,v3] avfilter/vf_showinfo: Fix erroneous results for mean and stdev with pixel bits >8

Message ID 20191129112823.14439-1-lance.lmwang@gmail.com
State New
Headers show

Commit Message

Lance Wang Nov. 29, 2019, 11:28 a.m. UTC 
From: Limin Wang <lance.lmwang@gmail.com>

Have tested with be and le pixel format on be and le system for >8bit.
System:
lmwang@ubuntu:~/ffmpeg.git.mips$ grep HAVE_BIGENDIAN config.h
#define HAVE_BIGENDIAN 1
ffmpeg.git git:(showinfo) ✗ grep HAVE_BIGENDIAN config.h
#define HAVE_BIGENDIAN 0

Test result:
1, yuv420p
./ffmpeg -f lavfi  -i color=black:duration=1:r=1:size=1280x720,format=yuv420p,showinfo
Master:
mean:[16 128 128] stdev:[0.0 0.0 0.0]
After applied the patch:
 mean:[16 128 128] stdev:[0.0 0.0 0.0]

2, yuv420p10le
./ffmpeg -f lavfi  -i color=black:duration=1:r=1:size=1280x720,format=yuv420p10le,showinfo
Master:
mean:[32 1 1] stdev:[32.0 1.0 1.0]
After applied the patch:
mean:[64 512 512] stdev:[0.0 0.0 0.0]

3, yuv420p10be
./ffmpeg -f lavfi  -i color=black:duration=1:r=1:size=1280x720,format=yuv420p10be,showinfo
Master:
mean:[32 1 1] stdev:[32.0 1.0 1.0]
After applied the patch:
mean:[64 512 512] stdev:[0.0 0.0 0.0]


Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavfilter/vf_showinfo.c | 34 +++++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

Comments

Lance Wang Jan. 6, 2020, 1:33 a.m. UTC | #1 
ping, please review it. I'll add packed format support after that.

On Fri, Nov 29, 2019 at 07:28:23PM +0800, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Have tested with be and le pixel format on be and le system for >8bit.
> System:
> lmwang@ubuntu:~/ffmpeg.git.mips$ grep HAVE_BIGENDIAN config.h
> #define HAVE_BIGENDIAN 1
> ffmpeg.git git:(showinfo) ✗ grep HAVE_BIGENDIAN config.h
> #define HAVE_BIGENDIAN 0
> 
> Test result:
> 1, yuv420p
> ./ffmpeg -f lavfi  -i color=black:duration=1:r=1:size=1280x720,format=yuv420p,showinfo
> Master:
> mean:[16 128 128] stdev:[0.0 0.0 0.0]
> After applied the patch:
>  mean:[16 128 128] stdev:[0.0 0.0 0.0]
> 
> 2, yuv420p10le
> ./ffmpeg -f lavfi  -i color=black:duration=1:r=1:size=1280x720,format=yuv420p10le,showinfo
> Master:
> mean:[32 1 1] stdev:[32.0 1.0 1.0]
> After applied the patch:
> mean:[64 512 512] stdev:[0.0 0.0 0.0]
> 
> 3, yuv420p10be
> ./ffmpeg -f lavfi  -i color=black:duration=1:r=1:size=1280x720,format=yuv420p10be,showinfo
> Master:
> mean:[32 1 1] stdev:[32.0 1.0 1.0]
> After applied the patch:
> mean:[64 512 512] stdev:[0.0 0.0 0.0]
> 
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavfilter/vf_showinfo.c | 34 +++++++++++++++++++++++++++++++---
>  1 file changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
> index 31f6b32aa4..5856722122 100644
> --- a/libavfilter/vf_showinfo.c
> +++ b/libavfilter/vf_showinfo.c
> @@ -24,6 +24,7 @@
>  
>  #include <inttypes.h>
>  
> +#include "libavutil/bswap.h"
>  #include "libavutil/adler32.h"
>  #include "libavutil/display.h"
>  #include "libavutil/imgutils.h"
> @@ -202,7 +203,7 @@ static void dump_color_property(AVFilterContext *ctx, AVFrame *frame)
>      av_log(ctx, AV_LOG_INFO, "\n");
>  }
>  
> -static void update_sample_stats(const uint8_t *src, int len, int64_t *sum, int64_t *sum2)
> +static void update_sample_stats_8(const uint8_t *src, int len, int64_t *sum, int64_t *sum2)
>  {
>      int i;
>  
> @@ -212,6 +213,30 @@ static void update_sample_stats(const uint8_t *src, int len, int64_t *sum, int64
>      }
>  }
>  
> +static void update_sample_stats_16(int be, const uint8_t *src, int len, int64_t *sum, int64_t *sum2)
> +{
> +    const uint16_t *src1 = (const uint16_t *)src;
> +    int i;
> +
> +    for (i = 0; i < len / 2; i++) {
> +        if ((HAVE_BIGENDIAN && !be) || (!HAVE_BIGENDIAN && be)) {
> +            *sum += av_bswap16(src1[i]);
> +            *sum2 += av_bswap16(src1[i]) * av_bswap16(src1[i]);
> +        } else {
> +            *sum += src1[i];
> +            *sum2 += src1[i] * src1[i];
> +        }
> +    }
> +}
> +
> +static void update_sample_stats(int depth, int be, const uint8_t *src, int len, int64_t *sum, int64_t *sum2)
> +{
> +    if (depth <= 8)
> +        update_sample_stats_8(src, len, sum, sum2);
> +    else
> +        update_sample_stats_16(be, src, len, sum, sum2);
> +}
> +
>  static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
>  {
>      AVFilterContext *ctx = inlink->dst;
> @@ -220,12 +245,15 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
>      uint32_t plane_checksum[4] = {0}, checksum = 0;
>      int64_t sum[4] = {0}, sum2[4] = {0};
>      int32_t pixelcount[4] = {0};
> +    int bitdepth = desc->comp[0].depth;
> +    int be = desc->flags & AV_PIX_FMT_FLAG_BE;
>      int i, plane, vsub = desc->log2_chroma_h;
>  
>      for (plane = 0; plane < 4 && s->calculate_checksums && frame->data[plane] && frame->linesize[plane]; plane++) {
>          uint8_t *data = frame->data[plane];
>          int h = plane == 1 || plane == 2 ? AV_CEIL_RSHIFT(inlink->h, vsub) : inlink->h;
>          int linesize = av_image_get_linesize(frame->format, frame->width, plane);
> +        int width = linesize >> (bitdepth > 8);
>  
>          if (linesize < 0)
>              return linesize;
> @@ -234,8 +262,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
>              plane_checksum[plane] = av_adler32_update(plane_checksum[plane], data, linesize);
>              checksum = av_adler32_update(checksum, data, linesize);
>  
> -            update_sample_stats(data, linesize, sum+plane, sum2+plane);
> -            pixelcount[plane] += linesize;
> +            update_sample_stats(bitdepth, be, data, linesize, sum+plane, sum2+plane);
> +            pixelcount[plane] += width;
>              data += frame->linesize[plane];
>          }
>      }
> -- 
> 2.21.0
>
Michael Niedermayer Jan. 7, 2020, 1:53 a.m. UTC | #2 
On Fri, Nov 29, 2019 at 07:28:23PM +0800, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Have tested with be and le pixel format on be and le system for >8bit.
> System:
> lmwang@ubuntu:~/ffmpeg.git.mips$ grep HAVE_BIGENDIAN config.h
> #define HAVE_BIGENDIAN 1
> ffmpeg.git git:(showinfo) ✗ grep HAVE_BIGENDIAN config.h
> #define HAVE_BIGENDIAN 0
> 
> Test result:
> 1, yuv420p
> ./ffmpeg -f lavfi  -i color=black:duration=1:r=1:size=1280x720,format=yuv420p,showinfo
> Master:
> mean:[16 128 128] stdev:[0.0 0.0 0.0]
> After applied the patch:
>  mean:[16 128 128] stdev:[0.0 0.0 0.0]
> 
> 2, yuv420p10le
> ./ffmpeg -f lavfi  -i color=black:duration=1:r=1:size=1280x720,format=yuv420p10le,showinfo
> Master:
> mean:[32 1 1] stdev:[32.0 1.0 1.0]
> After applied the patch:
> mean:[64 512 512] stdev:[0.0 0.0 0.0]
> 
> 3, yuv420p10be
> ./ffmpeg -f lavfi  -i color=black:duration=1:r=1:size=1280x720,format=yuv420p10be,showinfo
> Master:
> mean:[32 1 1] stdev:[32.0 1.0 1.0]
> After applied the patch:
> mean:[64 512 512] stdev:[0.0 0.0 0.0]
> 
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavfilter/vf_showinfo.c | 34 +++++++++++++++++++++++++++++++---
>  1 file changed, 31 insertions(+), 3 deletions(-)

will apply

thx

[...]
diff mbox

Patch

diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
index 31f6b32aa4..5856722122 100644
--- a/libavfilter/vf_showinfo.c
+++ b/libavfilter/vf_showinfo.c
@@ -24,6 +24,7 @@ 
 
 #include <inttypes.h>
 
+#include "libavutil/bswap.h"
 #include "libavutil/adler32.h"
 #include "libavutil/display.h"
 #include "libavutil/imgutils.h"
@@ -202,7 +203,7 @@  static void dump_color_property(AVFilterContext *ctx, AVFrame *frame)
     av_log(ctx, AV_LOG_INFO, "\n");
 }
 
-static void update_sample_stats(const uint8_t *src, int len, int64_t *sum, int64_t *sum2)
+static void update_sample_stats_8(const uint8_t *src, int len, int64_t *sum, int64_t *sum2)
 {
     int i;
 
@@ -212,6 +213,30 @@  static void update_sample_stats(const uint8_t *src, int len, int64_t *sum, int64
     }
 }
 
+static void update_sample_stats_16(int be, const uint8_t *src, int len, int64_t *sum, int64_t *sum2)
+{
+    const uint16_t *src1 = (const uint16_t *)src;
+    int i;
+
+    for (i = 0; i < len / 2; i++) {
+        if ((HAVE_BIGENDIAN && !be) || (!HAVE_BIGENDIAN && be)) {
+            *sum += av_bswap16(src1[i]);
+            *sum2 += av_bswap16(src1[i]) * av_bswap16(src1[i]);
+        } else {
+            *sum += src1[i];
+            *sum2 += src1[i] * src1[i];
+        }
+    }
+}
+
+static void update_sample_stats(int depth, int be, const uint8_t *src, int len, int64_t *sum, int64_t *sum2)
+{
+    if (depth <= 8)
+        update_sample_stats_8(src, len, sum, sum2);
+    else
+        update_sample_stats_16(be, src, len, sum, sum2);
+}
+
 static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
 {
     AVFilterContext *ctx = inlink->dst;
@@ -220,12 +245,15 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
     uint32_t plane_checksum[4] = {0}, checksum = 0;
     int64_t sum[4] = {0}, sum2[4] = {0};
     int32_t pixelcount[4] = {0};
+    int bitdepth = desc->comp[0].depth;
+    int be = desc->flags & AV_PIX_FMT_FLAG_BE;
     int i, plane, vsub = desc->log2_chroma_h;
 
     for (plane = 0; plane < 4 && s->calculate_checksums && frame->data[plane] && frame->linesize[plane]; plane++) {
         uint8_t *data = frame->data[plane];
         int h = plane == 1 || plane == 2 ? AV_CEIL_RSHIFT(inlink->h, vsub) : inlink->h;
         int linesize = av_image_get_linesize(frame->format, frame->width, plane);
+        int width = linesize >> (bitdepth > 8);
 
         if (linesize < 0)
             return linesize;
@@ -234,8 +262,8 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
             plane_checksum[plane] = av_adler32_update(plane_checksum[plane], data, linesize);
             checksum = av_adler32_update(checksum, data, linesize);
 
-            update_sample_stats(data, linesize, sum+plane, sum2+plane);
-            pixelcount[plane] += linesize;
+            update_sample_stats(bitdepth, be, data, linesize, sum+plane, sum2+plane);
+            pixelcount[plane] += width;
             data += frame->linesize[plane];
         }
     }