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

Peter Belkner pbelkner at snafu.de
Sun Apr 17 09:45:31 CEST 2011

On 16.04.2011 23:36, Reimar Döffinger wrote:
> In reality not really.
> Running the a conversion even if 99.9% of all cases don't need
> it doesn't seem reasonable.

The type conversion is an implicit goody. The main reason for the loops 
is to split interleaved into seperated channels.

I rewrote the patch (cf. attachment):

    * 32 bit mode is only activated if (in that order)
         1. int is 4 byte (uses lame_encode_buffer_int),
         2. long is 4 byte (uses lame_encode_buffer_long2).
         3. Otherwise 32 bit mode is not available.
    * Indroduced MP3lame_encode_close_internal() for consistent clean-up
      in case of an error.
    * The patch doesn't assume any longer that the VBR patch is already
      applied (i.e. the patch can be applied as it is).

> I don't mind much how it is done, but I'd probably just make it
> fail (and preferable not advertise the feature in the first place)
> when int isn't 4 bytes.

What's your OS? Wich lame version are you using? What's happening if you 
run a lame instance linked to the same libmp3lame.a as FFmpeg on the 
same data?

>> @@ -151,7 +167,13 @@ static int MP3lame_encode_frame(AVCodecContext *avctx,
>>       /* lame 3.91 dies on '1-channel interleaved' data */
>> -    if(data){
>> +    if(NULL == data){
> I'd very much appreciate without such pointless changes

That's not a "pointless change".

The original code had flushing (i.e. NULL == data) as the final case. 
Because this case is common to the 16 and 32 bit modes I moved it to the 
top and hence had to invert the condition.

That the code block guarded by the condition is moved may be not obvious 
by only looking at the patch.

>> +        lame_result= lame_encode_flush(
>> +    }else if(AV_SAMPLE_FMT_S16 == avctx->sample_fmt){
>> +            lame_result = lame_encode_buffer_long2(
>> +        } else {
> And a bit more consistent way of placing spaces.
> I know it can seem quite pointless, but in the long run
> such small things also make the difference between
> code most people like working with or code they
> seriously consider running away from it screaming.

   1. FFmpeg's formatting style in a lot of aspects drastically differs
      from my own.
   2. I tried to format as close as possible to what I've found.
   3. However, what I've found is inconsistent in al lot of subtile aspects.
   4. Please advise.

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: libmp3lame-32-bit-v2.patch
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110417/4ff42644/attachment.ksh>

More information about the ffmpeg-devel mailing list