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

[Justin Ruggles]

Michael Niedermayer wrote:
> 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

applied.