[FFmpeg-devel] [PATCH] ALAC Encoder

[Jai Menon]

Hi,

On Monday 18 Aug 2008 2:38:24 am 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:
> > > 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.
>

Newer version of the previous patch with some redundancy removed.

Regards,

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