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

[Rostislav Pehlivanov]

On 27 July 2018 at 03:04, Pedro Arthur <bygrandao at gmail.com> wrote:

> Hi,
>
> I'm surprised with this patch, there wasn't any concern raised in the
> patch review process.
>

It slipped through, and we (and you) should have known better.



> 2018-07-26 16:26 GMT-03:00 Rostislav Pehlivanov <atomnuker at gmail.com>:
> > As discussed recently, the vf_sr filter and the DNN framework have an
> > issue: unreproducable weights and questionable license, as well as
> > overall unfitting coding style to the rest of the project.
> I think I'm not aware of these discussions could you provide a
> reference? I also don't understand why the coding style is unfitting
> (again no concern was raised).
>

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.


>
> > The vf_sr filter in particular has weights embedded which weight the
> > libavfilter binary by a bit and cannot currently be reproduced.
> > There's an overall consensus that NN filters should accept external
> > weights only, as the nnedi filter currently does.
> >
> > So, temporarily remove both until the coding style issues have been
> > fixed with the framework and the filter has been modified to accept
> > external weights.
> What are these issues so we can fix them?
>

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.


>
> > Also, there's a discussion by the Debian folks as to whether to treat
> > pretrained NNs as non-free[0], hence its not just our project that's
> > affected by the questionable license of distributing pretrained NN
> > weights.
> >
> > Due to the weight of the patch (more than 1mb!) I've uploaded it to
> > https://0x0.st/sVEH.patch if anyone wants to test it. The change stat
> > is printed below.
> >
> > [0]: https://lwn.net/Articles/760142/
> I took the time to read the whole discussion and in my opinion it is
> flawed, except for the storage requirement, which Sergey already
> worked on a patch to reduce the stored data.
>
>
> I think before any discussion, it should be clear what is the ffmpeg
> policy on adding data, and what is considered data, and it should be
> consistent.
>
> I'll try to address the topics in the above discussion.
>
> First what is data? is it expected that all data stored should be
> easily reproducible?
>

Easily? No. Always? Absolutely.


I guess not, what is the point in storing data that is easily reproducible?
>

We do it for speed reasons. Large tables take time to generate on startup.


The entire humanity is built on previous stored knowledge, namely
> data, do we require each time one is going to use some form of
> knowledge to reproduce it? that is, proof everything one is using?
> The answer is no, the whole point in storing data is that you had once
> worked hard to proof it works and onwards just use it and believe it
> works. That does not mean it is imposible to proof everything.
>
> I think the above fits perfectly with NN weights as data.
>
> The next point is the reproducibility, one should be reasonable, it is
> hard to reproduce bit by bit of NN stored data but is totally doable
> to achieve similar results following the same training metodology
> used.
>

Very well, provide code and references to sources to reproduce please.



> Then there is the question what is open-source, once again one should
> be reasonable.
> The NN model is public available, everything is documented, the math
> machinery is also widely available and documented.
> There is also a repository containing everything one needs to train
> the NN and achieve similar results, the trainig data is public also.
> The training software is open-source, the user could also use any
> machine learning framework of their choice to perform the training
> since the model is documented, there is nothing locking one to a
> specific software or hardware.
> I can't see what is not open.
>

The process to replicate the weights. Provide script(s) and references to
sources.


Does we impose all requiriments imposed for NN weights on all other
> data stored in ffmpeg?
> I guess not, once more one should be consistent.
>
>
> If you compare NN weights with quantization tables they are pretty
> similar, both can be obtained from a training process over a dataset
> so it achieves better results (quality/compression). Are quantization
> tables evil? I don't think so.
> It seems people is afraid of  NN just because we give it a fancy name,
> while it is just tables of data as we always used in our code.
>

Doesn't matter, unless there's a way to reproduce them they're still
unscientific. Even codecs without actual generation functions for
quantization tables (e.g. Daala) still always provide ways to reproduce
them (the tools directory has scripts which if ran over a certain specified
set of image values will always output the same tables).
This requirement can be dropped if the weights are in a separate file,
though I still request that a script to regenerate them is included in our
tools directory. We could provide some pretrained weights as files in a
separate repo, similar to what we do for the cuda header.