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

Justin Ruggles justin.ruggles
Tue Mar 3 02:08:05 CET 2009


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

It could be used in 1 place in alac.c:547:
audiobits = get_bits_long(&alac->gb, alac->setinfo_sample_size);
audiobits = extend_sign32(audiobits, alac->setinfo_sample_size);

additionally, that extend_sign32() function, which is also used in other
places in alac.c, could be replaced by calls to sign_extend() in mathops.h.

-Justin





More information about the ffmpeg-devel mailing list