[Ffmpeg-devel] [PATCH] Vorbis decoder

[Balatoni Denes]

Hi!

New version of the diff attached.

2005. m?jus 12. 13.42-kor Michael Niedermayer ezeket a bolcs gondolatokat 
fogalmazta meg:
> Hi
>
> On Wednesday 11 May 2005 22:42, Balatoni Denes wrote:
> > Hi!
> > +#ifdef CONFIG_VORBIS_DECODER
> > +    register_avcodec(&vorbis_decoder);
> > +#endif //CONFIG_WS_SND1_DECODER
>
> typo
Fixed, thanks.


> > +    uint32_t ret=0;
>
> exact width types should only be used when an exact width is needed (not
> for local variables like ret,i, ...)
I tried to use the possible minimal size for each variable. So if for example 
somebody wants to port this decoder to C64 or ZX Spectrum, he will know where 
he can use native 8 bit types, or needs 16 bits, or must use more than 16 
bits (in those cases 32 bits are usually not needed though).

> the file is full of tabs please convert them to spaces
> you might also wish to remove the trailing whitespace

Done.

>
> > +    av_free(vc->channel_residues);
>
> av_freep(&vc->channel_residues); would zero the pointer afterwards too,
> which is IMHO a nice safety meassure against misstakly using it afterwards

Done.


> > +for(t=0;t<(1<<16);++t) tmp_vlc_bits[t]=0;
>
> memset(tmp_vlc_bits, 0, sizeof(tmp_vlc_bits)); would be cleaner

Done.
> > +	    codebook_setup->codevectors=(float
>
> *)av_mallocz(used_entries*codebook_setup->dimensions * sizeof(float));
>
> is used_entries*codebook_setup->dimensions * sizeof(float) somehow
> guranteed not to overflow 32bit? if not then this might be exploitable
It was exploitable, now there are sane limits for codebook_setup->dimensions
and used_entries, which no encoder would exceed  imho.

> > +    switch (vc->blocksize_1) {
> > +	case 64:
> > ...
>
> code duplication, not to mention that the 1 page switch() can be replaced
> by a log2() and a table of pointers to vwin*

Done.

> > +#ifdef V_DEBUG
> > +    av_log(vc->avccontext, AV_LOG_INFO, " Codebooks: %d \n",
>
> vc->codebook_count);
>
> > +#endif
>
> maybe add a
> AV_DEBUG(...) av_log(NULL, AV_LOG_DEBUG, ...)
> macro to common.h which could then easily be #defined to nothing to avoid
> the #ifdefs around, it would also make a few developers happy who
> complained about having to type too much for a quick debug av_log

#ifdef - #endif pairs are eliminated, a patch to common.h has not yet been 
prepared.

> > + static float vwin64[32] = {
>
> should either be static const or generated during init
Done.

> --
> Michael

If somebody would like to fix the decoder to handle the new (not yet commited) 
extradata format, I would appreciate it very much.

bye
Denes


-- 
- Use the Source Luke ! -
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vorbis_native.diff.bz2
Type: application/x-bzip2
Size: 52246 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20050513/e55ecf4f/attachment.bin>