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

[Måns Rullgård]

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.

-- 
M?ns Rullg?rd
mans at mansr.com