[FFmpeg-devel] [GSOC] [PATCH] DNN module introduction and SRCNN filter update

[James Almer]

On 5/25/2018 10:01 AM, Sergey Lavrushkin wrote:
> 2018-05-24 22:52 GMT+03:00 James Almer <jamrial at gmail.com
> <mailto:jamrial at gmail.com>>:
> 
>     On 5/24/2018 4:24 PM, Sergey Lavrushkin wrote:
>     > Hello,
>     >
>     > This patch introduces DNN inference interface and simple native
>     backend.
>     > For now implemented backend supports only convolutions with relu
>     activation
>     > function, that are sufficient for simple convolutional networks,
>     > particularly SRCNN.
>     > SRCNN filter was updated using implemented DNN inference interface and
>     > native backend.
>     >
>     >
>     > adds_dnn_srcnn.patch
>     >
>     >
>     > From 60247d3deca3c822da0ef8d7390cda08db958830 Mon Sep 17 00:00:00 2001
>     > From: Sergey Lavrushkin <dualfal at gmail.com <mailto:dualfal at gmail.com>>
>     > Date: Thu, 24 May 2018 22:05:54 +0300
>     > Subject: [PATCH] Adds dnn inference module for simple
>     convolutional networks.
>     >  Reimplements srcnn filter based on it.
>     >
>     > ---
>     >  Changelog                      |   2 +
>     >  libavfilter/vf_srcnn.c         | 300 +++------------
>     >  libavfilter/vf_srcnn.h         | 855
>     -----------------------------------------
>     >  libavutil/Makefile             |   3 +
>     >  libavutil/dnn_backend_native.c | 382 ++++++++++++++++++
>     >  libavutil/dnn_backend_native.h |  40 ++
>     >  libavutil/dnn_interface.c      |  48 +++
>     >  libavutil/dnn_interface.h      |  64 +++
>     >  libavutil/dnn_srcnn.h          | 854
>     ++++++++++++++++++++++++++++++++++++++++
>     >  9 files changed, 1455 insertions(+), 1093 deletions(-)
>     >  delete mode 100644 libavfilter/vf_srcnn.h
>     >  create mode 100644 libavutil/dnn_backend_native.c
>     >  create mode 100644 libavutil/dnn_backend_native.h
>     >  create mode 100644 libavutil/dnn_interface.c
>     >  create mode 100644 libavutil/dnn_interface.h
>     >  create mode 100644 libavutil/dnn_srcnn.h
> 
>     With this change you're trying to use libavformat API from libavutil,
>     which is not ok as the latter must not depend on the former at all. So
>     if anything, this belongs in libavformat.
> 
>     That aside, you're using the ff_ prefix on an installed header, which is
>     unusable from outside the library that contains the symbol, and the
>     structs are not using the AV* namespace either.
>     Does this need to be public API to begin with? If it's only going to be
>     used by one or more filters, then it might as well remain as an internal
>     module in libavfilter.
> 
> 
> Yes, I think it will be used only in libavfilter. I'll move it there then.
> And should I use  ff_ prefix  and  AV* namespace for structs in
> an internal module in libavfilter?

You should use the ff_ prefix for internal, non-static functions only.
You don't need to use any prefix for internal structs.
The AV* prefix is only for public structs, and the av_ prefix is for
public functions.

>  
> 
>     And you need to indent and prettify the tables a bit.
> 
> 
> Do you mean kernels and biases in dnn_srcnn.h?
> Their formatting represents their 4D structure a little bit and it is
> similar to one that I used for the first srcnn filter version that was
> successfully pushed before.

Yes, and i suppose they were pushed like that without the reviewer
noticing they had no indentation or vertical alignment.

See https://ffmpeg.org/developer.html#Coding-Rules-1