[FFmpeg-cvslog] Add support for AMR-WB encoding via libvo-amrwbenc

[Martin Storsjö]

Hi Reimar,

On Tue, 12 Apr 2011, Reimar Döffinger wrote:

> On 12 Apr 2011, at 03:50, git at videolan.org (Martin Storsjö) wrote:
> > +static const char wb_bitrate_unsupported[] =
> > +    "bitrate not supported: use one of 6.6k, 8.85k, 12.65k, 14.25k, 15.85k, "
> > +    "18.25k, 19.85k, 23.05k, or 23.85k\n";
> 
> Huh, what is that?

A reused error message, but it's a quite weird way of doing it.

> > +    /* make the correspondance between bitrate and mode */
> > +    AMRWB_bitrates rates[] = { { 6600, 0},
> > +                               { 8850, 1},
> > +                               {12650, 2},
> > +                               {14250, 3},
> > +                               {15850, 4},
> > +                               {18250, 5},
> > +                               {19850, 6},
> > +                               {23050, 7},
> > +                               {23850, 8}, };
> 
> Quite a few compilers will actually make a on-stack copy for that.
> It really should static const.
> Also it's quite silly with the second value being the index.
> And if that function printed the message that string array wouldn't be necessary.

Yeah, this can be improved.

All of this is very old code from the old libamr wrapper, and now when I 
look at it a bit closer, I find even more similar weirdness in the 
libopencore-amr wrapper. I'll try to clean it all up.

> > +static int amr_wb_encode_frame(AVCodecContext *avctx,
> > +                               unsigned char *frame/*out*/,
> > +                               int buf_size, void *data/*in*/)
> > +{
> > +    AMRWBContext *s = avctx->priv_data;
> > +    int size;
> > +
> > +    if ((s->mode = getWBBitrateMode(avctx->bit_rate)) < 0) {
> > +        av_log(avctx, AV_LOG_ERROR, wb_bitrate_unsupported);
> > +        return -1;
> > +    }
> 
> Failing in the middle of an encode and just because we can't match the 
> bitrate seems a bit extreme and also not very consistent with other 
> codecs that might not take the bitrate very seriously in the first 
> place. From a user perspective it also means the codec can't be used 
> without special-case code.

The bitrate is checked during the init, too, so if you set it to an 
incorrect value, it will error out already at that point, but you're 
allowed to change between frames. Do you have a better suggestion on how 
to handle the case if the caller sets it to an unsupported rate during the 
encoding, while starting out at a supported bitrate?

// Martin