[FFmpeg-devel] [PATCH] avfilter: add radial and circular blur video filter

[Steinar H. Gunderson]

On Sat, Jul 18, 2020 at 01:31:49PM +0200, Paul B Mahol wrote:
>> In short, this patch has:
>>
>>  - An unusually slow algorithm (sin, cos, atan, division; all per-pixel).
> This is actually good.

Is it good that it's slow? Or do you mean that the algorithm isn't actually
slow?

> Performance is not affected at all.

It sure is. With -vf vblur, a simple single-threaded transcode (to magicyuv)
drops from 64 to 5 fps. A quick perf run indicates that about 67% of the CPU
time is spent in converting to and from polar coordinates (p2c, c2p, atan2),
give or take a little.

>>  - Poor output quality (strong aliasing, no subpixel precision).
> I fixed this. No thanks to your patch.

It's good that you fixed the quality issue you claimed didn't exist less than
an hour earlier. Could you please post the updated patch?

>>  - No comments, hardly any commit message.
> No comments are needed for developers that know how to code.

The FFmpeg coding style disagrees:

https://ffmpeg.org/developer.html#Comments -- “All nontrivial functions
should have a comment above them explaining what the function does, even if
it is just one sentence. All structures and their member variables should be
documented, too.”

https://ffmpeg.org/developer.html#Patches_002fCommitting -- “Always fill out
the commit log message. Describe in a few lines what you changed and why.”

I don't see any exceptions for “developers that know how to code”; and after
all, also those who are not so good coders should be able to understand the
code.

>> Thus, I believe it should not be merged.
> It is just your belief, no any proof given.

I believe I fairly clearly spelled out my reasoning in the text you quoted?

/* Steinar */
-- 
Homepage: https://www.sesse.net/