[FFmpeg-devel] [PATCH] ALAC Encoder

[Jai Menon]

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:
> > Hi,
> >
> > On Sunday 17 Aug 2008 8:05:14 am Michael Niedermayer wrote:
> > > On Sun, Aug 17, 2008 at 04:14:43AM +0530, Jai Menon wrote:
> > > > Hi,
> > > >
> > > > The attached ALAC encoder was written as part of GSoC and mentored by
> > > > Justin Ruggles. I'm posting it for inclusion into FFmpeg-svn.
> > >
> > > [...]
> > >
> > > > +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];
> > > >
> > > > +        i = s->avctx->frame_size - 1;
> > > > +        while(i > 0) {
> > > > +            s->predictor_buf[i] = s->sample_buf[ch][i] -
> > > > s->sample_buf[ch][i-1];
> > > > +            i--;
> > > > +        }
> > >
> > > i suspect that 0 .. n is faster than n .. 0
> >
> > I'll need a temp variable in that case.   Is that okay?
>
> why?
>
> for(i=1; i<s->avctx->frame_size; i++)
>     s->predictor_buf[i] = s->sample_buf[ch][i] - s->sample_buf[ch][i-1];
>
> looks fine to me ...
>

Yeah, that was my mistake.
Changed.

> > > [...]
> > >
> > > > +    if(buf_size < s->max_coded_frame_size) {
> > > > +        av_log(avctx, AV_LOG_ERROR, "buffer size is too small\n");
> > > > +        return -1;
> > > > +    }
> > > > +
> > > > +    init_put_bits(pb, frame, buf_size);
> > > > +
> > > > +    if(s->compression_level == 0) {
> > > > +        // Verbatim mode
> > > > +        int16_t *samples = data;
> > > > +        write_frame_header(s, 1);
> > > > +        for(i=0; i<avctx->frame_size*s->channels; i++) {
> > > > +            put_sbits(pb, 16, *samples++);
> > > > +        }
> > > > +    } else {
> > > > +        init_sample_buffers(s, data);
> > > > +        write_frame_header(s, 0);
> > > > +        write_compressed_frame(s);
> > > > +    }
> > > > +
> > > > +    put_bits(pb, 3, 7);
> > > > +    flush_put_bits(pb);
> > > > +    out_bytes = put_bits_count(pb) >> 3;
> > > > +
> > > > +    if(out_bytes > s->max_coded_frame_size) {
> > >
> > > At this point it is possible that the encoder wrote over the end of the
> > > array, the check farther up should ensure that there really is enough
> > > space available even for larger then optimal frames
> >
> > I'm not sure If I understand this comment correctly. We already check to
> > see if buf_size < max_coded_frame_size. So it would not write past the
> > end. This check here would take care of reencoding in verbatim mode if
> > actual output was > max_coded_frame_size. It handles this case similar to
> > the flac encoder.
>
> well, if actual output was > max_coded_frame_size but actual buffer was
> just max_coded_frame_size, then i would assume that some of the output is
> after the buffers end.
>
> [...]

Modified according to comment.

Other review comments also addressed. Patch against FFmpeg svn is attached.


Regards,

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