[FFmpeg-devel] [PATCH] ffmpeg: replace log2 by faster variant

[Ganesh Ajjanagadde]

On Tue, Jan 5, 2016 at 6:44 PM, Michael Niedermayer
<michael at niedermayer.cc> wrote:
> On Mon, Jan 04, 2016 at 05:38:46PM -0800, Ganesh Ajjanagadde wrote:
>> On Sat, Jan 2, 2016 at 7:59 AM, Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
>> > On Sat, Jan 2, 2016 at 6:24 AM, Michael Niedermayer
>> > <michael at niedermayer.cc> wrote:
>> >> On Fri, Jan 01, 2016 at 05:55:31PM -0800, Ganesh Ajjanagadde wrote:
>> >>> On Wed, Dec 30, 2015 at 1:01 AM, Hendrik Leppkes <h.leppkes at gmail.com> wrote:
>> >>> > On Wed, Dec 30, 2015 at 4:39 AM, Ganesh Ajjanagadde
>> >>> > <gajjanagadde at gmail.com> wrote:
>> >>> >> The log is anyway rounded to an integer, so one may use an frexp
>> >>> >> based approach. Note that this may be made frexpf; if arguments are less than
>> >>> >> 2^24 there is no loss. Kept as double precision for simplicity; 2^32 is
>> >>> >> exactly representable as a double.
>> >>> >>
>> >>> >> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>> >>> >> ---
>> >>> >>  ffmpeg.c | 13 ++++++++++++-
>> >>> >>  1 file changed, 12 insertions(+), 1 deletion(-)
>> >>> >>
>> >>> >> diff --git a/ffmpeg.c b/ffmpeg.c
>> >>> >> index 6d01987..ee72f91 100644
>> >>> >> --- a/ffmpeg.c
>> >>> >> +++ b/ffmpeg.c
>> >>> >> @@ -1486,6 +1486,17 @@ static void print_final_stats(int64_t total_size)
>> >>> >>      }
>> >>> >>  }
>> >>> >>
>> >>> >> +static inline int log2i(double d)
>> >>> >> +{
>> >>> >> +    int exp;
>> >>> >> +    double mant;
>> >>> >> +
>> >>> >> +    mant = frexp(d, &exp);
>> >>> >> +    if (mant >= M_SQRT1_2)
>> >>> >> +        return exp;
>> >>> >> +    return exp-1;
>> >>> >> +}
>> >>> >> +
>> >>> >>  static void print_report(int is_last_report, int64_t timer_start, int64_t cur_time)
>> >>> >>  {
>> >>> >>      char buf[1024];
>> >>> >> @@ -1559,7 +1570,7 @@ static void print_report(int is_last_report, int64_t timer_start, int64_t cur_ti
>> >>> >>                  if (qp >= 0 && qp < FF_ARRAY_ELEMS(qp_histogram))
>> >>> >>                      qp_histogram[qp]++;
>> >>> >>                  for (j = 0; j < 32; j++)
>> >>> >> -                    snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), "%X", (int)lrintf(log2(qp_histogram[j] + 1)));
>> >>> >> +                    snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), "%X", log2i(qp_histogram[j] + 1));
>> >>> >>              }
>> >>> >>
>> >>> >>              if ((enc->flags & AV_CODEC_FLAG_PSNR) && (ost->pict_type != AV_PICTURE_TYPE_NONE || is_last_report)) {
>> >>> >> --
>> >>> >> 2.6.4
>> >>> >
>> >>> > This isn't exactly a performance critical area, and defining a custom
>> >>> > function just for this seems somewhat like over-optimization.
>> >>> > Just my opinion, of course, I'll leave the decision up to the
>> >>> > maintainers of ffmpeg.c
>> >>>
>> >>> Ping: based on the above, final decision left to you, Michael.
>> >>
>> >> maybe av_log2() could be used, so as to avoid any increase in
>> >> complexity
>> >
>> > Not really, or at least not without additional code. The lrint rounds
>> > the log; av_log2 either returns the log of the next higher power of 2,
>> > or the one before it. Thus they are quite different things. I don't
>> > know if this is wrong or not; I created this to preserve identical
>> > behavior.
>>
>> @Michael: final thoughts?
>
> is there any loss of usefullness of the output with av_log2() ?
> i dont think we need to maintain exactly identical output for
> such fancy / debug output

AFAIK none, but I was not around when it was made, and I am not a
heavy user :). Replaced by av_log2 locally, will push tomorrow in
absence of further comments.

>
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>