[FFmpeg-soc] SVQ3 depacketizer

Martin Storsjö martin at martin.st
Wed Jun 30 10:45:03 CEST 2010


On Wed, 30 Jun 2010, Martin Storsjö wrote:

> On Wed, 30 Jun 2010, Josh Allmann wrote:
> 
> > +    if (config_packet) {
> > +        GetBitContext gb;
> > +        int config;
> > +
> > +        if (len < 2 || !(st->codec->extradata = av_malloc(len + 8 + FF_INPUT_BUFFER_PADDING_SIZE)))
> > +            return AVERROR_INVALIDDATA;
> 
> Do av_freep() of extradata before allocating new data, otherwise we'd leak 
> memory if we'd receive bad or otherwise weird data.

Not only bad or weird data - these config packets are sent before each 
keyframe (which makes sense, so we're not totally screwed if we miss one, 
we just have to wait for the next one), so we'd be accumulating leaks 
here.

> > +    if (end_packet) {
> > +        av_init_packet(pkt);
> > +        pkt->stream_index = st->index;
> > +        pkt->pts          = sv->timestamp;
> > +        pkt->flags        = sv->is_keyframe ? AV_PKT_FLAG_KEY : 0;
> > +        pkt->size         = url_close_dyn_buf(sv->pktbuf, &pkt->data);
> 
> Neat, this is exactly what I meant, that could be used to save some 
> memcpys for the xiph depacketizer, too. But this lacks a pkt->destruct = 
> av_destruct_packet - at the moment, you're leaking every packet.

Ah, yes, as I mentioned in the status mail comments yesterday (but forgot 
when I reviewed this, but valgrind reminded me) - you need to add padding 
at the end of the packets, e.g. put_buffer with 
FF_INPUT_BUFFER_PADDING_SIZE zero bytes, then subtract the same amount 
from the size returned by url_close_dyn_buf.

For this, I guess it would be handy with a put_zero function (or similar) 
that writes a certain amount of zero bytes, so we wouldn't have to 
allocate an array just to get some padding to write. Or perhaps we should 
just loop with put_byte(, 0)?

// Martin


More information about the FFmpeg-soc mailing list