[FFmpeg-devel] [PATCH] libmp3lame: 32 bit encoding

[Peter Belkner]

On 16.04.2011 17:14, Michael Niedermayer wrote:
> On Sat, Apr 16, 2011 at 12:25:30PM +0200, Peter Belkner wrote:
>    
>> Find a revisewd patch below.
>>
>> On 16.04.2011 11:22, Carl Eugen Hoyos wrote:
>>      
>>> Peter Belkner<pbelkner<at>   snafu.de>   writes:
>>>
>>>
>>>        
>>>> the patch enables libmp3lame to encode at 32 bit. This is especially
>>>> useful if audio is fed from SoX (http://sox.sourceforge.net/) into
>>>> FFmpeg because SoX works at 32 bit and hence no additional
>>>> re-quantization is needed.
>>>>          
> nice feature
>    

Thanks.

>
>    
>>>>
>>>>          
>>> Does every supported lame version support 32bit encoding?
>>>
>>>        
>>  From where I know what are the supported lame versions? Are there any
>> according symbols defined from config?
>>      
> our configure says:
>   enabled libmp3lame&&  require  "libmp3lame>= 3.98.3" lame/lame.h lame_set_VBR_quality -lmp3lame
>
>   does 3.98.3 support 32bit?
> if not either the configure check must be updated or the code must be
> conditional
>
>
>    

For what ever reason 3.89.3 is not listed at their download page 
(http://sourceforge.net/projects/lame/files/lame/). Had instead look at 
3.98.2 and it has lame_encode_buffer_long2() declared in "lame .h".

>    
>>>        
>>>> +    if (AV_SAMPLE_FMT_S16 == avctx->sample_fmt) {
>>>> +        s->s32_data.left = NULL;
>>>> +        s->s32_data.right = NULL;
>>>>
>>>>          
>>> This looks unneeded.
>>>
>>>        
>> I removed this assuming that the memory block priv_data is pointing to
>> is zeroed out.
>>      
>>>
>>>        
>>>> +    } else {
>>>> +        s->s32_data.left = av_malloc(avctx->frame_size * sizeof(long));
>>>>
>>>>          
>>> And I suspect this is wrong. (long has different sizes on different platforms,
>>> or am I wrong?)
>>>
>>>        
>> I think this is correct independent on whether "long" is 32 or 64 bits
>> because these are the buffers given to lame. The following is from
>> "lame.h":
>>
>>     /* Same as lame_encode_buffer_long(), but with correct scaling.
>>       * !! NOTE: !! data must still be scaled to be in the same range as
>>       * type 'long'.   Data should be in the range:  +/- 2^(8*size(long)-1)
>>       *
>>       */
>>     int CDECL lame_encode_buffer_long2(
>>              lame_global_flags*  gfp,           /* global context
>>     handle         */
>>              const long     buffer_l [],       /* PCM data for left
>>     channel     */
>>              const long     buffer_r [],       /* PCM data for right
>>     channel    */
>>              const int           nsamples,      /* number of samples per
>>     channel */
>>              unsigned char*      mp3buf,        /* pointer to encoded
>>     MP3 stream */
>>              const int           mp3buf_size ); /* number of valid
>>     octets in this
>>                                                      stream
>>          */
>>
>> Assuming lame is compiled on a system with a 64 bit long than propably I
>> have to provide according buffers.
>>
>> On the other hand I corrected casting the frame buffer provided by
>> FFmpeg to (int32_t *).
>>      
> your mono code looks like it would fail with 64bit long
>    

Fixed it with it's own copy loop, cf. revised patch below.

Peter
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: libmp3lame-32-bit-revised-2.patch
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110416/e910b59a/attachment.ksh>