[FFmpeg-devel] [PATCH] delogo filter: new "uglarm" interpolation mode added

[Uwe Freese]

Hello,

here's a new version of the patch.

Changes since the last version of the patch (mail from 27.12. 13:00), 
according the comments from Moritz and Carl Eugen:


- using av_assert0 instead of throwing an error if planes > MAX_PLANES
- using av_freep with the reference operator in uninit
- copyright year 2019 -> 2018
- add MODE_NB value in "mode" enum and limit mode parameter in 
delogo_options to MODE_NB-1
- changed commit message style: "avfilter/vf_delogo: add uglarm 
interpolation mode"
- bracket style, line 360
- changed documentation, using @var{...}, @code{...}, @table @option 
etc., explaining "uglarm" and using the term "raised to the power of" 
(see https://en.wikipedia.org/wiki/Exponentiation). I hope the use of 
the macros is correct as I used them... (@var referencing a parameter 
and @code referencing a value)
- changed parameter "power" (int) to "exponent" (float)


Some considerations about the parameter name "power" / "exponent":

I noticed that for the specific calculation of the weight in the new 
mode, the wording should be better named "exponent" instead of "power".
Wikipedia (https://en.wikipedia.org/wiki/Exponentiation) writes:

base ^ exponent = power

So I changed the value type to float and the calculation to

weight = distance ^ exponent
(float value exponent).

That is IMHO more consistent and easier to explain.

The downside is that the term "power" (or "strength") would be more 
generic and would probably be better reusable for other modes.

What do you think? Is "exponent" now ok, or should I change it back to 
"power", "strength" or something alike?


Again, I would be glad about comments and reviews. - Let me know what 
should be changed and I'll create a new patch in some days.

Regards,
Uwe


Am 28.12.18 um 00:08 schrieb Carl Eugen Hoyos:
> 2018-12-27 22:02 GMT+01:00, Uwe Freese <uwe.freese at gmx.de>:
>
>> Am 27.12.18 um 20:25 schrieb Carl Eugen Hoyos:
>>>> I have now added as error handling:
>>>>
>>>> av_log(inlink->src, AV_LOG_ERROR, "More planes in frame than
>>>> expected.\n");
>>>> return AVERROR(ENOMEM);
>>>>
>>>> Is this ok, or how should this be implemented instead?
>>> Not sure I understand: How can plane get >= MAX_PLANES?
>>> If this is impossible (as I believe), please use av_assert0().
>> I meant the use of "ENOMEM" and if there's a better error
>> constant to use here.
>>
>> At this line, the error is not about memory, but that the video input
>> format is unexpected.
> Is there a format with too many planes?
> If not, please use an assert, do not return an error.
>
> Carl Eugen
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-avfilter-vf_delogo-add-uglarm-interpolation-mode.patch
Type: text/x-patch
Size: 14342 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20181229/8fd4ba42/attachment.bin>