On Sun, Oct 11, 2015 at 10:02:29PM -0400, Ronald S. Bultje wrote: > Hi, > > On Sun, Oct 11, 2015 at 9:17 PM, Ganesh Ajjanagadde <gajja...@mit.edu> > wrote: > > > On Sun, Oct 11, 2015 at 9:12 PM, Ganesh Ajjanagadde <gajja...@mit.edu> > > wrote: > > > On Sun, Oct 11, 2015 at 6:04 PM, Ronald S. Bultje <rsbul...@gmail.com> > > wrote: > > >> Hi, > > >> > > >> On Sun, Oct 11, 2015 at 5:52 PM, Andreas Cadhalpun < > > >> andreas.cadhal...@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.
we assume int >= 32 bit all over (posix gurantees it) int == 32bit is assumed in some arch specific code like simd asm generic code should not assume int to be 32bit or rather IMHO we should not conciously add any such code as it just violates C and will break on some (currently) uncommon platforms/configs [...] -- 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
