:: Re: [Frei0r] api documentation
Etusivu
Poista viesti
Vastaa
Lähettäjä: Dan Dennedy
Päiväys:  
Vastaanottaja: Minimalistic plugin API for video effects
Aihe: Re: [Frei0r] api documentation
On Sun, Apr 3, 2016 at 1:54 PM Jaromil <jaromil@???> wrote:

>
> well,
>
> [TL;DR] no problem, but lets document the change!
>
>
> gentlemen, lets please refrain from unnecessary and self-defeating
> personal attacks.



I am sorry for the veiled personal attacks. The messages triggered
something bad in me last night.


> There is also no need to be surprised of this change, which is a fix.
>
>

Also, it has been in git since July, 2015. But, perhaps for a low activity
mailing list, it might not be a bad idea to reflect commit messages to the
list?


> but we need to document the reason in the changelog of API changes,
> even if this only affects C++ implementations.
>
>

The C API, which is the only public interface for frei0r plugins, is
unaffected; and the Doxygen docs only documents that API.
I assume you are referring to this section, specifically:
https://frei0r.dyne.org/codedoc/html/index.html#sec_changes
Adding information about the non-public C++ API to frei0r.h and this
section only leads to confusion IMO.
However, I do see the README needs update for this.


> Dan: can you provide such information? it can be concise.
> Else I'll write it up.
>
> BTW am I the only one affected by this change? indeed I'm not sure how
> many hosts are using the C++ besides FreeJ, which I should anyway
> update for many more reasons and API changes, so really I see no
> problem for my own work.
>


it does not matter if the host is using C++, it is still only using the
public C API. I fail to see how FreeJ is affected.
Here it gets the C function:
https://github.com/jaromil/FreeJ/blob/master/src/frei0r.cpp#L217
and here it uses it:
https://github.com/jaromil/FreeJ/blob/master/src/frei0r.cpp#L301
Finally, here is the C API exposed by frei0r.hpp, which does not deviate
from the API spec.:
https://github.com/ddennedy/frei0r/blob/master/include/frei0r.hpp#L294

On Sun, Apr 3, 2016 at 1:53 AM salsaman <salsaman@???> wrote:

> You are entitled to your opinion, however you single handledly made a
> change to every plugin, and yet did not mention a word about it on the
> mailing list. It would have been nice to have discussed it first. If it
> fixes an issue then perhaps there would have been alternate / better ways
> to do it. Even if not it would have been a courtesy to mention it.
>


I agree. In the future I will submit big changes to the mailing for review
before commit. I ask for their to be a 10 day expiration where no response
means OK, and I will send a reminder e-mail after one week if there was no
response.


>
>
>
>> And no plugins were converted to mixer3 in the process. Maybe you need to
>> brush up on your C++.
>>
>>
> I noticed that you added 3 frame inputs to each plugin, even those which
> are simple filter effects. I fail to see the reason for this.
>


To each C++ plugin, yes. The old frei0r.hpp C++ wrapper would store the
image buffer pointers on the object and access them through those members.
If there is more than one thread calling the same f0r_update function the
C++ wrapper classes will change the image pointers of the first thread to
that of the second thread. The only value of this limiting statefulness was
to give C++ plugins a cuter, simpler update() method signature for their
implementation. After the change, the image pointers are moved from the
instance and onto the stack, and each thread's f0r_update() is using
thread-local pointers and not overwriting each others' buffers.

All that has really changed is the C++ plugins can no longer use the *cute*
update() method signature in their implementation and must instead use the
more verbose "update(double time, uint32_t* out, const uint32_t* in, const
uint32_t* in2, const uint32_t* in3). If a plugin does not need all those
input parameters (because of the type declared in f0r_plugin_info), it
should ignore them. It may be possible to make a C++ wrapper where each
plugin receives only the number of params corresponding to its plugin_type.
I am preparing a patch that will remove the extra input buffer parameters
from each C++ update() method, but it does add the overhead of an
additional method call plus some virtual function lookups within the C++
wrapper.