[FFmpeg-devel] Patch - Allow disabling of bit reservoir when encoding MP3 audio

[Paul Kelly]

On Sat, 9 Feb 2008, Michael Niedermayer wrote:

> On Sat, Feb 09, 2008 at 03:28:16PM +0000, Paul Kelly wrote:
>> On Fri, 8 Feb 2008, Michael Niedermayer wrote:
>>
>>> On Fri, Feb 08, 2008 at 09:57:47PM +0000, Paul Kelly wrote:
>>>
>> [...]
>>> Everything else is disabled by default and has enable flags.
>>> I definitly want the flag to be an enable flag. The default is a seperate
>>> issue, we might choose to leave it enabled.
>>> "-flags2 -reservoir" would disable it
>>> "-flags2 +reservoir" would enable it
>>> this is much cleaner than a +disable_reservoir
>>>
>> [...]
>>>> So I'm not sure what to do about the WMA encoder. Would it be acceptable
>>>> to put in some code that prints a warning that the bit reservoir will not
>>>> be used even though it is specified? I mean just so there is some
>>>> reference to this AVCodecContext disable bit reservoir flag there so
>>>> someone hacking the code in the future will be reminded to use it.
>>>
>>> If you want to add that ...
>>
>> OK, I've attached two patches for consideration. The first, flags2.patch,
>> re-arranges the code for setting WMA flags in wmaenc.c to make it clearer.
>> It seemed to be a copy and paste of the code in wmadec.c, otherwise it
>> doesn't make much sense. It should be clear from the patch my understanding
>> of what the code is supposed to do, and I hope my changes are acceptable.
>> This patch also adds the warning if the bit reservoir flag is set, and
>> unsets it.
>
> Rearanging is a cosmetical change and must be in a seperate patch from
> functional changes.

But it is - are you saying the change to add the error message should go 
in the second patch? I thought, since the value of use_bit_reservoir is 
hard-coded to 0 - the warning will never be printed, so there is no change 
in functionality.
But yes you have a point. I'll send another version tomorrow!

Paul