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

[Aurelien Jacobs]

On Mon, 17 Sep 2007 12:07:06 +0300
Kostya <kostya.shishkov at gmail.com> wrote:

> On Mon, Sep 17, 2007 at 10:10:37AM +0200, Diego Biurrun wrote:
> > On Mon, Sep 17, 2007 at 08:19:50AM +0300, Kostya wrote:
> > > On Sun, Sep 16, 2007 at 11:10:43PM +0200, Michael Niedermayer wrote:
> > > > 
> > > > On Sun, Sep 16, 2007 at 07:58:51PM +0300, Kostya wrote:
> > > > 
> > > > [...]
> > > > > +static int rv30_parse_slice_header(RV40DecContext *r, GetBitContext *gb, SliceInfo *si)
> > > > > +{
> > > > 
> > > > please split rv30 and rv40 into seperate files (rv30/rv40/common code) and
> > > > merge rv30/rv40 functions if they are nearly idenitcal
> > > 
> > > There are only two functions specific for RV30 - slice and macroblock type parsing.
> > 
> > It would still be preferable to have them in a separate file so that
> > they can be conditionally compiled.
> 
> It is okay to have function pointers in context to parse_slice_header() and
> decode_intra_types()? In that case decoder-specific code will result in two
> or three functions plus custom decode_init().

Yes, I think it is OK. Have a look at how vp5 an vp6 are split with common
code in vp56.c and function pointers initialization in vp5_decode_init() and
vp6_decode_init().
It would be really nice if you could split rv30/rv40 specific code.

> Otherwise it would result in duplicating rv40_decode_frame, rv40_decode_slice,
> rv40_decode_macroblock and rv40_decode_mb_header.

This is for sure not the way to go !

Aurel