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

[Michael Niedermayer]

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


>>>      
>> 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



>>
>>> +    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



[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110416/fc85d4e2/attachment.asc>