[FFmpeg-soc] SVQ3 depacketizer

[Ronald S. Bultje]

Hi,

On Wed, Jun 30, 2010 at 4:45 AM, Martin Storsjö <martin at martin.st> wrote:
> 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)?

Generic problem with the dyn_buf API. I'd say, please fix
close_dyn_buf() to automatically add padding to the returned buffer,
that's much more user-friendly.

Ronald