On Tue, Nov 3, 2015 at 1:20 PM, Ganesh Ajjanagadde <gajja...@mit.edu> wrote: > On Tue, Nov 3, 2015 at 12:54 PM, Muhammad Faiz <mfc...@gmail.com> wrote: >> On Tue, Nov 3, 2015 at 3:47 AM, Ganesh Ajjanagadde <gajja...@mit.edu> wrote: >>> On Tue, Nov 3, 2015 at 5:06 AM, Muhammad Faiz <mfc...@gmail.com> wrote: >>>> On Sat, Oct 31, 2015 at 10:15 AM, Ganesh Ajjanagadde <gajja...@mit.edu> >>>> wrote: >>>>> On Fri, Oct 30, 2015 at 7:29 PM, Michael Niedermayer >>>>> <mich...@niedermayer.cc> wrote: >>>>>> On Fri, Oct 30, 2015 at 06:53:34PM -0400, Ganesh Ajjanagadde wrote: >>>>>>> On Fri, Oct 30, 2015 at 6:35 PM, Michael Niedermayer <michae...@gmx.at> >>>>>>> wrote: >>>>>>> > From: Michael Niedermayer <mich...@niedermayer.cc> >>>>>>> > >>>>>>> > This should fix the build failure of avf_showcqt.c >>>>>>> > >>>>>>> > An alternative solution would be to add a check for fmin/fmax to >>>>>>> > fate-source and >>>>>>> > then to replace them by FFMIN/FFMAX, i can do that if preferred? >>>>>>> > >>>>>>> > Untested due to lack of a affected platform >>>>>>> >>>>>>> I recall some interest on my end to get fmin, fmax etc for different >>>>>>> reasons, and it was remarked that commit >>>>>>> 4436a8f44dedc83767b3d9da9beb85d1fae2ca30 may be relevant. The summary >>>>>>> seems to be that getting it to work on all platforms is not so simple. >>>>>> >>>>>> :/ >>>>>> ill replace the problematic ones by FFMIN/MAX for now so the build >>>>>> failure on the affected msvc platforms is fixed >>>>>> tieing a build fix to this complexity would be unwise >>>>>> >>>>>> >>>>>>> I am definitely interested in getting it to work in order to replace >>>>>>> FFMAX/FFMIN for floating point in especially libavfilter. This will >>>>>>> allow better nan signalling at a slight performance cost. >>>>>> >>>>>> would that performance cost affect all systems or just ones not >>>>>> having fmin/fmax ? >>>>>> i think affecting all systems would be bad >>>>> >>>>> A correct fmin and fmax will be slower than FFMIN/FFMAX, simply >>>>> because they do NaN handling. How much slower, I have not tested. Also >>>>> note that flags may be passed to the compiler to ignore NaN's, >>>>> something which can possibly be done locally if desired. However, I >>>>> view FFMAX/FFMIN as the cleaner solution if NaN signalling/propagation >>>>> is not an issue, so I may not pursue this. >>>>> >>>> >>>> Common problem with FFMIN/FFMAX (and other macros that evaluates >>>> arguments more than once): >>>> FFMAX(a, myfunc(b)) >>>> will call myfunc twice (OK compiler may optimize it and call myfunc once) >>>> and often people don't care about it. >>> >>> That goes without saying, but sometimes compiler can't optimize due to >>> sequence point stuff. This is why the macro should be used cautiously. >>> However, FFmpeg devs are generally cautious about these things so I no >>> longer press this that hard. The real technical benefit (if any) is >>> the different NaN handling as noted above. >> >> Here some examples: >> libavfilter/g729postfilter.c: 544 >> *voicing = FFMAX(*voicing, long_term_filter(adsp, pitch_delay_int, >> residual, >> residual_filt_buf + 10, >> subframe_size)); >> >> others that call sqrt, strlen, etc (altough possibly compiler can optimize >> it) >> >> commit 94494dab910133106e80ece0f79935d78138e415 >> + volume = FFABS(av_expr_eval(volume_expr, expr_vars_val, NULL)); >> I posted the patch, and I didn't noticed or warned that it is wrong. > > So on the note of FFABS - I think all floating point usages at least > have been converted to proper C functions. > > Yes, things like this can be missed, and I personally use the libc > variants unless there is no alternative. > However, it was quite some work for me to convince others to use them, > and I am trying to stay on the sidelines this time by not posting a > patch for it. > OK, it is just matter of programming style, whatever we choose, has advantages and deficiency. The most important thing is that it is work.
Thank's _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel