Opened 8 years ago
Closed 8 years ago
#5543 closed defect (fixed)
av_packet_ref incorrectly processes packets with offset between data and buffer
Reported by: | mrlika | Owned by: | |
---|---|---|---|
Priority: | normal | Component: | avcodec |
Version: | git-master | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | Reproduced by developer: | no | |
Analyzed by developer: | no |
Description
As example aac_adtstoasc bitstream filter returns pakcets with pkt->data != pkt->buf->data (i.e. with offset) when I started to use new av_bsf_* API.
Then if you call av_packet_ref on such packet you will get incorrect dst packet because it just sets dst->data = dst->buf->data in the code:
int av_packet_ref(AVPacket *dst, const AVPacket *src) { int ret; ret = av_packet_copy_props(dst, src); if (ret < 0) return ret; if (!src->buf) { ret = packet_alloc(&dst->buf, src->size); if (ret < 0) goto fail; memcpy(dst->buf->data, src->data, src->size); } else { dst->buf = av_buffer_ref(src->buf); if (!dst->buf) { ret = AVERROR(ENOMEM); goto fail; } } dst->size = src->size; dst->data = dst->buf->data; return 0; fail: av_packet_free_side_data(dst); return ret; }
For example FLV muxer calls av_packet_ref internally that causes errors when processing packets from aac_adtstoasc bitstream filter.
Change History (9)
follow-up: 2 comment:1 by , 8 years ago
Priority: | critical → normal |
---|
follow-up: 3 comment:2 by , 8 years ago
Replying to cehoyos:
How can I reproduce the issue?
I don't know how to reproduce the issue using ffmpeg currently because in trunc it still uses already deprecated av_bitstream_* API.
Once ffmpeg switches to new av_bsf_* API you will get errors with aac_adtstoasc filter -> FLV muxer flow.
To reproduce by coding you can create valid packet with pkt->data that has offset to pkt->buf->data and pass it to av_packet_ref. dst packet will not have the offset as you can see ffrom source code of av_packet_ref above.
comment:3 by , 8 years ago
comment:4 by , 8 years ago
This looks like a pretty bad bug, and the code looks incorrect to me. Just write a patch that fixes this issue (should be simple), and send it to the mailing list.
comment:5 by , 8 years ago
The fix is just to change
dst->data = dst->buf->data;
to
dst->data = dst->buf->data + (src->data - src->buf->data);
Probably some src->data and src->buf->data validations are required.
By the way because of this bug AVPacket *av_packet_clone(AVPacket *src) that calls av_packt_ref internally clones incorrectly packets with offset.The offset is lost.
comment:6 by , 8 years ago
Source code to reproduce:
static void testcase_for_av_packet_ref(void) { AVPacket *src = av_packet_alloc(); AVPacket *dst = av_packet_alloc(); av_new_packet(src, 32); /* Allocate payload 32 bytes */ assert(src->data == src->buf->data); assert(src->size == 32); assert(src->buf->size == 32 + AV_INPUT_BUFFER_PADDING_SIZE); /* Doing payload offset by 16 bytes. Buffer remains the same */ src->data += 16; src->size -= 16; av_packet_ref(dst, src); assert(dst->size == src->size); assert(dst->data == src->data); /* This fails */ } static void testcase_for_av_packet_clone(void) { AVPacket *src = av_packet_alloc(); AVPacket *dst; av_new_packet(src, 32); /* Allocate payload 32 bytes */ assert(src->data == src->buf->data); assert(src->size == 32); assert(src->buf->size == 32 + AV_INPUT_BUFFER_PADDING_SIZE); /* Doing payload offset by 16 bytes. Buffer remains the same */ src->data += 16; src->size -= 16; dst = av_packet_clone(src); assert(dst->size == src->size); assert(dst->data == src->data); /* This fails */ }
comment:8 by , 8 years ago
comment:9 by , 8 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
How can I reproduce the issue?