[FFmpeg-devel] [PATCH v2] avfilter/vf_edgedetect: properly implement double_threshold()

[Andriy Gelman]

Hi Alex,
Thanks for testing.

On Mon, 6 Jul 2020 at 14:49, Alexander Strasser <eclipse7 at gmx.net> wrote:

> On 2020-07-04 23:19 -0400, Andriy Gelman wrote:
> > On Sun, 05. Jul 09:37, lance.lmwang at gmail.com wrote:
> > > On Sat, Jul 04, 2020 at 01:03:48PM -0400, Andriy Gelman wrote:
> > > > On Mon, 29. Jun 09:26, Valery Kot wrote:
> > > > > On Sun, Jun 28, 2020 at 12:03 AM Andriy Gelman <
> andriy.gelman at gmail.com> wrote:
> > > > > >
> > > > > > lgtm. I saw a small improvement when testing.
> > > > > >
> > > > > > Would be nice to allow weak edges that become strong to trigger
> neighboring weak
> > > > > > edges in the future.
> > > > >
> > > > > Thanks for the comment.
> > > > >
> > > > > You are right, ideally double_threshold should be recursive (seed
> from
> > > > > each "high" peak and spread over "low" peaks). But then
> potentially we
> > > > > may end up with width*height/2 recursion depth, which may lead to
> > > > > stack overflow. So probably some recursion limit is needed, and
> hence
> > > > > suboptimal solution.
> > > > >
> > > > > Or iterative approach, running through the complete image again and
> > > > > again checking if "low" peaks are touching already selected edge
> > > > > pixels. Stop when no new pixels can be added to the edge. Better,
> but
> > > > > still potentially width*height/2 iterations with width*height
> > > > > operations each, completely killing performance.
> > > > >
> > > > > Maybe later I'll try to implement it in a generic way, but this is
> out
> > > > > of scope for this patch.
> > > > >
> > > > > Regards,
> > > > >
> > > > > Valery Kot
> > > >
> > > > Any objections if I apply this patch?
> >
> > >
> > > The patch look fine to me, but no sign-off?
> > >
> >
> > Thanks, will add one.
>
> I guess it's OK to push. In my few tests for some cases it looked
> a bit better.
>
> I didn't understand why this doesn't make things worse for the
> outermost frame of pixels, but Valery commented it wouldn't make
> any difference because of the surrounding code.
>
> I just looked at that function and because it checks all the eight
> surrounding pixels, I thought we might now miss cases that wouldn't
> have been missed before. Should of course be usually be better for
> most pictures.


The point of the first check is to skip pixels on the boundary so that the
surrounding pixels check does not seg fault.
It was only by luck that src[i] is always zero on the boundary so that the
surrounding pixels check is not evaluated.

You could treat pixels on the boundary separately, but I don't think it's
worth it.

Andriy