Re: [FFmpeg-devel] [PATCH] avfilter: add ssim filter
Hi, On Wed, Jun 24, 2015 at 9:21 AM, Paul B Mahol one...@gmail.com wrote: Signed-off-by: Paul B Mahol one...@gmail.com --- Changelog| 1 + doc/filters.texi | 58 +++ libavfilter/Makefile | 1 + libavfilter/allfilters.c | 1 + libavfilter/vf_ssim.c| 399 +++ 5 files changed, 460 insertions(+) create mode 100644 libavfilter/vf_ssim.c LGTM! Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter: add ssim filter
On 6/23/15, Nicolas George geo...@nsup.org wrote: Le quintidi 5 messidor, an CCXXIII, Paul B Mahol a ecrit : Looks like there are no new comments so I can safely assume current state as is is good enough. If you want to be really safe, you can observe that there is an unanswered question and assume that 20 hours is not enough to be sure there will be no answer. So here it is right now, since you use ultimatums: Just to put it into same file? Moving it into same filter would be also possible but how would then new filter be named then? That depends on the code, you know it better than me. If it often makes sense to compute both the PSNR and the SSIM, then a single filter would be preferred, especially if some of the computations can be used for both. Well tiny_ssim computes PSNR from SSIM but numbers do not match other implementations. I'm sure that our MSE(and PSNR) calculations in vf_psnr.c matches numbers from some page linked from wikipedia. Computations of PSNR and SSIM have not much in common. So the only code that is actually shared is set_meta() which could be moved to some other place in lavfi. If this is done in a single filter, various names come to mind: -vf quality is the most obvious. I just do not like big filters, I made mistake putting multiple different filters in single histogram filter. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter: add ssim filter
Hi, On Tue, Jun 23, 2015 at 10:42 AM, Paul B Mahol one...@gmail.com wrote: On 6/22/15, Paul B Mahol one...@gmail.com wrote: On 6/22/15, Nicolas George geo...@nsup.org wrote: Le quartidi 4 messidor, an CCXXIII, Paul B Mahol a ecrit : Signed-off-by: Paul B Mahol one...@gmail.com --- Changelog| 1 + doc/filters.texi | 53 libavfilter/Makefile | 1 + libavfilter/allfilters.c | 1 + libavfilter/vf_ssim.c| 340 +++ 5 files changed, 396 insertions(+) create mode 100644 libavfilter/vf_ssim.c Would it make sense to merge it with the psnr filter, sharing code and offering a common user interface for various quality measurements? Just to put it into same file? Moving it into same filter would be also possible but how would then new filter be named then? Looks like there are no new comments so I can safely assume current state as is is good enough. Yeah lgtm. Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter: add ssim filter
Le quintidi 5 messidor, an CCXXIII, Paul B Mahol a écrit : Looks like there are no new comments so I can safely assume current state as is is good enough. If you want to be really safe, you can observe that there is an unanswered question and assume that 20 hours is not enough to be sure there will be no answer. So here it is right now, since you use ultimatums: Just to put it into same file? Moving it into same filter would be also possible but how would then new filter be named then? That depends on the code, you know it better than me. If it often makes sense to compute both the PSNR and the SSIM, then a single filter would be preferred, especially if some of the computations can be used for both. If this is done in a single filter, various names come to mind: -vf quality is the most obvious. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter: add ssim filter
Hi, On Tue, Jun 23, 2015 at 10:45 AM, Ronald S. Bultje rsbul...@gmail.com wrote: On Tue, Jun 23, 2015 at 10:42 AM, Paul B Mahol one...@gmail.com wrote: On 6/22/15, Paul B Mahol one...@gmail.com wrote: On 6/22/15, Nicolas George geo...@nsup.org wrote: Le quartidi 4 messidor, an CCXXIII, Paul B Mahol a ecrit : Signed-off-by: Paul B Mahol one...@gmail.com --- Changelog| 1 + doc/filters.texi | 53 libavfilter/Makefile | 1 + libavfilter/allfilters.c | 1 + libavfilter/vf_ssim.c| 340 +++ 5 files changed, 396 insertions(+) create mode 100644 libavfilter/vf_ssim.c Would it make sense to merge it with the psnr filter, sharing code and offering a common user interface for various quality measurements? Just to put it into same file? Moving it into same filter would be also possible but how would then new filter be named then? Looks like there are no new comments so I can safely assume current state as is is good enough. Yeah lgtm. Actually one question - how would I use this with with a raw yuv file as reference? Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter: add ssim filter
Le quintidi 5 messidor, an CCXXIII, Paul B Mahol a écrit : Well tiny_ssim computes PSNR from SSIM but numbers do not match other implementations. I'm sure that our MSE(and PSNR) calculations in vf_psnr.c matches numbers from some page linked from wikipedia. Computations of PSNR and SSIM have not much in common. So the only code that is actually shared is set_meta() which could be moved to some other place in lavfi. I just do not like big filters, I made mistake putting multiple different filters in single histogram filter. Ok. As long as you thought about and made an informed decision I have no objection. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter: add ssim filter
On 6/22/15, Paul B Mahol one...@gmail.com wrote: On 6/22/15, Nicolas George geo...@nsup.org wrote: Le quartidi 4 messidor, an CCXXIII, Paul B Mahol a ecrit : Signed-off-by: Paul B Mahol one...@gmail.com --- Changelog| 1 + doc/filters.texi | 53 libavfilter/Makefile | 1 + libavfilter/allfilters.c | 1 + libavfilter/vf_ssim.c| 340 +++ 5 files changed, 396 insertions(+) create mode 100644 libavfilter/vf_ssim.c Would it make sense to merge it with the psnr filter, sharing code and offering a common user interface for various quality measurements? Just to put it into same file? Moving it into same filter would be also possible but how would then new filter be named then? Looks like there are no new comments so I can safely assume current state as is is good enough. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter: add ssim filter
On 6/23/15, Ronald S. Bultje rsbul...@gmail.com wrote: Hi, On Tue, Jun 23, 2015 at 10:45 AM, Ronald S. Bultje rsbul...@gmail.com wrote: On Tue, Jun 23, 2015 at 10:42 AM, Paul B Mahol one...@gmail.com wrote: On 6/22/15, Paul B Mahol one...@gmail.com wrote: On 6/22/15, Nicolas George geo...@nsup.org wrote: Le quartidi 4 messidor, an CCXXIII, Paul B Mahol a ecrit : Signed-off-by: Paul B Mahol one...@gmail.com --- Changelog| 1 + doc/filters.texi | 53 libavfilter/Makefile | 1 + libavfilter/allfilters.c | 1 + libavfilter/vf_ssim.c| 340 +++ 5 files changed, 396 insertions(+) create mode 100644 libavfilter/vf_ssim.c Would it make sense to merge it with the psnr filter, sharing code and offering a common user interface for various quality measurements? Just to put it into same file? Moving it into same filter would be also possible but how would then new filter be named then? Looks like there are no new comments so I can safely assume current state as is is good enough. Yeah lgtm. Actually one question - how would I use this with with a raw yuv file as reference? ffmpeg -i main.movie -s WIDTHxHEIGHT [-pix_fmt yuv420p] -i REF.yuv -lavfi ssim -f null - Both psnr and ssim: ffmpeg -i main.movie -s WIDTHxHEIGHT -i REF.yuv -lavfi [0:v][1:v]ssim;[0:v][1:v]psnr -f null - ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter: add ssim filter
Hi, On Mon, Jun 22, 2015 at 12:23 PM, Paul B Mahol one...@gmail.com wrote: +c[0] = ssim(main-data[0], main-linesize[0], +ref-data[0], ref-linesize[0], +s-planewidth[0], s-planeheight[0]); + +c[1] = ssim(main-data[1], main-linesize[1], +ref-data[1], ref-linesize[1], +s-planewidth[1], s-planeheight[1]); + +c[2] = ssim(main-data[2], main-linesize[2], +ref-data[2], ref-linesize[2], +s-planewidth[2], s-planeheight[2]); + +ssimv = c[0] * .8 + .1 * (c[1] + c[2]); + +set_meta(metadata, lavfi.ssim., s-comps[0], c[0]); +set_meta(metadata, lavfi.ssim., s-comps[1], c[1]); +set_meta(metadata, lavfi.ssim., s-comps[2], c[2]); +set_meta(metadata, lavfi.ssim.All, 0, ssimv); So there are just the floats right? Can you convert them to dB values as tiny_ssim does? Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter: add ssim filter
On 6/22/15, Nicolas George geo...@nsup.org wrote: Le quartidi 4 messidor, an CCXXIII, Paul B Mahol a ecrit : Signed-off-by: Paul B Mahol one...@gmail.com --- Changelog| 1 + doc/filters.texi | 53 libavfilter/Makefile | 1 + libavfilter/allfilters.c | 1 + libavfilter/vf_ssim.c| 340 +++ 5 files changed, 396 insertions(+) create mode 100644 libavfilter/vf_ssim.c Would it make sense to merge it with the psnr filter, sharing code and offering a common user interface for various quality measurements? Just to put it into same file? Moving it into same filter would be also possible but how would then new filter be named then? Regards, -- Nicolas George ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter: add ssim filter
On 6/22/15, Ronald S. Bultje rsbul...@gmail.com wrote: Hi, On Mon, Jun 22, 2015 at 12:23 PM, Paul B Mahol one...@gmail.com wrote: +c[0] = ssim(main-data[0], main-linesize[0], +ref-data[0], ref-linesize[0], +s-planewidth[0], s-planeheight[0]); + +c[1] = ssim(main-data[1], main-linesize[1], +ref-data[1], ref-linesize[1], +s-planewidth[1], s-planeheight[1]); + +c[2] = ssim(main-data[2], main-linesize[2], +ref-data[2], ref-linesize[2], +s-planewidth[2], s-planeheight[2]); + +ssimv = c[0] * .8 + .1 * (c[1] + c[2]); + +set_meta(metadata, lavfi.ssim., s-comps[0], c[0]); +set_meta(metadata, lavfi.ssim., s-comps[1], c[1]); +set_meta(metadata, lavfi.ssim., s-comps[2], c[2]); +set_meta(metadata, lavfi.ssim.All, 0, ssimv); So there are just the floats right? Can you convert them to dB values as tiny_ssim does? Sure, I hope file is still LGPL ;-) Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter: add ssim filter
Le quartidi 4 messidor, an CCXXIII, Paul B Mahol a écrit : Signed-off-by: Paul B Mahol one...@gmail.com --- Changelog| 1 + doc/filters.texi | 53 libavfilter/Makefile | 1 + libavfilter/allfilters.c | 1 + libavfilter/vf_ssim.c| 340 +++ 5 files changed, 396 insertions(+) create mode 100644 libavfilter/vf_ssim.c Would it make sense to merge it with the psnr filter, sharing code and offering a common user interface for various quality measurements? Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
