[FFmpeg-devel] lavfi/vf_libvmaf: update to use libvmaf v1.3.9

Submitted by Kyle Swanson on Aug. 7, 2018, 10:37 p.m.

Details

Message ID 20180807223742.96627-1-k@ylo.ph
State New
Headers show

Commit Message

Kyle Swanson Aug. 7, 2018, 10:37 p.m.
From: Kyle Swanson <kswanson@netflix.com>

Signed-off-by: Kyle Swanson <kswanson@netflix.com>
---
 configure                | 2 +-
 libavfilter/vf_libvmaf.c | 9 ++++++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

Comments

James Almer Aug. 8, 2018, 3:02 a.m.
On 8/7/2018 7:37 PM, Kyle Swanson wrote:
> From: Kyle Swanson <kswanson@netflix.com>
> 
> Signed-off-by: Kyle Swanson <kswanson@netflix.com>
> ---
>  configure                | 2 +-
>  libavfilter/vf_libvmaf.c | 9 ++++++++-
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/configure b/configure
> index 15a58935de..e718c1531c 100755
> --- a/configure
> +++ b/configure
> @@ -6089,7 +6089,7 @@ enabled libtwolame        && require libtwolame twolame.h twolame_init -ltwolame
>                                 die "ERROR: libtwolame must be installed and version must be >= 0.3.10"; }
>  enabled libv4l2           && require_pkg_config libv4l2 libv4l2 libv4l2.h v4l2_ioctl
>  enabled libvidstab        && require_pkg_config libvidstab "vidstab >= 0.98" vid.stab/libvidstab.h vsMotionDetectInit
> -enabled libvmaf           && require_pkg_config libvmaf "libvmaf >= 0.6.2" libvmaf.h compute_vmaf
> +enabled libvmaf           && require_pkg_config libvmaf "libvmaf >= 1.3.9" libvmaf.h compute_vmaf
>  enabled libvo_amrwbenc    && require libvo_amrwbenc vo-amrwbenc/enc_if.h E_IF_init -lvo-amrwbenc
>  enabled libvorbis         && require_pkg_config libvorbis vorbis vorbis/codec.h vorbis_info_init &&
>                               require_pkg_config libvorbisenc vorbisenc vorbis/vorbisenc.h vorbis_encode_init
> diff --git a/libavfilter/vf_libvmaf.c b/libavfilter/vf_libvmaf.c
> index 5d47a74375..249e50c720 100644
> --- a/libavfilter/vf_libvmaf.c
> +++ b/libavfilter/vf_libvmaf.c
> @@ -62,6 +62,9 @@ typedef struct LIBVMAFContext {
>      int ssim;
>      int ms_ssim;
>      char *pool;
> +    int n_threads;
> +    int n_subsample;
> +    int enable_conf_interval;
>      int error;
>  } LIBVMAFContext;
>  
> @@ -78,6 +81,9 @@ static const AVOption libvmaf_options[] = {
>      {"ssim",  "Enables computing ssim along with vmaf.",                                OFFSET(ssim), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, FLAGS},
>      {"ms_ssim",  "Enables computing ms-ssim along with vmaf.",                          OFFSET(ms_ssim), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, FLAGS},
>      {"pool",  "Set the pool method to be used for computing vmaf.",                     OFFSET(pool), AV_OPT_TYPE_STRING, {.str=NULL}, 0, 1, FLAGS},
> +    {"n_threads", "Set number of threads to be used when computing vmaf.",              OFFSET(n_threads), AV_OPT_TYPE_INT, {.i64=0}, 0, UINT_MAX, FLAGS},
> +    {"n_subsample", "Set interval for frame subsampling used when computing vmaf.",     OFFSET(n_subsample), AV_OPT_TYPE_INT, {.i64=1}, 1, UINT_MAX, FLAGS},
> +    {"enable_conf_interval",  "Enables confidence interval.",                           OFFSET(enable_conf_interval), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, FLAGS},
>      { NULL }
>  };
>  
> @@ -166,7 +172,8 @@ static void compute_vmaf_score(LIBVMAFContext *s)
>                              read_frame, s, s->model_path, s->log_path,
>                              s->log_fmt, 0, 0, s->enable_transform,
>                              s->phone_model, s->psnr, s->ssim,
> -                            s->ms_ssim, s->pool);
> +                            s->ms_ssim, s->pool,
> +                            s->n_threads, s->n_subsample, s->enable_conf_interval);

It would be ideal if this library could stop breaking the API so often.
If the current API is not extensible, maybe you should consider
introducing a new one that is.

>  }
>  
>  static void *call_vmaf(void *ctx)
>
Kyle Swanson Aug. 8, 2018, 3:21 a.m.
Hi,

