On Tue, Dec 1, 2015 at 7:35 AM, Claudio Freire <klaussfre...@gmail.com> wrote: > On Mon, Nov 30, 2015 at 1:04 PM, Hendrik Leppkes <h.lepp...@gmail.com> wrote: >> On Mon, Nov 30, 2015 at 4:50 PM, Claudio Freire <klaussfre...@gmail.com> >> wrote: >>> On Mon, Nov 30, 2015 at 12:27 PM, Rostislav Pehlivanov >>> <atomnu...@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 > <atomnu...@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@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel