diff mbox

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

Message ID 20191128143441.12443-1-lance.lmwang@gmail.com
State Superseded
Headers show

Commit Message

Lance Wang Nov. 28, 2019, 2:34 p.m. UTC
From: Limin Wang <lance.lmwang@gmail.com>

The following is one of the testing results, you can observe the result of 16bit isn't correct.
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, yuv420p10
./ffmpeg -f lavfi  -i color=black:duration=1:r=1:size=1280x720,format=yuv420p10,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 | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

Comments

Michael Niedermayer Nov. 28, 2019, 10:51 p.m. UTC | #1
On Thu, Nov 28, 2019 at 10:34:41PM +0800, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> The following is one of the testing results, you can observe the result of 16bit isn't correct.
> 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, yuv420p10
> ./ffmpeg -f lavfi  -i color=black:duration=1:r=1:size=1280x720,format=yuv420p10,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]

with >8bit, both little and big endian formats should be tested

thx

[...]
Lance Wang Dec. 3, 2019, 2:25 p.m. UTC | #2
On Thu, Nov 28, 2019 at 11:51:13PM +0100, Michael Niedermayer wrote:
> On Thu, Nov 28, 2019 at 10:34:41PM +0800, lance.lmwang@gmail.com wrote:
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > The following is one of the testing results, you can observe the result of 16bit isn't correct.
> > 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, yuv420p10
> > ./ffmpeg -f lavfi  -i color=black:duration=1:r=1:size=1280x720,format=yuv420p10,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]
> 
> with >8bit, both little and big endian formats should be tested

Michael, I have updated the patch and test with le and be formats on be
and le system. Please review whether the test is complete.

> 
> thx
> 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker. User
> questions about the command line tools should be sent to the ffmpeg-user ML.
> And questions about how to use libav* should be sent to the libav-user ML.



> _______________________________________________
> 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".
diff mbox

Patch

diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
index 31f6b32..96f7c59 100644
--- a/libavfilter/vf_showinfo.c
+++ b/libavfilter/vf_showinfo.c
@@ -202,7 +202,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 +212,25 @@  static void update_sample_stats(const uint8_t *src, int len, int64_t *sum, int64
     }
 }
 
+static void update_sample_stats_16(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++) {
+        *sum += src1[i];
+        *sum2 += src1[i] * src1[i];
+    }
+}
+
+static void update_sample_stats(int depth, 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(src, len, sum, sum2);
+}
+
 static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
 {
     AVFilterContext *ctx = inlink->dst;
@@ -220,12 +239,14 @@  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 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 +255,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, data, linesize, sum+plane, sum2+plane);
+            pixelcount[plane] += width;
             data += frame->linesize[plane];
         }
     }