On Tue, Aug 7, 2018 at 8:02 PM, James Almer <jamrial@gmail.com> wrote:

> On 8/7/2018 7:37 PM, Kyle Swanson wrote:
> > From: Kyle Swanson <kswanson@netflix.com>
> >
> > Signed-off-by: Kyle Swanson <kswanson@netflix.com>
> > ---
> >  configure                | 2 +-
> >  libavfilter/vf_libvmaf.c | 9 ++++++++-
> >  2 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/configure b/configure
> > index 15a58935de..e718c1531c 100755
> > --- a/configure
> > +++ b/configure
> > @@ -6089,7 +6089,7 @@ enabled libtwolame        && require libtwolame
> twolame.h twolame_init -ltwolame
> >                                 die "ERROR: libtwolame must be installed
> and version must be >= 0.3.10"; }
> >  enabled libv4l2           && require_pkg_config libv4l2 libv4l2
> libv4l2.h v4l2_ioctl
> >  enabled libvidstab        && require_pkg_config libvidstab "vidstab >=
> 0.98" vid.stab/libvidstab.h vsMotionDetectInit
> > -enabled libvmaf           && require_pkg_config libvmaf "libvmaf >=
> 0.6.2" libvmaf.h compute_vmaf
> > +enabled libvmaf           && require_pkg_config libvmaf "libvmaf >=
> 1.3.9" libvmaf.h compute_vmaf
> >  enabled libvo_amrwbenc    && require libvo_amrwbenc
> vo-amrwbenc/enc_if.h E_IF_init -lvo-amrwbenc
> >  enabled libvorbis         && require_pkg_config libvorbis vorbis
> vorbis/codec.h vorbis_info_init &&
> >                               require_pkg_config libvorbisenc vorbisenc
> vorbis/vorbisenc.h vorbis_encode_init
> > diff --git a/libavfilter/vf_libvmaf.c b/libavfilter/vf_libvmaf.c
> > index 5d47a74375..249e50c720 100644
> > --- a/libavfilter/vf_libvmaf.c
> > +++ b/libavfilter/vf_libvmaf.c
> > @@ -62,6 +62,9 @@ typedef struct LIBVMAFContext {
> >      int ssim;
> >      int ms_ssim;
> >      char *pool;
> > +    int n_threads;
> > +    int n_subsample;
> > +    int enable_conf_interval;
> >      int error;
> >  } LIBVMAFContext;
> >
> > @@ -78,6 +81,9 @@ static const AVOption libvmaf_options[] = {
> >      {"ssim",  "Enables computing ssim along with vmaf.",
>                 OFFSET(ssim), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, FLAGS},
> >      {"ms_ssim",  "Enables computing ms-ssim along with vmaf.",
>                 OFFSET(ms_ssim), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, FLAGS},
> >      {"pool",  "Set the pool method to be used for computing vmaf.",
>                  OFFSET(pool), AV_OPT_TYPE_STRING, {.str=NULL}, 0, 1,
> FLAGS},
> > +    {"n_threads", "Set number of threads to be used when computing
> vmaf.",              OFFSET(n_threads), AV_OPT_TYPE_INT, {.i64=0}, 0,
> UINT_MAX, FLAGS},
> > +    {"n_subsample", "Set interval for frame subsampling used when
> computing vmaf.",     OFFSET(n_subsample), AV_OPT_TYPE_INT, {.i64=1}, 1,
> UINT_MAX, FLAGS},
> > +    {"enable_conf_interval",  "Enables confidence interval.",
>                  OFFSET(enable_conf_interval), AV_OPT_TYPE_BOOL, {.i64=0},
> 0, 1, FLAGS},
> >      { NULL }
> >  };
> >
> > @@ -166,7 +172,8 @@ static void compute_vmaf_score(LIBVMAFContext *s)
> >                              read_frame, s, s->model_path, s->log_path,
> >                              s->log_fmt, 0, 0, s->enable_transform,
> >                              s->phone_model, s->psnr, s->ssim,
> > -                            s->ms_ssim, s->pool);
> > +                            s->ms_ssim, s->pool,
> > +                            s->n_threads, s->n_subsample,
> s->enable_conf_interval);
>
> It would be ideal if this library could stop breaking the API so often.
> If the current API is not extensible, maybe you should consider
> introducing a new one that is.
>

Yes, I'm in agreement with you. The libvmaf API hasn't been stable and has
more or less been evolving over time. I've had a few discussions with Zhi
about a new API that is both more manageable to use and also extensible.
For now this is the interface, but I plan to invest some of my time in the
future to help with a new API. At that point there will be another breaking
change, but it should be more stable going forward. I can also manage the
updates to this filter during that time as well.


