| Message ID | 20171011135348.4347-1-ashk43712@gmail.com |
|---|---|
| State | New |
| Headers | show |
2017-10-11 15:53 GMT+02:00 Ashish Pratap Singh <ashk43712@gmail.com>: > Hi, this patch fixes the seg fault which ocuured while running libvmaf filter > with option psnr=1. This also improves libvmaf doc a bit. Please split in two patches. Carl Eugen
Hi, On Thu, Oct 12, 2017 at 4:43 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2017-10-11 15:53 GMT+02:00 Ashish Pratap Singh <ashk43712@gmail.com>: > > > Hi, this patch fixes the seg fault which ocuured while running libvmaf > filter > > with option psnr=1. This also improves libvmaf doc a bit. > > Please split in two patches. Split, and pushed. Ronald
2017-11-06 13:52 GMT+01:00 Ronald S. Bultje <rsbultje@gmail.com>: > Hi, > > On Thu, Oct 12, 2017 at 4:43 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> > wrote: > >> 2017-10-11 15:53 GMT+02:00 Ashish Pratap Singh <ashk43712@gmail.com>: >> >> > Hi, this patch fixes the seg fault which ocuured while running libvmaf >> > filter with option psnr=1. This also improves libvmaf doc a bit. >> >> Please split in two patches. > > Split, and pushed. + format = (char *) s->desc->name; Isn't this cast a bad idea? Thank you for reviving this, Carl Eugen
Hi, On Nov 6, 2017 18:22, "Ronald S. Bultje" <rsbultje@gmail.com> wrote: Hi, On Thu, Oct 12, 2017 at 4:43 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2017-10-11 15:53 GMT+02:00 Ashish Pratap Singh <ashk43712@gmail.com>: > > > Hi, this patch fixes the seg fault which ocuured while running libvmaf > filter > > with option psnr=1. This also improves libvmaf doc a bit. > > Please split in two patches. Split, and pushed. Ronald Thanks.
Hi, On Mon, Nov 6, 2017 at 8:21 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2017-11-06 13:52 GMT+01:00 Ronald S. Bultje <rsbultje@gmail.com>: > > Hi, > > > > On Thu, Oct 12, 2017 at 4:43 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> > > wrote: > > > >> 2017-10-11 15:53 GMT+02:00 Ashish Pratap Singh <ashk43712@gmail.com>: > >> > >> > Hi, this patch fixes the seg fault which ocuured while running libvmaf > >> > filter with option psnr=1. This also improves libvmaf doc a bit. > >> > >> Please split in two patches. > > > > Split, and pushed. > > + format = (char *) s->desc->name; > > Isn't this cast a bad idea? It's an API thing. We can either strdup(), or use a cast, or have a compiler warning. libvmaf does not modify the argument even if it doesn't use const here. Ronald
2017-11-06 15:09 GMT+01:00 Ronald S. Bultje <rsbultje@gmail.com>: > Hi, > > On Mon, Nov 6, 2017 at 8:21 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> 2017-11-06 13:52 GMT+01:00 Ronald S. Bultje <rsbultje@gmail.com>: >> > Split, and pushed. >> >> + format = (char *) s->desc->name; >> >> Isn't this cast a bad idea? > > It's an API thing. We can either strdup(), or use a cast, or have a > compiler warning. Yes. > libvmaf does not modify the argument even if it doesn't > use const here. I hoped (and assumed) so. What I meant was: It is one thing to know that within FFmpeg, we do not modify a pointer target and therefore decide to cast, but isn't it another (bad) thing to assume this for an external library? Carl Eugen
Hi, On Mon, Nov 6, 2017 at 1:58 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > It is one thing to know that within FFmpeg, we do not modify > a pointer target and therefore decide to cast, but isn't it > another (bad) thing to assume this for an external library? I'm guessing that what you mean is "libvmaf may not touch it right now, but the API doesn't guarantee that such an implementation detail doesn't change in the future", right? Yes, you're right. Potential solutions are to make libvmaf const-correct or to strdup in our implementation. I'm fine with both, although I'd prefer if we didn't strdup every frame, i.e. move it to some global init/uninit function. Ronald
2017-11-06 21:27 GMT+01:00 Ronald S. Bultje <rsbultje@gmail.com>: > Hi, > > On Mon, Nov 6, 2017 at 1:58 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> It is one thing to know that within FFmpeg, we do not modify >> a pointer target and therefore decide to cast, but isn't it >> another (bad) thing to assume this for an external library? > > I'm guessing that what you mean is "libvmaf may not touch it right now, but > the API doesn't guarantee that such an implementation detail doesn't change > in the future", right? > > Yes, you're right. Potential solutions are to make libvmaf const-correct or > to strdup in our implementation. > I'm fine with both Me too. Carl Eugen
diff --git a/doc/filters.texi b/doc/filters.texi index e26dde4b1a..90790c2261 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -9885,25 +9885,16 @@ distances from the focal point in the source and target images, respectively. @section libvmaf -Obtain the average VMAF (Video Multi-Method Assessment Fusion) +Obtain the VMAF (Video Multi-Method Assessment Fusion) score between two input videos. -This filter takes two input videos. - -Both video inputs must have the same resolution and pixel format for -this filter to work correctly. Also it assumes that both inputs -have the same number of frames, which are compared one by one. - -The obtained average VMAF score is printed through the logging system. +The obtained VMAF score is printed through the logging system. It requires Netflix's vmaf library (libvmaf) as a pre-requisite. After installing the library it can be enabled using: @code{./configure --enable-libvmaf}. If no model path is specified it uses the default model: @code{vmaf_v0.6.1.pkl}. -On the below examples the input file @file{main.mpg} being processed is -compared with the reference file @file{ref.mpg}. - The filter has following options: @table @option @@ -9934,12 +9925,14 @@ Enables computing ssim along with vmaf. Enables computing ms_ssim along with vmaf. @item pool -Set the pool method to be used for computing vmaf. +Set the pool method (mean, min or harmonic mean) to be used for computing vmaf. @end table This filter also supports the @ref{framesync} options. -For example: +On the below examples the input file @file{main.mpg} being processed is +compared with the reference file @file{ref.mpg}. + @example ffmpeg -i main.mpg -i ref.mpg -lavfi libvmaf -f null - @end example diff --git a/libavfilter/vf_libvmaf.c b/libavfilter/vf_libvmaf.c index 2c3a9f3349..7670c51e21 100644 --- a/libavfilter/vf_libvmaf.c +++ b/libavfilter/vf_libvmaf.c @@ -40,7 +40,6 @@ typedef struct LIBVMAFContext { const AVClass *class; FFFrameSync fs; const AVPixFmtDescriptor *desc; - char *format; int width; int height; double vmaf_score; @@ -149,6 +148,7 @@ static void compute_vmaf_score(LIBVMAFContext *s) { int (*read_frame)(float *ref_data, float *main_data, float *temp_data, int stride, void *ctx); + char *format; if (s->desc->comp[0].depth <= 8) { read_frame = read_frame_8bit; @@ -156,7 +156,9 @@ static void compute_vmaf_score(LIBVMAFContext *s) read_frame = read_frame_10bit; } - s->vmaf_score = compute_vmaf(s->format, s->width, s->height, read_frame, s, + format = (char *) s->desc->name; + + s->vmaf_score = compute_vmaf(format, s->width, s->height, 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); @@ -258,7 +260,6 @@ static int config_input_ref(AVFilterLink *inlink) return 0; } - static int config_output(AVFilterLink *outlink) { AVFilterContext *ctx = outlink->src;