[FFmpeg-devel] [PATCH] AAC encoder: improve SF range utilization

Hendrik Leppkes h.leppkes at gmail.com
Tue Dec 1 21:42:51 CET 2015


On Tue, Dec 1, 2015 at 7:35 AM, Claudio Freire <klaussfreire at gmail.com> wrote:
> On Mon, Nov 30, 2015 at 1:04 PM, Hendrik Leppkes <h.leppkes at gmail.com> wrote:
>> On Mon, Nov 30, 2015 at 4:50 PM, Claudio Freire <klaussfreire at gmail.com> wrote:
>>> On Mon, Nov 30, 2015 at 12:27 PM, Rostislav Pehlivanov
>>> <atomnuker at gmail.com> wrote:
>>>> On Sun, 2015-11-29 at 16:54 -0300, Claudio Freire wrote:
>>>>> Before pushing this, I'd like some feedback,
>>>>> especially about
>>>>> the implementation of point 3. I'm not sure the AAC encoder
>>>>> setting the cutoff in the encoder context like this is legal or
>>>>> desirable.
>>>> I think setting the cutoff is necessary. You can't avoid holes and yet
>>>> keep a full bandwidth as you decrease bits unless you like the sound of massive quantization errors.
>>>
>>> My point was more about whether a codec should write into the context
>>> struct like that or not. Basically an API technicality that's unclear
>>> to me.
>>>
>>
>> It seems slightly odd to write into that variable, since thats the
>> cutoff the user requests.
>> Maybe you can use/write into a private variable instead? ac3enc seems
>> to use a private variable for similar purposes.
>
> Attached patch does that.
>
> I'm far more comfortable with this implementation, so if all agree, I'll push.

This part looks good to me now, thanks! I'll leave the other parts for
atomnuker to judge. ;)

>
> On Mon, Nov 30, 2015 at 2:20 PM, Rostislav Pehlivanov
> <atomnuker at gmail.com> wrote:
>> On Mon, 2015-11-30 at 12:50 -0300, Claudio Freire wrote:
>>> Also I don't see how a static var would help or even be correct here.
>>> Perhaps you meant something else?
>> static uint8_t cond1 = param1 && param2;
>> static uint8_t cond2 = param3 && !param4;
>> ...etc
>> return cond1 && cond2;
>
> I didn't do that since it would not short circuit some terms that
> ought to be short-circuited.
>
> But I did split lines and it does read better. In fact, maybe that was
> all that was needed?
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


More information about the ffmpeg-devel mailing list