[FFmpeg-cvslog] r12144 - trunk/libavcodec/mpeg12.c

[Rich Felker]

On Tue, Feb 19, 2008 at 08:24:22PM +0200, Ivan Kalvachev wrote:
> On Feb 19, 2008 7:33 PM, Rich Felker <dalias at aerifal.cx> wrote:
> > On Tue, Feb 19, 2008 at 09:24:30AM -0800, Mike Melanson wrote:
> > > Rich Felker wrote:
> > > > On Tue, Feb 19, 2008 at 04:55:14PM +0100, michael wrote:
> > > >> Author: michael
> > > >> Date: Tue Feb 19 16:55:14 2008
> > > >> New Revision: 12144
> > > >>
> > > >> Log:
> > > >> Reduce the number of senselessly scanned bytes.
> > > >>
> > > >>
> > > >> Modified:
> > > >>    trunk/libavcodec/mpeg12.c
> > > >>
> > > >> Modified: trunk/libavcodec/mpeg12.c
> > > >> ==============================================================================
> > > >> --- trunk/libavcodec/mpeg12.c      (original)
> > > >> +++ trunk/libavcodec/mpeg12.c      Tue Feb 19 16:55:14 2008
> > > >> @@ -1869,7 +1869,7 @@ static int mpeg_decode_slice(Mpeg1Contex
> > > >>          }
> > > >>      }
> > > >>  eos: // end of slice
> > > >> -    *buf += get_bits_count(&s->gb)/8 - 1;
> > > >> +    *buf += (get_bits_count(&s->gb)-1)/8;
> > > >
> > > > Have you checked what code gcc generates for this /8?? I suspect it's
> > > > a division!! Use >>3 or fix get_bits_count to return unsigned instead
> > > > of int.
> > >
> > > gcc 4.1.3 on Ubuntu/x86_32 compiles the operation down to a sar
> > > instruction. Though I agree that it's a good idea to specify bit shifts
> > > in the interest of bit accuracy, at least for these applications.
> >
> > I don't see how it's an issue of bit accuracy here. Rather it's an
> > issue of the compiler not necessarily knowing that it's safe to use a
> > bit shift due to signedness issues. Also for instance, if
> > get_bits_count can return 0, then using a shift is not safe unless
> > it's a signed shift.
> 
> Even gcc 2.95.3 won't issue divide on exact power of two. The code
> would check the sign  add correction and then do the shift.
> Doing unsigned >> is still faster :)

The conditional is probably at least as slow as the divide...
Unless it has a nice trick with flags/carry..

Rich