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

[Andriy Gelman]

On Mon, 06. Jul 16:02, Andriy Gelman wrote:
> 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.

Thanks, applied.

-- 
Andriy