[FFmpeg-cvslog] r24942 - trunk/libavformat/a64.c

[Tobias Bindhammer]

> At least lifetime should be checked, to avoid a division by 0.

Actually in the interleaving case, it should also be member of 2^x so
that a block can be divided into nice chunks, however i am playing with
the thought of putting the sanity checks into the codec, as i need (and
generate) the value for lifetime there.

> nit: I believe the rule is:
> if () {
> } else {
> }

Can fix that on the next cosmetic changes commit. And: you are right :-)

> Also, but that may just be me, the code would benefit from some empty
> lines to ease readability.

>> +                if(c->prev_pkt.data) {
>> +                    /* put frame (screen + colram) from last packet into buffer */
>> +                    put_buffer(s->pb, c->prev_pkt.data + charset_size + frame_size * i, frame_size);
>> +                }
>> +                else {
>> +                       /* a bit ugly, but is there an alternative to put many zeros? */
>> +                        for(j = 0; j < frame_size; j++) put_byte(s->pb, 0);
> 
> what would be typical values for ch_chunksize and frame_size ?

frame_size is either 0x400 or 0x500, ch_chunksize typically 0x400,
depending on the lifetime, but 4 is default.

>> +            /* backup current packet for next turn */
>> +            if(pkt->data) {
>> +                av_new_packet(&c->prev_pkt, pkt->size);
> 
> This is leaking the previous prev_pkt.

eeks, right, actually one new packet per encoding process would be
enough for me and i shall either reuuse it, as packet size does not grow
(only optionally shrink in last packet), or simply destruct it beforehand.

>> +    AVPacket pkt;
>> +    /* need to flush last packet? */
>> +    if(c->interleaved) a64_write_packet(s, &pkt);
> 
> You're passing a non-initialized packet to the function?
> The check for pkt->data in the interleaved case there is dubious then.

Okay, can be fixed easily. Thanks for giving all that hints!

Toby

-- 
Dipl. Ing. Tobias Bindhammer
Institut f?r Verteilte Systeme
Oberer Eselsberg          Phone: + 49 731/502-4235
Universit?t Ulm           Fax  : + 49 731/502-4142
D-89069 Ulm               mailto:tobias.bindhammer at uni-ulm.de
http://www-vs.informatik.uni-ulm.de/~bindhammer/