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

[Marton Balint]

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.

> 
> -    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.

> +        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].

> +    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.

Regards,
Marton