[FFmpeg-devel] [PATCH] lavfi: add dctdnoiz filter

[Clément Bœsch]

On Sat, May 04, 2013 at 04:25:48PM -0400, Derek Buitenhuis wrote:
> On 2013-05-04 3:48 PM, Clément Bœsch wrote:
> > + at section dctdnoiz
> 
> Perhaps some more explanation as to what it actually is. I kind of doubt
> most users who want to denoise have enough in-depth knowledge to know
> what a 'dct denoiser' is (though many seem to understand to understand
> what frequency-domain filtering is, weirdly...)
> 

Do you have any suggestion? I'm not sure what to add. I've added a
parenthesis but I guess you're expecting more...

> Personally, I'm interested in seeing some before/after examples not from
> the paper, since they seem kinda 'fake' to me. ;)
> 

Do you have any sample to propose?

Otherwise, feel free to try it out:

  git remote add ux git://github.com/ubitux/FFmpeg.git
  git fetch ux
  git checkout ux/dctdnoiz

  make config ... make -j ...

  ./ffplay denoiseme.png -vf "split[a][b]; [a]pad=iw*2[p]; [b]dctdnoiz=5[f]; [p][f]overlay=w"

  <go make yourself some coffee while it's processing>

> > +#define OFFSET(x) offsetof(DCTdnoizContext, x)
> 
> This still has to be done on a per-filter basis?
> 

What do you propose?

> > *line *= x == 0 ? 1. / sqrt(BSIZE) : sqrt(2. / BSIZE);
> 
> This is uh, fairly unreadable at best. Same for future
> incantations.
> 

Well, it's just a normalization, which is different for the DC. Might not
be the optimal way to do so though.

> > +    if (s->pr_width != inlink->w)
> > +        av_log(ctx, AV_LOG_WARNING, "The last %d horizontal pixels won't be denoised\n",
> > +               inlink->w - s->pr_width);
> > +    if (s->pr_height != inlink->h)
> > +        av_log(ctx, AV_LOG_WARNING, "The last %d vertical pixels won't be denoised\n",
> > +               inlink->h - s->pr_height);
> 
> Is it not feasible to extend the buffer (with black) to support this, or
> will that break the denoising entirely?
> 

It might require some more data copy. If the user wants to pad with black,
it can use a pad filter, which will likely make things the most optimal
possible (using linesize if possible). I could provide in the filter some
pad or crop parameters eventually though.

> > +        s->cbuf[i][0] = av_malloc(linesize * s->pr_height * sizeof(*s->cbuf[i][0]));
> > +        s->cbuf[i][1] = av_malloc(linesize * s->pr_height * sizeof(*s->cbuf[i][1]));
> > +        s->cbuf[i][2] = av_malloc(linesize * s->pr_height * sizeof(*s->cbuf[i][2]));
> > +        if (!s->cbuf[i][0] || !s->cbuf[i][1] || !s->cbuf[i][2])
> > +            return AVERROR(ENOMEM);
> 
> Won't this leak memory? Same for other such multiple-malloc checks.
> 

I'm hoping for uninit() to be called in this case.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130505/ea76f7d6/attachment.asc>