[FFmpeg-devel] [PATCH] TXD demuxer and decoder

[Ivo]

Hi,

On Saturday 05 May 2007 21:43, Michael Niedermayer wrote:
> On Sat, May 05, 2007 at 01:57:56PM +0200, Ivo wrote:
> > +        g3  = (((2*g1 +g0 ) * 42) >> 7) & 0x00ff00;
>
> *42 >> 7 == *21 >> 6

Fixed.

> > +void ff_decode_dxt1(const uint8_t *src, uint8_t *dst,
> > +                    const unsigned int w, const unsigned int h,
> > +                    const unsigned int stride) {
> > +    unsigned int bx, by, qstride = stride/4;
> > +    uint8_t *s = (uint8_t *) src;
>
> casting/changing const to non const is bad, there is no need to change
> src so it should stay const, casting it to non const makes the code look
> like it will change src ...

Yes. I mistake const int* for int const* sometimes. Fixed.

> > +    *picture = *(AVFrame *)&s->picture;
>
> unneeded cast

Fixed.

[..]
> > +    for(;;) {
> > +        if (state->end_of_txd_file)
> > +            return AVERROR_IO;
[..]
> > +                if (!state->stack) {
> > +                    state->end_of_txd_file = 1;
> > +                    continue;
>
> the s/continue/return AVERROR_IO/ is IMHO more clear ...

Fixed.

> > [..stack based demuxer..]
>
> whats the stack good for?
> i think TXD_FILE / TXD_INFO should be read in the header reading code
> like read and ignore TXD_FILE
> skip all TXD_INFO
>
> and then the packet read code could do
> read next chunk
>
> if(TXD_TEXTURE)
>     ignore
> if(TXD_TEXTURE_DATA)
>     av_get_packet() ...
> else
>     skip
>
> or does that fail with any file? if so why?
> your stack code might appear more robust at first (it did to me) but if
> the sizes of TXD_TEXTURE and the contained packets missmatch then it will
> also fail like my suggestion above and if they dont missmatch then i dont
> see where the stack would do any good ...

When I started writing the demuxer, I was undecided which approach would be 
best. I chose to implement a full-blown stack based demuxer because I do 
not know what other chunks and sub-chunks might exist in other files. I  
only tested files of Grand Theft Auto: San Andreas, that only have v8 and 
v9 texture_data chunks and the (sub)chunks I described at the wiki page. 
But there might well be other sub-chunks that re-use id's of other scopes 
and/or more levels of encapsulation. The Renderware engine has been in use 
for over six years and there are more than 60 games listed at 
http://en.wikipedia.org/wiki/Renderware. I did not want to risk being 
forced to overhaul the whole demuxer when I encounter a file that cannot be 
demuxed without knowing the proper context in which a subchunk appears. I 
think the cost (both in size and speed) of the current demuxer compared to 
what you suggest is negligible. Sadly I do not have more test files (I 
might be able to get my hands on older GTA games) so I can not be sure if 
the current filedescription is conclusive. If it were, I could simplify the 
demuxer.

--Ivo