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

[Paul B Mahol]

On 7/18/20, Steinar H. Gunderson <steinar+ffmpeg at gunderson.no> wrote:
> 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?
>

It is pretty fast here.


>> 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.

It can be made faster, but why i would do that when you complain on
current code?

Actually, fact is that you came here at first to bring some jumbo
about better algorithms
with pages that had almost no valuable information.
And later when proven wrong you started to object to current patch
just for pure amusement.

>
>>>  - 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?

I will keep it to my fork only. You can enjoy your negative reviews
later at any time.

>
>>>  - 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.

Code is self explanatory and does not need comments for mortals.

>
>>> 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?

The reasoning is completely flawed as explained multiple times.

>
> /* Steinar */
> --
> Homepage: https://www.sesse.net/
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".