[FFmpeg-devel] [PATCH] RV30/40 decoder

Michael Niedermayer michaelni
Sat Nov 24 17:51:10 CET 2007


On Sat, Nov 24, 2007 at 04:43:56PM +0200, Kostya wrote:
> On Fri, Nov 23, 2007 at 09:46:19PM +0100, Michael Niedermayer wrote:
> > On Sun, Nov 18, 2007 at 11:11:24AM +0200, Kostya wrote:
> > > Well, it roughly the same feature-wise as it was,
> > > I just don't think I will improve it soon, yet
> > > it is playable (and maybe will attract samples
> > > and patches, I'm an optimist).
> > 
> > rv30data.h
> > ok, commit
> 
> So I can commit only header filer as they get approved, right?

yes


>  
> [...] 
>  
> > rv34data.h
> > > +/**
> > > + * Chroma quantizer values
> > > + * Second table is used for DC-only blocks
> > > + */
> > > +static const uint8_t rv34_chroma_quant[2][32] = {
> > > + {  0,  1,  2,  3,  4,  5,  6,  7,  8,  9, 10, 11, 12, 13, 14, 15,
> > > +   16, 17, 17, 18, 19, 20, 20, 21, 22, 22, 23, 23, 24, 24, 25, 25 },
> > > + {  0,  0,  0,  1,  2,  3,  4,  5,  6,  7,  8,  9, 10, 11, 12, 13,
> > > +   14, 15, 15, 16, 17, 18, 18, 19, 20, 20, 21, 21, 22, 22, 23, 23 }
> > > +};
> > 
> > the name is inconsistant to rv30_luma_dc_quant
> > or if its noz just for dc then the comment is wrong
> 
> Comment is wrong, first subtable is for scaling AC, the second one is for
> scaling DC.
>  
> > except these rv34data.h can be commited
> > 
> > 
> > rv40vlc2.h
> > ok, commit
> > 
> > 
> > rv34vlc.h
> > > +// INTRA TABLES
> > > +// BLOCK 0
> > > +static const uint8_t rv34_table_0_0_0[CBPPAT_VLC_SIZE] =
> > 
> > comment is not doxygen compatible
> > 
> > also the use of names like rv34_table_X_Y_Z instead of tables with
> > proper names is unacceptable
> > 
> > non table data not yet reviewed
> 
> Here's new version.

> /*
>  * RealVideo 4 decoder
[...]
> /**
>  * @file rv40vlc.h
>  * RV40 VLC tables.
>  */
> 
> #ifndef FFMPEG_RV40VLC_H
> #define FFMPEG_RV40VLC_H
[...]
> #endif /* FFMPEG_RV40VLC_H */

hmm


> static const uint8_t* rv34_inter_secondpatvlc_pointers[NUM_INTER_TABLES][2] = {
>  { rv34_table_inter_otherpat0_0[0], rv34_table_inter_otherpat1_0[0] },
>  { rv34_table_inter_otherpat0_1[0], rv34_table_inter_otherpat1_1[0] },
>  { rv34_table_inter_otherpat0_2[0], rv34_table_inter_otherpat1_2[0] },
>  { rv34_table_inter_otherpat0_3[0], rv34_table_inter_otherpat1_3[0] },
>  { rv34_table_inter_otherpat0_4[0], rv34_table_inter_otherpat1_4[0] },
>  { rv34_table_inter_otherpat0_5[0], rv34_table_inter_otherpat1_5[0] },
>  { rv34_table_inter_otherpat0_6[0], rv34_table_inter_otherpat1_6[0] },
> };
> 
> static const uint8_t* rv34_inter_thirdpatvlc_pointers[NUM_INTER_TABLES][2] = {
>  { rv34_table_inter_otherpat0_0[1], rv34_table_inter_otherpat1_0[1] },
>  { rv34_table_inter_otherpat0_1[1], rv34_table_inter_otherpat1_1[1] },
>  { rv34_table_inter_otherpat0_2[1], rv34_table_inter_otherpat1_2[1] },
>  { rv34_table_inter_otherpat0_3[1], rv34_table_inter_otherpat1_3[1] },
>  { rv34_table_inter_otherpat0_4[1], rv34_table_inter_otherpat1_4[1] },
>  { rv34_table_inter_otherpat0_5[1], rv34_table_inter_otherpat1_5[1] },
>  { rv34_table_inter_otherpat0_6[1], rv34_table_inter_otherpat1_6[1] },
> };

the names rv34_table_inter_otherpat seems not very specific to its use
also why not

static const uint8_t rv34_inter_second/thirdpatvlc[NUM_INTER_TABLES][2][OTHERBLK_VLC_SIZE]
?
these dont seem used anywhere else and seem to be of the same size
this may also apply to other tables



> 
> /** Tables used in variable code parts decoding */
> //@{
> #define NUM_OMEGA 31
> #define OMEGA_BITS 8
> 
> static const uint8_t omega_part_vlc_codes[NUM_OMEGA] = {
>  0x00, 0x01, 0x01, 0x04, 0x05, 0x03, 0x01,
>  0x10, 0x11, 0x09, 0x14, 0x15, 0x0B, 0x03, 0x01,
>  0x40, 0x41, 0x21, 0x44, 0x45, 0x23, 0x09,
>  0x50, 0x51, 0x29, 0x54, 0x55, 0x2B, 0x0B, 0x03, 0x01
> };
> 
> static const uint8_t omega_part_vlc_bits[NUM_OMEGA] = {
>  8, 8, 7, 8, 8, 7, 5,
>  8, 8, 7, 8, 8, 7, 5, 3,
>  8, 8, 7, 8, 8, 7, 5,
>  8, 8, 7, 8, 8, 7, 5, 3, 1
> };
> 
> static const uint8_t omega_part_vlc_syms[NUM_OMEGA] = {
>  0x80, 0x81, 0x70, 0x82, 0x83, 0x71, 0x50,
>  0x84, 0x85, 0x72, 0x86, 0x87, 0x73, 0x51, 0x30,
>  0x88, 0x89, 0x74, 0x8A, 0x8B, 0x75, 0x52,
>  0x8C, 0x8D, 0x76, 0x8E, 0x8F, 0x77, 0x53, 0x31, 0x10
> };
> //@}

could it be that ff_rv34_get_omega() is nothing else than an interleaved
exp golomb reader?
if so the existing code in golomb.h should be used and you should
benchmark that against your code and if yours is faster replace the code
in golomb.h by it

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Thouse who are best at talking, realize last or never when they are wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071124/86608c25/attachment.pgp>



More information about the ffmpeg-devel mailing list