On Tue, 12 Apr 2011, Ronald S. Bultje wrote:
> On Tue, Apr 12, 2011 at 5:58 PM, Martin Storsjö <mar...@martin.st> wrote:
> > ---
> > libavcodec/libopencore-amr.c | 18 ++++++++----------
> > libavcodec/libvo-amrwbenc.c | 17 ++++++-----------
> > 2 files changed, 14 insertions(+), 21 deletions(-)
> [..]
> > -static const char nb_bitrate_unsupported[] =
> > - "bitrate not supported: use one of 4.75k, 5.15k, 5.9k, 6.7k, 7.4k,
> > 7.95k, 10.2k or 12.2k\n";
> [..]
> > + av_log(log_ctx, AV_LOG_ERROR, "bitrate not supported: use one of ");
> > + for (i = 0; i < 8; i++)
> > + av_log(log_ctx, AV_LOG_ERROR, "%d%s", rates[i].rate,
> > + i < 7 ? ", " : "\n");
>
> This isn't a good idea. 2 reasons:
> - 1, what if threading is enabled? Threads could basically log
> interleaved, and that would lead to garbage on the terminal
I'm not sure if there actually is any guarantee that each log line will be
atomic even if writing with a single av_log(), but there's at least less
risk for messups. Changed
> - 2, you're printing in decimals instead of in k units now.
Changed.
> What I'd prefer is to keep the nice output that you have now, maybe
> put it in a temporary buffer before printing to the terminal (I know,
> av_hex_dump_log() has that issue also and I have a patch for that
> somewhere). Then lastly, if the bitrate is not right, simply select
> the best one. Don't error out. Just print a warning. And then
> continue.
Changed not to error out but just print a warning, and not to recheck and
re-warn for each frame unless the user actually changed the bitrate
inbetween, the rest of the patchset coming up.
// Martin
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
