[FFmpeg-devel] [PATCH] s302menc: Add S302M encoder.

[Reimar Döffinger]

On Tue, Apr 10, 2012 at 02:45:17PM -0700, Thierry Foucu wrote:
> +    if (buf_size < s302m_frame_size + AES3_HEADER_LEN) {
> +      av_log(avctx, AV_LOG_ERROR, "Buffer too small %d to encode %d Bytes.\n",
> +             buf_size, s302m_frame_size + AES3_HEADER_LEN);
> +      return -1;
> +    }
> +
> +    PutBitContext pb;

Mixing declarations and code. Please look out for compiler warnings.

> +    init_put_bits(&pb, o, buf_size*8);
> +    put_bits(&pb, 16, s302m_frame_size);  // Frame size
> +    put_bits(&pb, 2, (avctx->channels - 2) >> 1);  // Number channels
> +    put_bits(&pb, 8, 0);  // Channel id
> +    put_bits(&pb, 2, 0);  // bits per samples (16bit)
> +    put_bits(&pb, 4, 0);  // alignments
> +    flush_put_bits(&pb);

Seems almost a bit like overkill to use put_bits for these, compared
to e.g. two bytestream_put_be16.

> +    num_samples = S302M_NUMBER_SAMPLES * avctx->channels;
> +    for (; num_samples > 0; num_samples -= avctx->channels) {
> +        vucf = (s->framing_index == 0)? 0x10 : 0;
> +        for (channels = 0; channels < avctx->channels; channels += 2) {
> +            vucf = (s->framing_index == 0)? 0x10 : 0;

Why are you setting vucf twice?

> +    .encode         = s302m_encode_frame,

Please implement a encode2 function instead, "encode" is deprecated.
Also a regression test would be very good thing to have.