On Sat, Oct 07, 2017 at 12:12:15PM +0200, Michael Niedermayer wrote: > On Sat, Oct 07, 2017 at 10:09:45AM +0200, Paul B Mahol wrote: > > On 10/7/17, James Almer <jamr...@gmail.com> wrote: > > > On 10/6/2017 7:09 PM, wm4 wrote: > > >> On Fri, 6 Oct 2017 18:02:44 -0300 > > >> James Almer <jamr...@gmail.com> wrote: > > >> > > >>> On 10/6/2017 5:44 PM, Paul B Mahol wrote: > > >>>> On 10/6/17, Michael Niedermayer <mich...@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 > > >>>>>> <mich...@niedermayer.cc> > > >>>>>> wrote: > > >>>>>> > > >>>>>>> On Sat, Sep 30, 2017 at 03:51:41PM +0000, Ashish Singh wrote: > > >>>>>>>> ffmpeg | branch: master | Ashish Singh <ashk43...@gmail.com> | Sat > > >>>>>>>> Sep > > >>>>>>> 16 02:35:58 2017 +0530| [148c8e88c43cfbabd6aee9f01ef30942cee9d359] | > > >>>>>>> committer: Ronald S. Bultje > > >>>>>>>> > > >>>>>>>> avfilter: add vmafmotion filter > > >>>>>>>> > > >>>>>>>> Signed-off-by: Ashish Singh <ashk43...@gmail.com> > > >>>>>>>> Signed-off-by: Ronald S. Bultje <rsbul...@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
yet another solution (if people dislike all suggestions so far) would be adding a flag to filters which are safe to use with untrusted parameters. Or a flag to options which are unsafe so they are blocked from being set from an untrusted input string There are many possibilities ... [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If a bugfix only changes things apparently unrelated to the bug with no further explanation, that is a good sign that the bugfix is wrong.
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel