[FFmpeg-devel] [PATCH] lavf/vf_freezedetect: improve for the freeze frame detection

Limin Wang lance.lmwang at gmail.com
Sun Jul 14 01:42:24 EEST 2019


On Sat, Jul 13, 2019 at 07:24:59PM +0200, Marton Balint wrote:
> 
> 
> On Sat, 13 Jul 2019, lance.lmwang at gmail.com wrote:
> 
> >From: Limin Wang <lance.lmwang at gmail.com>
> >
> >I have samples failed to detect the freeze frame with the default -60dB
> >noise(-40dB is OK to detect),
> >after apply the patch, it's ok to detect.
> >
> >I run the testing with fate-suite sample for your testing:
> >old: no freeze frame detect.
> >
> >with the patch:
> >./ffmpeg -i fate-suite/svq3/Vertical400kbit.sorenson3.mov -vf
> >freezedetect=n=-60dB -an -f null -
> >[freezedetect @ 0x7fe18a604900] lavfi.freezedetect.freeze_start: 38.7667
> >[freezedetect @ 0x7fe18a604900] lavfi.freezedetect.freeze_duration: 2.5
> >[freezedetect @ 0x7fe18a604900] lavfi.freezedetect.freeze_end: 41.2667
> >[freezedetect @ 0x7fe18a604900] lavfi.freezedetect.freeze_start: 41.2667
> >
> >Have run make fate testing although haven't find any freezedetect checking for
> >the fate.
> 
> You are changing the way the score is calculated and not describing
> why. Of course you will get different results.
> 
> >
> >Signed-off-by: Limin Wang <lance.lmwang at gmail.com>
> >---
> >libavfilter/vf_freezedetect.c | 22 ++++++++++++++++------
> >1 file changed, 16 insertions(+), 6 deletions(-)
> >
> >diff --git a/libavfilter/vf_freezedetect.c b/libavfilter/vf_freezedetect.c
> >index cc086af..0288fb0 100644
> >--- a/libavfilter/vf_freezedetect.c
> >+++ b/libavfilter/vf_freezedetect.c
> >@@ -38,7 +38,9 @@ typedef struct FreezeDetectContext {
> >    ptrdiff_t height[4];
> >    ff_scene_sad_fn sad;
> >    int bitdepth;
> >+    int nb_planes;
> >    AVFrame *reference_frame;
> >+    double prev_mafd;
> >    int64_t n;
> >    int64_t reference_n;
> >    int frozen;
> >@@ -102,13 +104,15 @@ static int config_input(AVFilterLink *inlink)
> >    AVFilterContext *ctx = inlink->dst;
> >    FreezeDetectContext *s = ctx->priv;
> >    const AVPixFmtDescriptor *pix_desc = av_pix_fmt_desc_get(inlink->format);
> >+    int vsub = pix_desc->log2_chroma_h;
> >
> >    s->bitdepth = pix_desc->comp[0].depth;
> >+    s->nb_planes = av_pix_fmt_count_planes(inlink->format);
> 
> This is not needed, for invalid planes we simply get 0 for linesize
> and width.

It's more general to get nb_planes instead of checking the linesize
result. If have one plane, it's unneed to check 4 times.

> 
> >
> >-    for (int plane = 0; plane < 4; plane++) {
> >+    for (int plane = 0; plane < s->nb_planes; plane++) {
> >        ptrdiff_t line_size = av_image_get_linesize(inlink->format, inlink->w, plane);
> >-        s->width[plane] = line_size >> (s->bitdepth > 8);
> >-        s->height[plane] = inlink->h >> ((plane == 1 || plane == 2) ? pix_desc->log2_chroma_h : 0);
> >+        s->width[plane] = line_size;
> 
> Why? Width is the width of the plane in pixels, simply setting
> linesize does not seem right.

the sad init function have set the bitdepth, so I think it's OK to set
the width in bytes. Maybe it's my misunderstand for the code.

> 
> >+        s->height[plane] = plane == 1 || plane == 2 ? AV_CEIL_RSHIFT(inlink->h, vsub) : inlink->h;
> 
> Mix of a cosmetic and functional change. This should do the same
> with existing code and variables:
> s->height[plane] = AV_CEIL_RSHIFT(inlink->h, ((plane == 1 || plane == 2) ? pix_desc->log2_chroma_h : 0)
> 
> >    }
> >
> >    s->sad = ff_scene_sad_get_fn(s->bitdepth == 8 ? 8 : 16);
> >@@ -129,7 +133,9 @@ static int is_frozen(FreezeDetectContext *s, AVFrame *reference, AVFrame *frame)
> >    uint64_t sad = 0;
> >    uint64_t count = 0;
> >    double mafd;
> >-    for (int plane = 0; plane < 4; plane++) {
> >+    double diff, cmp;
> >+
> >+    for (int plane = 0; plane < s->nb_planes; plane++) {
> 
> Unneeded.
> 
> >        if (s->width[plane]) {
> >            uint64_t plane_sad;
> >            s->sad(frame->data[plane], frame->linesize[plane],
> >@@ -140,8 +146,12 @@ static int is_frozen(FreezeDetectContext *s, AVFrame *reference, AVFrame *frame)
> >        }
> >    }
> >    emms_c();
> >-    mafd = (double)sad / count / (1ULL << s->bitdepth);
> >-    return (mafd <= s->noise);
> >+    mafd = (double)sad /(count >> (s->bitdepth > 8));
> 
> Why? MAFD should be the mean difference normalized to [0..1].
if the bitdeth is 16, it'll divide by 1UL << 16, it'll cause the mafd is
very small. So I choose the scenecut way in the below. 

> 
> >+    diff = fabs(mafd - s->prev_mafd);
> >+    cmp  = av_clipf(FFMIN(mafd, diff) / 100., 0, 1);
> >+    s->prev_mafd = mafd;
> 
> Why? This is not scene change detection, MAFD change is not useful
> for us. E.g. two frames alternating will be detected as frozen. Also
> note that not always consecutive frames are compared, so involving
> MAFD change seems even more bogus to me.
Isn't possible to add one standard sampel into fate, it's useful to
test the function. We can choose different noise setting to get the same
result, so it's difficult to say whether it's covered.


> 
> Regards,
> Marton
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".


More information about the ffmpeg-devel mailing list