Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Dana Feng via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Cc: hassan hany <hassanhanyrashad@gmail.com>,
	Dana Feng <Dana.Feng@twosigma.com>
Subject: [FFmpeg-devel] Re: [PATCH] lavfi/mpdecimate: fix three bugs in keep_count logic
Date: Wed, 25 Feb 2026 15:12:19 +0000
Message-ID: <DM4PR08MB82450C32687189E3881C11BA8675A@DM4PR08MB8245.namprd08.prod.outlook.com> (raw)
In-Reply-To: <DM4PR08MB82452B5193904476A05DE36E8675A@DM4PR08MB8245.namprd08.prod.outlook.com>

Thanks! Is there a way to get it reviewed / merged? I opened one here: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/22261

-----Original Message-----
From: Dana Feng
Sent: Wednesday, February 25, 2026 10:01 AM
To: 'FFmpeg development discussions and patches' <ffmpeg-devel@ffmpeg.org>
Cc: hassan hany <hassanhanyrashad@gmail.com>
Subject: RE: [FFmpeg-devel] Re: [PATCH] lavfi/mpdecimate: fix three bugs in keep_count logic

Thanks! Is there a way to get it reviewed / merged? I opened one here: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/22261

-----Original Message-----
From: hassan hany via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>
Sent: Wednesday, February 18, 2026 5:13 PM
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Cc: hassan hany <hassanhanyrashad@gmail.com>
Subject: [FFmpeg-devel] Re: [PATCH] lavfi/mpdecimate: fix three bugs in keep_count logic

yes

On Thu, Feb 19, 2026 at 12:06 AM Dana Feng via ffmpeg-devel < ffmpeg-devel@ffmpeg.org> wrote:

> Ah got it, thank you! Do I just create a pull request?
>
> -----Original Message-----
> From: hassan hany via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>
> Sent: Wednesday, February 18, 2026 5:00 PM
> To: FFmpeg development discussions and patches
> <ffmpeg-devel@ffmpeg.org>
> Cc: hassan hany <hassanhanyrashad@gmail.com>
> Subject: [FFmpeg-devel] Re: [PATCH] lavfi/mpdecimate: fix three bugs
> in keep_count logic
>
> Hi dana
> you probably want to submit this to
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcode
> .ffmpeg.org%2F&data=05%7C02%7CDana.Feng%40twosigma.com%7Cdab688b157d34
> 74f196408de6f3afb73%7C43fb3c014eda479a96f34fb85967560f%7C1%7C0%7C63907
> 0496309163828%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIw
> LjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C
> %7C&sdata=VcYNpioY7KW7aHcMIbzFRbTt5aSicAlNZTGPogCaBXE%3D&reserved=0
> since thats where FFmpeg development happens nowadays (yes I am aware
> the readme says you should submit patches to the ML but its outdated)
>
> On Wed, Feb 18, 2026 at 11:55 PM Dana Feng via ffmpeg-devel <
> ffmpeg-devel@ffmpeg.org> wrote:
>
> > Hello! Just following up on this.
> >
> > From: Dana Feng
> > Sent: Friday, February 6, 2026 4:38 PM
> > To: 'ffmpeg-devel@ffmpeg.org' <ffmpeg-devel@ffmpeg.org>
> > Subject: [PATCH] lavfi/mpdecimate: fix three bugs in keep_count
> > logic
> >
> > Fix the following issues with the keep option:
> >
> > 1. No similarity check during keep period: The code returned early
> >    during the keep period without checking if the frame was actually
> >    similar to the reference.
> >
> > 2. keep_count doesn't reset on different frames: The counter should
> >    track "consecutive similar frames," but when a different frame
> >    arrived mid-count, keep_count didn't reset to 0. It only reset
> >    after going negative (after a drop), causing the counter to
> >    accumulate across non-consecutive similar frames and drop frames
> >    earlier than expected.
> >
> > 3. Reference frame drift: When similar frames were kept due to
> >    keep_count, they became the new reference. Over time, tiny
> >    differences (sensor noise, compression artifacts) accumulated.
> >    Frame N got compared to frame N-1 instead of the last different
> >    frame, allowing gradual scene changes to slip through undetected.
> >
> > Now decimate_frame() checks similarity first, then applies
> > keep_count logic, returning distinct values:
> >    1 = drop (similar, over keep threshold)
> >    0 = keep (different frame) - updates reference, resets keep_count
> >   -1 = keep (similar, under keep threshold) - preserves reference
> >
> > Signed-off-by: Dana Feng
> > danaf@twosigma.com<mailto:danaf@twosigma.com>
> > ---
> > libavfilter/vf_mpdecimate.c | 49
> > +++++++++++++++++++++++++------------
> > 1 file changed, 34 insertions(+), 15 deletions(-)
> >
> > diff --git a/libavfilter/vf_mpdecimate.c
> > b/libavfilter/vf_mpdecimate.c index 0fc95556b7..5b1654f671 100644
> > --- a/libavfilter/vf_mpdecimate.c
> > +++ b/libavfilter/vf_mpdecimate.c
> > @@ -109,19 +109,18 @@ static int diff_planes(AVFilterContext *ctx,
> > /**
> >   * Tell if the frame should be decimated, for example if it is no much
> >   * different with respect to the reference frame ref.
> > + *
> > + * @return 1 if frame should be dropped (similar, over keep threshold),
> > + *         0 if frame should be kept (different),
> > + *        -1 if frame should be kept (similar, under keep threshold)
> >   */
> > static int decimate_frame(AVFilterContext *ctx,
> >                            AVFrame *cur, AVFrame *ref) {
> >      DecimateContext *decimate = ctx->priv;
> > +    int is_similar;
> >      int plane;
> >
> > -    if (decimate->max_keep_count > 0 &&
> > -        decimate->keep_count > -1 &&
> > -        decimate->keep_count < decimate->max_keep_count) {
> > -        decimate->keep_count++;
> > -        return 0;
> > -    }
> >      if (decimate->max_drop_count > 0 &&
> >          decimate->drop_count >= decimate->max_drop_count)
> >          return 0;
> > @@ -129,6 +128,7 @@ static int decimate_frame(AVFilterContext *ctx,
> >          (decimate->drop_count-1) > decimate->max_drop_count)
> >          return 0;
> >
> > +    is_similar = 1;
> >      for (plane = 0; ref->data[plane] && ref->linesize[plane]; plane++) {
> >          /* use 8x8 SAD even on subsampled planes.  The blocks won't
> > match up with
> >           * luma blocks, but hopefully nobody is depending on this
> > to catch @@ -141,8 +141,21 @@ static int
> > decimate_frame(AVFilterContext
> *ctx,
> >                          cur->data[plane], cur->linesize[plane],
> >                          ref->data[plane], ref->linesize[plane],
> >                          AV_CEIL_RSHIFT(ref->width,  hsub),
> > -                        AV_CEIL_RSHIFT(ref->height, vsub)))
> > -            return 0;
> > +                        AV_CEIL_RSHIFT(ref->height, vsub))) {
> > +            is_similar = 0;
> > +            break;
> > +        }
> > +    }
> > +
> > +    if (!is_similar)
> > +        return 0;  /* Frame is different - keep it */
> > +
> > +    /* Frame is similar - apply keep_count logic */
> > +    if (decimate->max_keep_count > 0 &&
> > +        decimate->keep_count > -1 &&
> > +        decimate->keep_count < decimate->max_keep_count) {
> > +        decimate->keep_count++;
> > +        return -1;  /* Similar but under keep threshold - keep
> > + without
> > updating ref */
> >      }
> >
> >      return 1;
> > @@ -200,29 +213,35 @@ static int filter_frame(AVFilterLink *inlink,
> > AVFrame *cur)
> >      DecimateContext *decimate = inlink->dst->priv;
> >      AVFilterLink *outlink = inlink->dst->outputs[0];
> >      int ret;
> > +    int result = decimate->ref ? decimate_frame(inlink->dst, cur,
> > decimate->ref) : 0;
> >
> > -    if (decimate->ref && decimate_frame(inlink->dst, cur,
> decimate->ref))
> > {
> > +    if (result == 1) {
> > +        /* Drop: similar frame, over keep threshold */
> >          decimate->drop_count = FFMAX(1, decimate->drop_count+1);
> > -        decimate->keep_count = -1; // do not keep any more frames until
> > non-similar frames are detected
> > +        decimate->keep_count = -1;
> > +    } else if (result == -1) {
> > +        /* Keep: similar frame, under keep threshold - don't update
> > + ref
> */
> > +        decimate->drop_count = FFMIN(-1, decimate->drop_count-1);
> > +        if ((ret = ff_filter_frame(outlink, cur)) < 0)
> > +            return ret;
> >      } else {
> > +        /* Keep: different frame - update ref and reset keep_count
> > + */
> >          av_frame_free(&decimate->ref);
> >          decimate->ref = cur;
> >          decimate->drop_count = FFMIN(-1, decimate->drop_count-1);
> > -        if (decimate->keep_count < 0) // re-enable counting similar
> > frames to ignore before dropping
> > -            decimate->keep_count = 0;
> > -
> > +        decimate->keep_count = 0;
> >          if ((ret = ff_filter_frame(outlink, av_frame_clone(cur))) < 0)
> >              return ret;
> >      }
> >
> >      av_log(inlink->dst, AV_LOG_DEBUG,
> >             "%s pts:%s pts_time:%s drop_count:%d keep_count:%d\n",
> > -           decimate->drop_count > 0 ? "drop" : "keep",
> > +           result > 0 ? "drop" : "keep",
> >             av_ts2str(cur->pts), av_ts2timestr(cur->pts,
> > &inlink->time_base),
> >             decimate->drop_count,
> >             decimate->keep_count);
> >
> > -    if (decimate->drop_count > 0)
> > +    if (result > 0)
> >          av_frame_free(&cur);
> >
> >      return 0;
> > --
> > 2.39.5
> > _______________________________________________
> > ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org To unsubscribe
> > send an email to ffmpeg-devel-leave@ffmpeg.org
> >
> _______________________________________________
> ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org To unsubscribe
> send an email to ffmpeg-devel-leave@ffmpeg.org
> _______________________________________________
> ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org To unsubscribe
> send an email to ffmpeg-devel-leave@ffmpeg.org
>
_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org
_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

  reply	other threads:[~2026-02-25 15:12 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-06 21:37 [FFmpeg-devel] " Dana Feng via ffmpeg-devel
2026-02-18 21:54 ` [FFmpeg-devel] " Dana Feng via ffmpeg-devel
2026-02-18 22:00   ` hassan hany via ffmpeg-devel
2026-02-18 22:05     ` Dana Feng via ffmpeg-devel
2026-02-18 22:12       ` hassan hany via ffmpeg-devel
2026-02-25 15:00         ` Dana Feng via ffmpeg-devel
2026-02-25 15:12           ` Dana Feng via ffmpeg-devel [this message]
2026-02-26  9:40             ` Nicolas George via ffmpeg-devel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM4PR08MB82450C32687189E3881C11BA8675A@DM4PR08MB8245.namprd08.prod.outlook.com \
    --to=ffmpeg-devel@ffmpeg.org \
    --cc=Dana.Feng@twosigma.com \
    --cc=hassanhanyrashad@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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