[FFmpeg-devel] [PATCH] intmath: remove av_ctz.

[Ronald S. Bultje]

Hi,

On Sun, Oct 11, 2015 at 9:17 PM, Ganesh Ajjanagadde <gajjanag at mit.edu>
wrote:

> On Sun, Oct 11, 2015 at 9:12 PM, Ganesh Ajjanagadde <gajjanag at mit.edu>
> wrote:
> > On Sun, Oct 11, 2015 at 6:04 PM, Ronald S. Bultje <rsbultje at gmail.com>
> wrote:
> >> Hi,
> >>
> >> On Sun, Oct 11, 2015 at 5:52 PM, Andreas Cadhalpun <
> >> andreas.cadhalpun at googlemail.com> wrote:
> >>
> >>> On 11.10.2015 23:44, Ronald S. Bultje wrote:
> >>> > It's a non-installed header and only used in one place (flacenc).
> >>> > Since ff_ctz is static inline, it's fine to use that instead.
> >>> > ---
> >>> >  doc/APIchanges       |  3 ---
> >>> >  libavcodec/flacenc.c |  2 +-
> >>> >  libavutil/intmath.c  |  5 -----
> >>> >  libavutil/intmath.h  | 14 ++++++--------
> >>> >  4 files changed, 7 insertions(+), 17 deletions(-)
> >>>
> >>> Should be fine.
> >>
> >>
> >> Thanks, pushed.
> >
> > Since there is still time, and I did not think of this before, I would
> > like to replace ff_ctz with ff_ctz32. There are a couple of reasons:
> > 1. It is used with an int32 argument in flacenc.
> > 2. I can do a deBruijn optimization for this as well. With an int also
> > I could do it, but it would need some ifdefry depending on whether int
> > is 32 bit or 64 bits.
> >
> > Let me see if I understand API/ABI with respect to this proposed
> > change correctly now:
> > API is not broken, as this is not public.
> > ABI is broken, since the width of operands to ff_ctz has could change
> > from 64 to 32 bits.
>
> That actually raises the question of whether int = 32 bits is assumed
> everywhere. For instance, if int = 64 bits, at least on the Intel
> compiler, ff_ctz is broken: AFAIK, _bit_scan_forward operates on 32
> bit operands.


I think you can safely assume int is 32bit, we do so in numerous places.

Ronald