[FFmpeg-devel] [PATCH] ALS decoder

[Diego Biurrun]

On Sun, Sep 06, 2009 at 01:52:57PM +0200, Thilo Borgmann wrote:
> Michael Niedermayer schrieb:
> > On Fri, Sep 04, 2009 at 09:48:46PM +0200, Thilo Borgmann wrote:
> >> Michael Niedermayer schrieb:
> >>> On Thu, Sep 03, 2009 at 09:44:25PM +0200, Thilo Borgmann wrote:
> >>>> Revision 16 attached.
> >>>> +/** Decodes an ALS frame.
> >>>> + */
> >>>> +static int decode_frame(AVCodecContext *avctx,
> >>>> +                        void *data, int *data_size,
> >>>> +                        AVPacket *avpkt)
> >>>> +{
> >>>> +    ALSDecContext *ctx       = avctx->priv_data;
> >>>> +    ALSSpecificConfig *sconf = &ctx->sconf;
> >>>> +    const uint8_t *buffer    = avpkt->data;
> >>>> +    int buffer_size          = avpkt->size;
> >>>> +    int invalid_frame, size;
> >>>> +    unsigned int c, sample, ra_frame, bytes_read, shift;
> >>>> +
> >>>> +    init_get_bits(&ctx->gb, buffer, buffer_size * 8);
> >>>> +    ra_frame = sconf->ra_distance && !(ctx->frame_id % sconf->ra_distance);
> >>> is it true that if ra_distance == 0 then no frame is a ra frame?
> >>> seems odd
> >>>
> >> As mentioned above, this is the specified behaviour.
> >> And that is the reason for using av_mallocz() for the raw_buffer buffer,
> >> to have fake 0's for prediction in the carryover samples from frame "-1".
> > 
> > does the spec say they should be 0 ?
> 
> This whole case is not mentioned explicitly in the spec (as fas as I can
> see). But this is produced by the reference encoder and can be followed
> implicitly - before the first frame, there is silence (0). Until we
> figured that out, we had some "randomly" broken files because of
> non-zero memory chunks allocated for the first carryover buffer.
> 
> If we would not implicitly assume this from the specs and treat the very
> first frame to be a RA frame (keyframe) in all cases, decoding fails.

This is clearly something that you need to document, not mention in
passing during the review process.

Diego