[FFmpeg-devel] [PATCH] libavfilter: temporarily remove DNN framework and vf_sr filter

[Pedro Arthur]

2018-07-27 12:07 GMT-03:00 Rostislav Pehlivanov <atomnuker at gmail.com>:
> On 27 July 2018 at 15:11, James Almer <jamrial at gmail.com> wrote:
>
>> On 7/27/2018 8:04 AM, Rostislav Pehlivanov wrote:
>> > Reference - IRC and j-b's earlier email.
>> > Coding style issues:
>> > DNNModel* ff_dnn_load_model_native(const char* model_filename)
>> >
>> > We never ever do stupid things like put the asterix first. The author of
>> > the patch should have known better and the patch should have been
>> checked.
>> > Even a glance could have told you its wrong.
>>
>> Tone it down. It's a style issue. New contributors don't always know
>> things like that and we always tell them to fix it. It's the reviewer
>> who should have pointed it out, and if they missed it then it's
>> harmless. It's not like he used a public suffix like av_, where it can
>> be a problem.
>>
>
> It shows the code wasn't reviewed properly. These style issues are
> propagated throughout all the DNN framework, which was many thousands of
> lines of code and shows that not even a glimpse was spent on the actual
> code. We don't commit such huge patches without at least some form of
> review, even if months have passed and no one has bothered to yet.
The style issue is my fault, I did not ensure the student were aware
of the coding style.
That does not mean that other aspects of the code were not properly
reviewed, I spent a good amount of time reviewing and testing the
code.
The code style issue is easily fixable, the student will already
provide a fix, but if it is too much urgent I could fix it myself in a
few minutes.
I don't see how reverting the code and asking for "proper" review,
which no one (in almost 3 months) besides me did, will do any good.
After reverting will you be compromised (or anyone else) to review it?
or the code will get forgotten waiting for review until no one cares
about it?

It is not fair to propose a patch, and push it within 24h, reverting
the student work of almost 3 months where no one has complained before
of issues to be fixed.

The code style issue will be addressed.
We already provide a repo with training scripts, and it will be
referenced in the code.

Objectively, what else needs to be fixed? let me know and give the
student at least a few days so he can provide a fix.

>
>
>
>>
>> > I described them in the sentence above.
>> > But I'm not willing to wait for a potential fix, and especially not for a
>> > whole bunch of them rewriting everything. The whole code needs to be
>> thrown
>> > out and thoroughly reviewed properly, by at least yourself and one other
>> > person, preferably before gsoc ends.
>> > You should start coordinating with your student on how to fix everything
>> > mentioned and then resend the patchsets once fixed. I'll apply the revert
>> > patch tomorrow.
>>
>> No, you wont. Not until this has been discussed.
>
>
> Yes I will, unless someone objects I still intend to. No one has yet.
>
>
> I don't know what got
>> to you but you're acting like someone pushed code that would get a cop
>> on your doorstep. Calm down for once in your life, you're seemingly
>> angry in every other email you write, and you're not helping making this
>> project a friendly place at all for new and old contributors alike.
>>
>
> I didn't intend to sound such but nevertheless the facts I mentioned
> remain, and I can't really think of a way to present them better.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel