[FFmpeg-devel] [FFmpeg-cvslog] avfilter: add vmafmotion filter

[Michael Niedermayer]

On Sat, Oct 07, 2017 at 10:09:45AM +0200, Paul B Mahol wrote:
> On 10/7/17, James Almer <jamrial at gmail.com> wrote:
> > On 10/6/2017 7:09 PM, wm4 wrote:
> >> On Fri, 6 Oct 2017 18:02:44 -0300
> >> James Almer <jamrial at gmail.com> wrote:
> >>
> >>> On 10/6/2017 5:44 PM, Paul B Mahol wrote:
> >>>> On 10/6/17, Michael Niedermayer <michael at niedermayer.cc> wrote:
> >>>>> On Fri, Oct 06, 2017 at 10:03:16AM -0400, Ronald S. Bultje wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> On Thu, Oct 5, 2017 at 7:52 PM, Michael Niedermayer
> >>>>>> <michael at niedermayer.cc>
> >>>>>> wrote:
> >>>>>>
> >>>>>>> On Sat, Sep 30, 2017 at 03:51:41PM +0000, Ashish Singh wrote:
> >>>>>>>> ffmpeg | branch: master | Ashish Singh <ashk43712 at gmail.com> | Sat
> >>>>>>>> Sep
> >>>>>>> 16 02:35:58 2017 +0530| [148c8e88c43cfbabd6aee9f01ef30942cee9d359] |
> >>>>>>> committer: Ronald S. Bultje
> >>>>>>>>
> >>>>>>>> avfilter: add vmafmotion filter
> >>>>>>>>
> >>>>>>>> Signed-off-by: Ashish Singh <ashk43712 at gmail.com>
> >>>>>>>> Signed-off-by: Ronald S. Bultje <rsbultje at gmail.com>
> >>>>>>>>
> >>>>>>>>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=
> >>>>>>> 148c8e88c43cfbabd6aee9f01ef30942cee9d359
> >>>>>>>> ---
> >>>>>>>>
> >>>>>>>>  Changelog                   |   1 +
> >>>>>>>>  doc/filters.texi            |  14 ++
> >>>>>>>>  libavfilter/Makefile        |   1 +
> >>>>>>>>  libavfilter/allfilters.c    |   1 +
> >>>>>>>>  libavfilter/vf_vmafmotion.c | 365 ++++++++++++++++++++++++++++++
> >>>>>>> ++++++++++++++
> >>>>>>>>  libavfilter/vmaf_motion.h   |  58 +++++++
> >>>>>>>>  6 files changed, 440 insertions(+)
> >>>>>>> [...]
> >>>>>>>> +static av_cold int init(AVFilterContext *ctx)
> >>>>>>>> +{
> >>>>>>>> +    VMAFMotionContext *s = ctx->priv;
> >>>>>>>> +
> >>>>>>>> +    if (s->stats_file_str) {
> >>>>>>>> +        if (!strcmp(s->stats_file_str, "-")) {
> >>>>>>>
> >>>>>>>> +            s->stats_file = stdout;
> >>>>>>>
> >>>>>>> Using stdout can interfere with the user application using the filter
> >>>>>>>
> >>>>>>>
> >>>>>>>> +        } else {
> >>>>>>>
> >>>>>>>> +            s->stats_file = fopen(s->stats_file_str, "w");
> >>>>>>>
> >>>>>>> Opening a filter parameter provided string for writing is a dangerous
> >>>>>>> way to output data. It allows one with access to the parameters to
> >>>>>>> overwrite any writable file
> >>>>>>>
> >>>>>>> data should only be output in a safe way
> >>>>>>>
> >>>>>>
> >>>>>> The same mechanism is present in ssim/psnr filters. I'm open to any
> >>>>>> alternative method you suggest. These are only settable using explicit
> >>>>>> user
> >>>>>> interaction (and are disabled by default) so I don't particularly see
> >>>>>> the
> >>>>>> problem.
> >>>>>
> >>>>> With this a filter graph can never be taken from an untrusted source
> >>>>>
> >>>>> One filter that outputs statistics without writing to a user specified
> >>>>> filename is libavfilter/af_astats.c
> >>>>
> >>>> So what? Get over it.
> >>>
> >>> What kind of reply is this? What made you think it's justified?
> >>> He literally gave an example of an alternative method as Ronald
> >>> requested.
> >>>
> >>> Some of you people need to chill out already when discussing patches.
> >>
> >> Michael apparently just comes up randomly with this stuff to block
> >> patches...
> >
> > Nobody comes up "randomly" with concerns. Especially not to block
> > patches for the sake of blocking, because if they were bullshit
> > arguments then they would be easy to counter and discard.
> >
> > At this point you're just expressing frustration over having negative
> > reviews or blocking concerns on patches. Sure, it would be great to
> > finish writing something, sending it to the ML and always get a LGTM as
> > reply, but if it doesn't happen then you shouldn't chalk it up to
> > pedantry from the other side.
> >
> > So again, chill out, and discuss/argue. Ask for other devs to chime in
> > if necessary to tip the scales. But please, stop heating up every single
> > thread.
> >
> 
> Sorry but Michael is just trolling around. Several filters already have support
> for writting to output file, also he does not provide solution at all,
> he is just
> blocking progress for without reason.

You do realize that the patch discussed here was applied long before
my comments about the arbitrary file write access ?
Theres nothing thats blocked by this as the code in question is
already applied.

About a solution, I didnt write alot about solutions as iam really fine
with any solution. There are many possible solutions ...
The libavfilter/af_astats.c filter i mentioned outputs statistics
via metadata or av_log as 2 alternatives to direct file write.
There are other possibilities. Also Hendriks suggestion about
turning metadata via some other application option into a file sounds
pretty good to me

Thanks

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20171007/1bcf35a5/attachment.sig>