[FFmpeg-cvslog] r14843 - trunk/libavcodec/alacenc.c

[Jai Menon]

Hi,

On Tuesday 19 Aug 2008 11:55:06 pm Ramiro Polla wrote:
> ramiro wrote:
> > Author: ramiro
> > Date: Tue Aug 19 18:07:58 2008
> > New Revision: 14843
> >
> > Log:
> > Import more OKed parts of ALAC encoder from GSoC repo.
>
> [...]
>
> > @@ -226,6 +295,32 @@ static av_cold int alac_encode_init(AVCo
> >          AV_WB8(alac_extradata+20, s->rc.k_modifier);
> >      }
> >
> > +    if(avctx->min_prediction_order >= 0) {
> > +        if(avctx->min_prediction_order < MIN_LPC_ORDER ||
> > +            avctx->min_prediction_order > MAX_LPC_ORDER) {
>
>                ^
>
> This could be more vertically aligned like the one below. (maybe it was
> me who botched the patch, I don't remember).
>

Hmm..slight indentation difference. I'll apply a cosmetic.

> > +            av_log(avctx, AV_LOG_ERROR, "invalid min prediction order:
> > %d\n", avctx->min_prediction_order); +                return -1;
> > +        }
> > +
> > +        s->min_prediction_order = avctx->min_prediction_order;
> > +    }
> > +
> > +    if(avctx->max_prediction_order >= 0) {
> > +        if(avctx->max_prediction_order < MIN_LPC_ORDER ||
> > +           avctx->max_prediction_order > MAX_LPC_ORDER) {
>
>                                              ^^^^^^^^^^^^^
>
> Sorry for only noticing now, but shouldn't these be ALAC_***_LPC_ORDER?
>

Yeah, it should be ALAC_MAX_LPC_ORDER. MIN_LPC_ORDER is the same
Changed.
 
> > +            av_log(avctx, AV_LOG_ERROR, "invalid max prediction order:
> > %d\n", avctx->max_prediction_order); +                return -1;
> > +        }
> > +
> > +        s->max_prediction_order = avctx->max_prediction_order;
> > +    }
> > +
> > +    if(s->max_prediction_order < s->min_prediction_order) {
> > +        av_log(avctx, AV_LOG_ERROR, "invalid prediction orders: min=%d
> > max=%d\n", +               s->min_prediction_order,
> > s->max_prediction_order); +        return -1;
> > +    }
>
> And it seems all this part was taken from flacenc.c. This is also useful
> for MLP. Can't this reading of avctx and populating a min/max predictor
> order struct be factored out into lpc.[ch]?
>
> Ramiro Polla

Now that you mention, I'll look into the refactoring..

Regards,

Jai Menon