[FFmpeg-devel] [PATCH] ALAC Encoder

Jai Menon realityman
Mon Aug 18 21:46:50 CEST 2008


Hi,

On Tuesday 19 Aug 2008 1:09:07 am Jai Menon wrote:
> Hi,
>
> On Monday 18 Aug 2008 3:46:23 am Michael Niedermayer wrote:
> > On Mon, Aug 18, 2008 at 02:38:24AM +0530, Jai Menon wrote:
> > > Hi,
> > >
> > > On Sunday 17 Aug 2008 5:17:52 pm Michael Niedermayer wrote:
> > > > On Sun, Aug 17, 2008 at 11:17:10AM +0530, Jai Menon wrote:
>
> [...]
>
> > > @@ -79,6 +122,205 @@
> > >      put_bits(&s->pbctx, 32, s->avctx->frame_size);          // No. of
> > > samples in the frame }
> > >
> > > +static void calc_predictor_params(AlacEncodeContext *s, int ch)
> > > +{
> > > +    int32_t coefs[MAX_LPC_ORDER][MAX_LPC_ORDER];
> > > +    int shift[MAX_LPC_ORDER];
> > > +    int opt_order;
> > > +
> > > +    opt_order = ff_lpc_calc_coefs(&s->dspctx, s->sample_buf[ch],
> > > s->avctx->frame_size, DEFAULT_MIN_PRED_ORDER, DEFAULT_MAX_PRED_ORDER, +
> > >                                 ALAC_MAX_LPC_PRECISION, coefs, shift,
> > > 1, ORDER_METHOD_EST, ALAC_MAX_LPC_SHIFT, 1); +
> > > +    s->lpc[ch].lpc_order = opt_order;
> > > +    s->lpc[ch].lpc_quant = shift[opt_order-1];
> > > +    memcpy(s->lpc[ch].lpc_coeff, coefs[opt_order-1],
> > > opt_order*sizeof(int)); +}
> > > +
> >
> > I think this should be using AVCodecContext.min/max_prediction_order
>
> Done.
>
> > > +    if(best == 0) {
> > > +        return ALAC_CHMODE_LEFT_RIGHT;
> > > +    } else if(best == 1) {
> > > +        return ALAC_CHMODE_LEFT_SIDE;
> > > +    } else if(best == 2) {
> > > +        return ALAC_CHMODE_RIGHT_SIDE;
> > > +    } else {
> > > +        return ALAC_CHMODE_MID_SIDE;
> > > +    }
> > > +}
> >
> > i think best could simply be returned
>
> Done.
>
> > > +
> > > +static void alac_stereo_decorrelation(AlacEncodeContext *s)
> > > +{
> > > +    int32_t *left = s->sample_buf[0], *right = s->sample_buf[1];
> > > +    int i, mode, n = s->avctx->frame_size;
> > > +
> > > +    mode = estimate_stereo_mode(left, right, n);
> > > +
> > > +    if(mode == ALAC_CHMODE_LEFT_RIGHT) {
> > > +        s->interlacing_leftweight = 0;
> > > +        s->interlacing_shift = 0;
> > > +        return;
> > > +    }
> > > +
> > > +    if(mode == ALAC_CHMODE_LEFT_SIDE) {
> > > +        for(i=0; i<n; i++) {
> > > +            right[i] = left[i] - right[i];
> > > +        }
> > > +        s->interlacing_leftweight = 1;
> > > +        s->interlacing_shift = 0;
> > > +
> > > +    } else {
> > > +        int32_t tmp;
> > > +        for(i=0; i<n; i++) {
> > > +            tmp = left[i];
> > > +            left[i] = (tmp + right[i]) >> 1;
> > > +            right[i] = tmp - right[i];
> > > +        }
> > > +        s->interlacing_leftweight = 1;
> > > +        s->interlacing_shift = 1;
> > > +    }
> >
> > i think 1 mode is missing
> > also it could be
> > if
> > else if
> > else if
> > else
> >
> > instead of
> > if
> >     return
> > if
> > else
> >
> > which would be cleaner IMHO
>
> Right/Side mode added and also cleaned up the code structure.
>
> > > +}
> > > +
> > > +static void alac_linear_predictor(AlacEncodeContext *s, int ch)
> > > +{
> > > +    int i;
> > > +    LPCContext lpc = s->lpc[ch];
> > > +
> > > +    if(lpc.lpc_order == 31) {
> > > +        s->predictor_buf[0] = s->sample_buf[ch][0];
> > > +
> > > +        for(i=1; i<s->avctx->frame_size; i++)
> > > +            s->predictor_buf[i] = s->sample_buf[ch][i] -
> > > s->sample_buf[ch][i-1]; +
> > > +        return;
> > > +    }
> > > +
> > > +    // generalised linear predictor
> > > +
> > > +    if(lpc.lpc_order > 0) {
> > > +        int32_t *samples  = s->sample_buf[ch];
> > > +        int32_t *residual = s->predictor_buf;
> > > +
> > > +        // generate warm-up samples
> > >
> > > +        i = lpc.lpc_order;
> > > +        residual[0] = samples[0];
> > > +        while(i > 0) {
> > > +            residual[i] = samples[i] - samples[i-1];
> > > +            i--;
> > > +        }
> >
> > this also can be changed to a for() loop
> > alternatively residual could be droped and the stuff could all be done
> > in place in samples but this may be tricky
>
> Changed to a 'for' loop. I'll look into the in-place thing after the basic
> encoder is committed ;-)
>
> > > +        // perform lpc on remaining samples
> > > +        for(i = lpc.lpc_order + 1; i < s->avctx->frame_size; i++) {
> > > +            int sum = 0, res_val, j;
> > > +
> > > +            for (j = 0; j < lpc.lpc_order; j++) {
> > > +                sum += (samples[lpc.lpc_order-j] - samples[0]) *
> > > +                        lpc.lpc_coeff[j];
> > > +            }
> > > +            sum += (1 << (lpc.lpc_quant - 1));
> > > +            sum >>= lpc.lpc_quant;
> > > +            sum += samples[0];
> > > +            residual[i] = samples[lpc.lpc_order+1] - sum;
> > > +            res_val = residual[i];
> > > +
> > > +            if(res_val) {
> > > +                int index = lpc.lpc_order - 1;
> > > +                int neg = (res_val < 0);
> > > +
> > > +                while(index >= 0 && (neg ? (res_val < 0):(res_val >
> > > 0))) { +                    int val = samples[0] -
> > > samples[lpc.lpc_order - index]; +                    int sign = (val ?
> > > FFSIGN(val) : 0); +
> > > +                    if(neg)
> > > +                        sign*=-1;
> > > +
> > > +                    lpc.lpc_coeff[index] -= sign;
> > > +                    val *= sign;
> > > +                    res_val -= ((val >> lpc.lpc_quant) *
> > > +                            (lpc.lpc_order - index));
> > > +                    index--;
> > > +                }
> > > +            }
> > > +            samples++;
> > > +        }
> > > +    }
> > > +}
> > > +
> > >
> > > +static void alac_entropy_coder(AlacEncodeContext *s)
> > > +{
> > > +    unsigned int history = s->rc.initial_history;
> > > +    int sign_modifier = 0, i, k;
> > > +    int32_t *samples = s->predictor_buf;
> > > +
> > > +    for(i=0;i < s->avctx->frame_size;) {
> > > +        int x;
> > > +
> > > +        k = av_log2((history >> 9) + 3);
> > > +
> > > +        x = -2*(*samples)-1;
> > > +        x ^= (x>>31);
> > > +
> > > +        samples++;
> > > +        i++;
> > > +
> > > +        encode_scalar(s, x - sign_modifier, k, s->write_sample_size);
> > > +
> > >
> > > +        history += x * s->rc.history_mult
> > > +                   - ((history * s->rc.history_mult) >> 9);
> >
> > not sure if its worth but this could be simplified to:
> >
> > history -= (((history - (x<<9)) * s->rc.history_mult) >> 9);
> > (assuming things dont overflow)
>
> There is no substantal change when verified using start/stop_timer. I
> prefer this as it looks clean and nicely split up :-)
>
> > > +
> > > +        sign_modifier = 0;
> > > +        if(x > 0xFFFF)
> > > +            history = 0xFFFF;
> > > +
> > > +        if((history < 128) && (i < s->avctx->frame_size)) {
> > > +            unsigned int block_size = 0;
> > > +
> > >
> > > +            sign_modifier = 1;
> >
> > unused
>
> It is very much used actually. After we have coded a block of zeros, the
> next scalar is non_zero and this sign modifier is applied to that.
>
> > > +
> > > +    init_put_bits(pb, frame, buf_size);
> > > +
> > > +verbatim:
> >
> > the label can be moved before the init_put_bits() which makes the second
> > call to it before the goto uneeded
>
> Indeed. Modified accordingly.
>
> > [...]
>
> New patch attached. Thanks for the review and help with the decorrelation
> stuff.
>

Sorry, wrong patch. Correct one attached.

Regards,

Jai Menon
-------------- next part --------------
A non-text attachment was scrubbed...
Name: alac_encoder.patch
Type: text/x-diff
Size: 8921 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080819/2a8a0d93/attachment.patch>



More information about the ffmpeg-devel mailing list