[FFmpeg-devel] [PATCH] RoQ decoder 4:4:4 update

[Michael Niedermayer]

Hi

On Mon, Jun 04, 2007 at 06:43:00PM -0400, Eric Lasota wrote:
> Michael Niedermayer wrote:
> 
> > what is the advantage of pre unpacking cells to 4:4:4 instead of doing
> > it during rendering?
> >   
> 
> Readability, simplifies vector applications into block copies, 

i dont agree that it improves readability or code simplicity
i rather think it has no real effect on these 2, its possible to
do the unpacking at different places and still have simple and readable
code

what it certainly does though is it doubles the memory needed for the
codebooks and doubles the memory reads needed when copying them into the
image

the situation would be different if the output format would be rgb but
it isnt (so unless you plan to add optional rgb output support i think
it would be better to do the upacking during rendering, though i wont
reject the patch just because of this if you prefer the preunpacking)


> represents codebook how it will be applied rather than how it's stored.

true but this is neither advantage nor disadvantage IMHO


> 
> >> +typedef unsigned char roq_mb2[3][4];
> >> +typedef unsigned char roq_mb4[3][16];
> >> +typedef unsigned char roq_mb8[3][64];
> >>     
> >
> > these do nothing but obfuscate the code
> >   
> 
> How so?

the reader sees roq_mbX and has no clue at all what that is
so he has to search for what it is and then wonders why uint8_t wasnt used


> 
> > reordering of functions
> >   
> 
> All of the prototyped functions were replaced.  Do you want them matched 
> to the existing prototype order?

yes

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

There will always be a question for which you do not know the correct awnser.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070605/b337b6e8/attachment.pgp>