Hi, On Wed, Jun 30, 2010 at 4:45 AM, Martin Storsjö <martin@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