[FFmpeg-devel] [PATCH] new function get_sbits_long()

[Michael Niedermayer]

On Tue, Mar 03, 2009 at 12:56:40AM +0000, M?ns Rullg?rd wrote:
> Michael Niedermayer <michaelni at gmx.at> writes:
> 
> > On Mon, Mar 02, 2009 at 07:38:09PM -0500, Justin Ruggles wrote:
> >> M?ns Rullg?rd wrote:
> >> > Justin Ruggles <justin.ruggles at gmail.com> writes:
> >> > 
> >> >> Reimar D?ffinger wrote:
> >> >>> On Mon, Mar 02, 2009 at 01:35:45PM -0500, Justin Ruggles wrote:
> >> >>>> Reimar D?ffinger wrote:
> >> >>>>> On Mon, Mar 02, 2009 at 06:58:49PM +0100, Michael Niedermayer wrote:
> >> >>>>>> On Mon, Mar 02, 2009 at 12:39:05PM -0500, Justin Ruggles wrote:
> >> >>>>>>> I need a get_sbits_long() function to support FLAC files that are more
> >> >>>>>>> than 24-bit.  There might be a better way to do it, but this works.
> >> >>>>>> only where int is 32bit which isnt guranteed even if likely
> >> >>>>> There is already the NEG_SSR32 macro to take care of that (and it
> >> >>>>> also optimizes gcc's brain-deadness away on x86).
> >> >>>> yes, this works well.
> >> >>>>
> >> >>>> -Justin
> >> >>>>
> >> >>>> Index: libavcodec/bitstream.h
> >> >>>> ===================================================================
> >> >>>> --- libavcodec/bitstream.h	(revision 17735)
> >> >>>> +++ libavcodec/bitstream.h	(working copy)
> >> >>>> @@ -707,6 +707,14 @@
> >> >>>>  }
> >> >>>>  
> >> >>>>  /**
> >> >>>> + * reads 0-32 bits as a signed integer.
> >> >>>> + */
> >> >>>> +static inline int get_sbits_long(GetBitContext *s, int n) {
> >> >>>> +    int val = get_bits_long(s, n);
> >> >>>> +    return NEG_SSR32(val, n);
> >> >>> Can't imagine that, you must use it like in the SHOW_SBITS macro:
> >> >>> NEG_SSR32(val<<(32-n), n)
> >> >> You're right. The FLAC file I tested just happened not to trigger any
> >> >> problem.  I tried another one.
> >> >>
> >> >> -Justin
> >> >>
> >> >>
> >> >> Index: libavcodec/bitstream.h
> >> >> ===================================================================
> >> >> --- libavcodec/bitstream.h	(revision 17735)
> >> >> +++ libavcodec/bitstream.h	(working copy)
> >> >> @@ -707,6 +707,14 @@
> >> >>  }
> >> >>  
> >> >>  /**
> >> >> + * reads 0-32 bits as a signed integer.
> >> >> + */
> >> >> +static inline int get_sbits_long(GetBitContext *s, int n) {
> >> >> +    int val = get_bits_long(s, n);
> >> >> +    return NEG_SSR32(val<<(32-n), n);
> >> >> +}
> >> > 
> >> > This is obfuscation IMO.  I'll prepare a patch adding a sign extend
> >> > function if nobody beats me to it.  What is the preferred return type,
> >> > int or int32_t?  I'd say int, but maybe there's a good reason for
> >> > something else.
> >> 
> >> Thanks M?ns. Here is a patch using the new function in mathops.h.
> >
> > was there another file that needs get_sbits_long() ? if not this code
> > belongs in flac*
> 
> Even if there isn't one now, I think this function belongs in
> bitstream.h.  It is the logical place for it to be, and it should be
> easy to find should someone need it in the future, which it not a
> far-fetched prospect.

hmm, then justins patch is ok

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090303/48482274/attachment.pgp>