[FFmpeg-devel] [PATCH] MLP/TrueHD decoder

[Ian Caulfield]

Hi,

New version attached, hopefully addressing most of your comments.

On Dec 10, 2007 11:20 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>
> On Tue, Dec 04, 2007 at 03:50:50PM +0000, Ian Caulfield wrote:
> [...]
> > +/** Initialize the decoder. */
> > +
> > +static int mlp_decode_init(AVCodecContext *avctx)
> > +{
> > +    MLPDecodeContext *m = avctx->priv_data;
> > +
> > +    init_static();
> > +    m->avctx = avctx;
> > +    memset(m->lossless_check_data, 0xff, sizeof(m->lossless_check_data));
> > +    return 0;
> > +}
>
> is there a special reason why init_static() is a seperate function?

Not really - I can fold it in if you'd like.

> [...]
> > +    dprintf(m->avctx, "Filter %d, order %d\n", filter, order);
>
> please merge these hundreads of dprintf() in few proper
> if(debug & FF_DEBUG_*)
>     av_log()
>
> or dprintf() instead of av_log() for the more obscure/less usefull ones
>
> having this debug stuff interleaved with normal code makes reading it
> harder ...

I've deleted a lot of the dprintfs that weren't especially useful to
me any more.

>
> [...]
> > +        if (restart_absent && !m->restart_seen[substr]) {
> > +            av_log(m->avctx, AV_LOG_ERROR,
> > +                   "No restart header indicated for substream %d", substr);
> > +            goto error;
> > +        }
>
> what is this check and the whole complicated restart_seen logic good
> for?

The main point is to detect if the restart header containing the
needed decoding parameters is missing and to error out if so. I've
reworked the check to be somewhat saner.

Ian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mlpdec.patch
Type: text/x-diff
Size: 38826 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071218/cbc0cd98/attachment.patch>