On Tue, Apr 5, 2011 at 1:44 AM, Kostya <[email protected]> wrote: > On Tue, Apr 05, 2011 at 01:05:23AM -0600, Nathan Caldwell wrote: >> + pctx->pe.min = 8192.0f; /* FIXME: 0.8 * 10 * FRAME_LENGTH_LONG * >> bandwidth / (sample_rate / 2) */ >> + pctx->pe.max = 12288.0f; /* FIXME: 1.2 * 10 * FRAME_LENGTH_LONG * >> bandwidth / (sample_rate / 2) */ > > what prevents you from using those numbers in fixme?
Our encoder doesn't have any concept of a cutoff, thus our bandwidth == sample rate / 2. If/when a cutoff is added I would like to keep track of what needs to be adjusted. >> + /* -25dB -1dB */ >> + coeff->min_snr = av_clipf(1.0f / minsnr, 3.1622776e-3f, >> 7.9432821e-1f); > > this comment looks slightly misaligned, also something like "min SNR should be > in range -1..-25 dB" sounds better It's aligned with a fixed-width font... I have no problem changing it, though. >> + ctx->fill_level += ctx->frame_bits - bits; >> + ctx->fill_level = av_clip(ctx->fill_level, 0, size); >> + fill_level = av_clipf((float)ctx->fill_level / size, clip_low, >> clip_high); >> + clipped_pe = av_clipf(pe, ctx->pe.min, ctx->pe.max); >> + bit_save = (fill_level + bitsave_add) * bitsave_slope; >> + assert(bit_save <= 0.3f && bit_save >= -0.05000001f); >> + bit_spend = (fill_level + bitspend_add) * bitspend_slope; >> + assert(bit_spend <= 0.5f && bit_spend >= -0.1f); > > I suspect those asserts may trigger quite often with current encoder, don't > they? If yes then something like reset should be done instead. They don't for me. Also, I wasn't sure on the correct way to do the bit save -0.05 assert. Using "-0.05" results in the assert triggering if bit_save == "-0.05", which it shouldn't. > [the rest looks ok] Thanks for the quick review! -- -Nathan Caldwell _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