>
> >  }
> >
> >  static void *call_vmaf(void *ctx)
> >
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Thanks,
Kyle
Carl Eugen Hoyos Aug. 8, 2018, 8:32 p.m.
2018-08-08 5:02 GMT+02:00, James Almer <jamrial@gmail.com>:

> It would be ideal if this library could stop breaking the API so often.

Not that I care much but don't we have a requirement
for external libs to have a stable api?

Carl Eugen
Kyle Swanson Aug. 9, 2018, 5:10 p.m.
Hi,

On Tue, Aug 7, 2018 at 3:37 PM, Kyle Swanson <k@ylo.ph> wrote:

> From: Kyle Swanson <kswanson@netflix.com>
>
> Signed-off-by: Kyle Swanson <kswanson@netflix.com>
> ---
>  configure                | 2 +-
>  libavfilter/vf_libvmaf.c | 9 ++++++++-
>  2 files changed, 9 insertions(+), 2 deletions(-)
>
>
I'll update docs and push this sometime in the next 24 hours unless anyone
has something.

Thanks,
Kyle
Kyle Swanson Aug. 10, 2018, 7:06 p.m.
Hi,

> On Aug 9, 2018, at 10:10 AM, Kyle Swanson <k@ylo.ph> wrote:
> 
> Hi,
> 
>> On Tue, Aug 7, 2018 at 3:37 PM, Kyle Swanson <k@ylo.ph> wrote:
>> From: Kyle Swanson <kswanson@netflix.com>
>> 
>> Signed-off-by: Kyle Swanson <kswanson@netflix.com>
>> ---
>>  configure                | 2 +-
>>  libavfilter/vf_libvmaf.c | 9 ++++++++-
>>  2 files changed, 9 insertions(+), 2 deletions(-)
>> 
> 
> I'll update docs and push this sometime in the next 24 hours unless anyone has something.
> 
> Thanks,
> Kyle

Pushed. Thanks.

Kyle
Carl Eugen Hoyos Aug. 13, 2018, 6:56 p.m.
2018-08-08 0:37 GMT+02:00, Kyle Swanson <k@ylo.ph>:

> +    {"n_threads", "Set number of threads to be used when computing vmaf.",
>             OFFSET(n_threads), AV_OPT_TYPE_INT, {.i64=0}, 0, UINT_MAX,
> FLAGS},

Why does this not use -filter_threads?

Carl Eugen
Kyle Swanson Aug. 13, 2018, 9:12 p.m.
Hi,

On Mon, Aug 13, 2018 at 11:56 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com>
wrote:

> 2018-08-08 0:37 GMT+02:00, Kyle Swanson <k@ylo.ph>:
>
> > +    {"n_threads", "Set number of threads to be used when computing
> vmaf.",
> >             OFFSET(n_threads), AV_OPT_TYPE_INT, {.i64=0}, 0, UINT_MAX,
> > FLAGS},
>
> Why does this not use -filter_threads?


Didn't know it was an option. I can certainly change the code to use it. I
assume this sets AVFilterContext::nb_threads?


> Carl Eugen
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Thanks,
Kyle
Carl Eugen Hoyos Aug. 13, 2018, 9:54 p.m.
2018-08-13 23:12 GMT+02:00, Kyle Swanson <k@ylo.ph>:
> Hi,
>
> On Mon, Aug 13, 2018 at 11:56 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com>
> wrote:
>
>> 2018-08-08 0:37 GMT+02:00, Kyle Swanson <k@ylo.ph>:
>>
>> > +    {"n_threads", "Set number of threads to be used when computing
>> vmaf.",
>> >             OFFSET(n_threads), AV_OPT_TYPE_INT, {.i64=0}, 0, UINT_MAX,
>> > FLAGS},
>>
>> Why does this not use -filter_threads?
>
>
> Didn't know it was an option.

> I can certainly change the code to use it.

Please do it if it works.

Carl Eugen
Gyan Aug. 14, 2018, 4:33 a.m.
On 14-08-2018 03:24 AM, Carl Eugen Hoyos wrote:

>>>
>>> Why does this not use -filter_threads?
>>
>>
>> Didn't know it was an option.
> 
>> I can certainly change the code to use it.
> 
> Please do it if it works.

filter_threads applies to all simple filtergraphs. The private option is 
specifically targeted. Isn't that better?

Gyan
Paul B Mahol Aug. 14, 2018, 7:50 a.m.
On 8/14/18, Gyan Doshi <gyandoshi@gmail.com> wrote:
>
>
> On 14-08-2018 03:24 AM, Carl Eugen Hoyos wrote:
>
>>>>
>>>> Why does this not use -filter_threads?
>>>
>>>
>>> Didn't know it was an option.
>>
>>> I can certainly change the code to use it.
>>
>> Please do it if it works.
>
> filter_threads applies to all simple filtergraphs. The private option is
> specifically targeted. Isn't that better?

filter_threads is used only for internal threading, so using it for something
external seems wrong.
Gyan Aug. 14, 2018, 7:57 a.m.
On 14-08-2018 01:20 PM, Paul B Mahol wrote:
> On 8/14/18, Gyan Doshi <gyandoshi@gmail.com> wrote:
>>
>>
>> On 14-08-2018 03:24 AM, Carl Eugen Hoyos wrote:
>>
>>>>>
>>>>> Why does this not use -filter_threads?
>>>>
>>>>
>>>> Didn't know it was an option.
>>>
>>>> I can certainly change the code to use it.
>>>
>>> Please do it if it works.
>>
>> filter_threads applies to all simple filtergraphs. The private option is
>> specifically targeted. Isn't that better?
> 
> filter_threads is used only for internal threading, so using it for something
> external seems wrong.


I also realized that libvmaf requires two inputs and should/will be used 
within a complex filtergraph so -filter_threads wouldn't apply even if 
wanted.

Gyan

Patch hide | download patch | download mbox

diff --git a/configure b/configure
index 15a58935de..e718c1531c 100755
--- a/configure
+++ b/configure
@@ -6089,7 +6089,7 @@  enabled libtwolame        && require libtwolame twolame.h twolame_init -ltwolame
                                die "ERROR: libtwolame must be installed and version must be >= 0.3.10"; }
 enabled libv4l2           && require_pkg_config libv4l2 libv4l2 libv4l2.h v4l2_ioctl
 enabled libvidstab        && require_pkg_config libvidstab "vidstab >= 0.98" vid.stab/libvidstab.h vsMotionDetectInit
-enabled libvmaf           && require_pkg_config libvmaf "libvmaf >= 0.6.2" libvmaf.h compute_vmaf
+enabled libvmaf           && require_pkg_config libvmaf "libvmaf >= 1.3.9" libvmaf.h compute_vmaf
 enabled libvo_amrwbenc    && require libvo_amrwbenc vo-amrwbenc/enc_if.h E_IF_init -lvo-amrwbenc
 enabled libvorbis         && require_pkg_config libvorbis vorbis vorbis/codec.h vorbis_info_init &&
                              require_pkg_config libvorbisenc vorbisenc vorbis/vorbisenc.h vorbis_encode_init
diff --git a/libavfilter/vf_libvmaf.c b/libavfilter/vf_libvmaf.c
index 5d47a74375..249e50c720 100644
--- a/libavfilter/vf_libvmaf.c
+++ b/libavfilter/vf_libvmaf.c
@@ -62,6 +62,9 @@  typedef struct LIBVMAFContext {
     int ssim;
     int ms_ssim;
     char *pool;
+    int n_threads;
+    int n_subsample;
+    int enable_conf_interval;
     int error;
 } LIBVMAFContext;
 
@@ -78,6 +81,9 @@  static const AVOption libvmaf_options[] = {
     {"ssim",  "Enables computing ssim along with vmaf.",                                OFFSET(ssim), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, FLAGS},
     {"ms_ssim",  "Enables computing ms-ssim along with vmaf.",                          OFFSET(ms_ssim), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, FLAGS},
     {"pool",  "Set the pool method to be used for computing vmaf.",                     OFFSET(pool), AV_OPT_TYPE_STRING, {.str=NULL}, 0, 1, FLAGS},
+    {"n_threads", "Set number of threads to be used when computing vmaf.",              OFFSET(n_threads), AV_OPT_TYPE_INT, {.i64=0}, 0, UINT_MAX, FLAGS},
+    {"n_subsample", "Set interval for frame subsampling used when computing vmaf.",     OFFSET(n_subsample), AV_OPT_TYPE_INT, {.i64=1}, 1, UINT_MAX, FLAGS},
+    {"enable_conf_interval",  "Enables confidence interval.",                           OFFSET(enable_conf_interval), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, FLAGS},
     { NULL }
 };
 
@@ -166,7 +172,8 @@  static void compute_vmaf_score(LIBVMAFContext *s)
                             read_frame, s, s->model_path, s->log_path,
                             s->log_fmt, 0, 0, s->enable_transform,
                             s->phone_model, s->psnr, s->ssim,
-                            s->ms_ssim, s->pool);
+                            s->ms_ssim, s->pool,
+                            s->n_threads, s->n_subsample, s->enable_conf_interval);
 }
 
 static void *call_vmaf(void *ctx)