[Ffmpeg-devel] [RFC] Fraps v2 support
Michael Niedermayer
michaelni
Fri Nov 3 21:02:49 CET 2006
Hi
On Fri, Nov 03, 2006 at 03:00:51PM +0200, Kostya wrote:
> On Thu, Nov 02, 2006 at 07:52:23PM +0100, Michael Niedermayer wrote:
> > Hi
> >
> > On Thu, Nov 02, 2006 at 08:24:46PM +0200, Kostya wrote:
> > > Please test if patch attached confirms to $subj. I have only a bit of one
> > > sample here.
> >
> > [...]
> >
> > > + cur_node = 510; // for futher use in huffman decoding
> > > + /* we have built Huffman table and are ready to decode plane */
> > > + /* XXX this bitreader ignores every 32th bit so lavc bitreader cannot be used*/
> >
> > hmm i can belive it ... are you sure that they ignore every 32th bit??
> > if its really true then id suggest that you use a temporary buffer and
> > convert that 31/32bit block stream to a normal one, i guess that this
> > with the get_vlc() code from lavc is still faster ...
>
> Sorry, I misunderstood it. New patch use default bitreader on bswapped() buffer
> (and ALT_BITSTREAM_READER_LE returns bits in reverse order)
> As a side effect decoder spends ~40% time less on decoding.
[...]
> #define FPS_TAG MKTAG('F', 'P', 'S', 'x')
>
> +/* symbol for Huffman tree node */
> +#define HNODE -1
> +
> /**
> + * Huffman node
> + * FIXME one day this should belong to one general framework
> + */
> +typedef struct Node{
> + int sym;
> + int count;
> + int n0;
> + int n1;
> +}Node;
sym,n0,n1 dont need an int
[...]
> +/**
> + * Comparator - our nodes should ascend by count
> + * but with preserved symbol order
> + */
> +static int huff_cmp(const Node *a, const Node *b){
> + if(a->count == b->count){
> + return (a->sym > b->sym) ? 1 : -1;
> + }
> + return (a->count > b->count) ? 1 : -1;
return (a->count - b->count)*256 + a->sym - b->sym;
[...]
> + for(i = 0; i < 256; i++){
> + s->nodes[i].sym = i;
> + s->nodes[i].count = LE_32(src);
> + s->nodes[i].n0 = s->nodes[i].n1 = -1;
> + src += 4;
> + }
> + size -= 1024;
> + qsort(s->nodes, 256, sizeof(Node), huff_cmp);
> + cur_node = 256;
> + // FIXME how it will handle nodes with zero count?
> + for(i = 0; i < 511; i += 2){
> + s->nodes[cur_node].sym = HNODE;
> + s->nodes[cur_node].count = s->nodes[i].count + s->nodes[i+1].count;
> + s->nodes[cur_node].n0 = i;
> + s->nodes[cur_node].n1 = i + 1;
it doesnt seem that n1 is ever changed, it will always be n0+1 so its
redundant
and n0,sym will either be -1,0..255 or 0..510,-1 so they could be merged
into one variable
> + for(j = cur_node; j > 0; j--){
> + if(s->nodes[j].count >= s->nodes[j - 1].count) break;
> + tmp = s->nodes[j];
> + s->nodes[j] = s->nodes[j - 1];
> + s->nodes[j - 1] = tmp;
(FF)SWAP()
> + }
there is a possible problem here, and that is that the sum of the 2 counts
can overflow, if it does then this loop will reorder too many elements
and by doing that trash the n0/n1 pointers which then a few lines later
could lead to a stack overflow due the recursive vlc building being
run on a tree which isnt one as it contains a loop from the corruption
> + cur_node++;
i is always 2*cur_node-512 so maybe one of them could be set based on the
other, like
int i= 2*cur_node-512;
at the top of the loop maybe thats more readable, iam not sure though ...
[...]
> + s->tmpbuf = av_realloc(s->tmpbuf, offs[1] - offs[0] - 1024);
> + if(fraps2_decode_plane(s, f->data[0], f->linesize[0], avctx->width,
> + avctx->height, buf + offs[0], offs[1] - offs[0], 0) < 0) {
> + av_log(avctx, AV_LOG_ERROR, "Error decoding plane 0\n");
> + return -1;
> + }
> + s->tmpbuf = av_realloc(s->tmpbuf, offs[2] - offs[1] - 1024);
> + if(fraps2_decode_plane(s, f->data[1], f->linesize[1], avctx->width/2,
> + avctx->height/2, buf + offs[1], offs[2] - offs[1], 1) < 0) {
> + av_log(avctx, AV_LOG_ERROR, "Error decoding plane 1\n");
> + return -1;
> + }
> + s->tmpbuf = av_realloc(s->tmpbuf, buf_size - offs[2] - 1024);
> + if(fraps2_decode_plane(s, f->data[2], f->linesize[1], avctx->width/2,
> + avctx->height/2, buf + offs[2], buf_size - offs[2], 1) < 0) {
> + av_log(avctx, AV_LOG_ERROR, "Error decoding plane 2\n");
> + return -1;
> + }
code triplification, imho make a loop out of the 3
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is
More information about the ffmpeg-devel
mailing list