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

Paul Kelly paul
Sun Feb 10 20:16:41 CET 2008


On Sat, 9 Feb 2008, Paul Kelly wrote:

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

Here it is now. The first patch only rearranges the flag-setting code in 
wmaenc.c and does not add anything. The second patch adds the new flag, 
sets it as the default but also adds the warning in wmaenc.c that it will 
not be used. Is this OK?

Paul
-------------- next part --------------
A non-text attachment was scrubbed...
Name: flags2-2.patch
Type: text/x-diff
Size: 941 bytes
Desc: 
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080210/234d4ba4/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bitres-2.patch
Type: text/x-diff
Size: 4103 bytes
Desc: 
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080210/234d4ba4/attachment-0001.patch>



More information about the ffmpeg-devel mailing list