[FFmpeg-devel] [PATCH] Added VMAF filter linking to Netflix's vmaf library

[Moritz Barsnick]

On Wed, Jun 21, 2017 at 17:46:40 +0530, Ashish Singh wrote:

What does vmaf's Apache License imply for ffmpeg? (Important for GPL vs LGPL builds.)

You *might* want to resolve the abbreviation VMAF somewhere. Probably
in the as-yet missing documentation.

>      libv4l2
> +	libvmaf
>      libvorbis

That's a tab instead of spaces. Please use tools/patcheck to find such
issues with your patch (probably in other locations as well).


> + * Caculate the VMAF between two input videos.
      ^Calculate

> +    av_dict_set(metadata,key,value,0);

Please use spaces properly (after commas).

> +    if(p == 0){

Style: if (p==0) {
I believe ffmpeg prefers: if (!p) {

> +    if(eof == 1){

Style. (I won't comment the other places.)

> +    char *model_path = "/usr/local/share/model/vmaf_v0.6.1.pkl";

I don't think this is acceptable in ffmpeg source, especially if it's
not configurable.

> +        av_log(ctx, AV_LOG_ERROR, "Width and height of input videos must be same.\n");

"... the same" or "identical". You could also quote the actual
dimensions (not totally necessary though).

> +        av_log(ctx, AV_LOG_ERROR, "Inputs must be of same pixel format.\n");

You could quote the actual formats (not totally necessary though).

I can't say much about the rest.

Cheers,
Moritz