| Message ID | 20210430180701.63566-2-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avformat/dv: add avpriv_dv_close_demux | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
James Almer: > As avpriv_dv_get_packet can fail now, make it return < 0 on error, 0 on no > packet found, and > 0 on packet found. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavdevice/iec61883.c | 2 +- > libavformat/avidec.c | 4 +++- > libavformat/dv.c | 51 ++++++++++++++++++++++++++---------------- > 3 files changed, 36 insertions(+), 21 deletions(-) > > diff --git a/libavdevice/iec61883.c b/libavdevice/iec61883.c > index 18ad704066..de9f48b8fc 100644 > --- a/libavdevice/iec61883.c > +++ b/libavdevice/iec61883.c > @@ -191,7 +191,7 @@ static int iec61883_parse_queue_dv(struct iec61883_data *dv, AVPacket *pkt) > int size; > > size = avpriv_dv_get_packet(dv->dv_demux, pkt); > - if (size > 0) > + if (size) > return size; > > packet = dv->queue_first; > diff --git a/libavformat/avidec.c b/libavformat/avidec.c > index 2d0d2a7389..2f493e42a6 100644 > --- a/libavformat/avidec.c > +++ b/libavformat/avidec.c > @@ -1440,8 +1440,10 @@ static int avi_read_packet(AVFormatContext *s, AVPacket *pkt) > > if (CONFIG_DV_DEMUXER && avi->dv_demux) { > int size = avpriv_dv_get_packet(avi->dv_demux, pkt); > - if (size >= 0) > + if (size > 0) > return size; > + else if (size < 0) > + return AVERROR(ENOMEM); > else > goto resync; > } > diff --git a/libavformat/dv.c b/libavformat/dv.c > index a948fc0b98..1adc9fdb7b 100644 > --- a/libavformat/dv.c > +++ b/libavformat/dv.c > @@ -45,7 +45,7 @@ struct DVDemuxContext { > AVFormatContext* fctx; > AVStream* vst; > AVStream* ast[4]; > - AVPacket audio_pkt[4]; > + AVPacket *audio_pkt[4]; > uint8_t audio_buf[4][8192]; > int ach; > int frames; > @@ -261,11 +261,11 @@ static int dv_extract_audio_info(DVDemuxContext *c, const uint8_t *frame) > c->ast[i]->codecpar->codec_type = AVMEDIA_TYPE_AUDIO; > c->ast[i]->codecpar->codec_id = AV_CODEC_ID_PCM_S16LE; > > - av_init_packet(&c->audio_pkt[i]); > - c->audio_pkt[i].size = 0; > - c->audio_pkt[i].data = c->audio_buf[i]; > - c->audio_pkt[i].stream_index = c->ast[i]->index; > - c->audio_pkt[i].flags |= AV_PKT_FLAG_KEY; > + av_packet_unref(c->audio_pkt[i]); > + c->audio_pkt[i]->size = 0; > + c->audio_pkt[i]->data = c->audio_buf[i]; > + c->audio_pkt[i]->stream_index = c->ast[i]->index; > + c->audio_pkt[i]->flags |= AV_PKT_FLAG_KEY; > } > c->ast[i]->codecpar->sample_rate = dv_audio_frequency[freq]; > c->ast[i]->codecpar->channels = 2; > @@ -327,6 +327,9 @@ void avpriv_dv_close_demux(DVDemuxContext **pc) > if (!c) > return; > > + for (int i = 0; i < 4; i++) > + av_packet_free(&c->audio_pkt[i]); > + > av_freep(pc); > } > > @@ -336,6 +339,12 @@ static int dv_init_demux(AVFormatContext *s, DVDemuxContext *c) > if (!c->vst) > return AVERROR(ENOMEM); > > + for (int i = 0; i < 4; i++) { > + c->audio_pkt[i] = av_packet_alloc(); > + if (!c->audio_pkt[i]) > + return AVERROR(ENOMEM); > + } > + > c->fctx = s; > c->vst->codecpar->codec_type = AVMEDIA_TYPE_VIDEO; > c->vst->codecpar->codec_id = AV_CODEC_ID_DVVIDEO; > @@ -361,13 +370,14 @@ DVDemuxContext *avpriv_dv_init_demux(AVFormatContext *s) > > int avpriv_dv_get_packet(DVDemuxContext *c, AVPacket *pkt) > { > - int size = -1; > + int size = 0; > int i; > > for (i = 0; i < c->ach; i++) { > - if (c->ast[i] && c->audio_pkt[i].size) { > - *pkt = c->audio_pkt[i]; > - c->audio_pkt[i].size = 0; > + if (c->ast[i] && c->audio_pkt[i]->size) { > + if (av_packet_ref(pkt, c->audio_pkt[i]) < 0) > + return -1; > + c->audio_pkt[i]->size = 0; > size = pkt->size; > break; > } > @@ -392,9 +402,9 @@ int avpriv_dv_produce_packet(DVDemuxContext *c, AVPacket *pkt, > /* FIXME: in case of no audio/bad audio we have to do something */ > size = dv_extract_audio_info(c, buf); > for (i = 0; i < c->ach; i++) { > - c->audio_pkt[i].pos = pos; > - c->audio_pkt[i].size = size; > - c->audio_pkt[i].pts = (c->sys->height == 720) ? (c->frames & ~1) : c->frames; > + c->audio_pkt[i]->pos = pos; > + c->audio_pkt[i]->size = size; > + c->audio_pkt[i]->pts = (c->sys->height == 720) ? (c->frames & ~1) : c->frames; > ppcm[i] = c->audio_buf[i]; > } > if (c->ach) > @@ -404,15 +414,15 @@ int avpriv_dv_produce_packet(DVDemuxContext *c, AVPacket *pkt, > * channels 0,1 and odd 2,3. */ > if (c->sys->height == 720) { > if (buf[1] & 0x0C) { > - c->audio_pkt[2].size = c->audio_pkt[3].size = 0; > + c->audio_pkt[2]->size = c->audio_pkt[3]->size = 0; > } else { > - c->audio_pkt[0].size = c->audio_pkt[1].size = 0; > + c->audio_pkt[0]->size = c->audio_pkt[1]->size = 0; > } > } > > /* Now it's time to return video packet */ > size = dv_extract_video_info(c, buf); > - av_init_packet(pkt); > + av_packet_unref(pkt); This code predates the introduction of refcounted AVPackets; it therefore doesn't handle this case very well. There are four callers of avpriv_dv_produce_packet(); of these, two have refcounted packets and therefore store the packet's AVBufferRef* and reattach it later. Here is the avi demuxer, the mov demuxer does the same: AVBufferRef *avbuf = pkt->buf; size = avpriv_dv_produce_packet(avi->dv_demux, pkt, pkt->data, pkt->size, pkt->pos); pkt->buf = avbuf; pkt->flags |= AV_PKT_FLAG_KEY; if (size < 0) av_packet_unref(pkt); With your code, the reference (and the underlying buffer) will be freed in avpriv_dv_produce_packet(), leading to use-after-free. The simple fix is of course to reset pkt->buf, but I think whoever touches this code should make it properly support refcounted packets. (Btw: Is the av_init_packet() even necessary? None of the packets it gets is uninitialized.) Furthermore, I don't like that you are adding another avpriv symbol that could be easily avoided by adding a struct that has exactly the members of the packet that are actually used. > pkt->data = buf; > pkt->pos = pos; > pkt->size = size; > @@ -447,8 +457,8 @@ static int64_t dv_frame_offset(AVFormatContext *s, DVDemuxContext *c, > void ff_dv_offset_reset(DVDemuxContext *c, int64_t frame_offset) > { > c->frames = frame_offset; > - c->audio_pkt[0].size = c->audio_pkt[1].size = 0; > - c->audio_pkt[2].size = c->audio_pkt[3].size = 0; > + c->audio_pkt[0]->size = c->audio_pkt[1]->size = 0; > + c->audio_pkt[2]->size = c->audio_pkt[3]->size = 0; > } > > /************************************************************ > @@ -547,7 +557,10 @@ static int dv_read_packet(AVFormatContext *s, AVPacket *pkt) > > size = avpriv_dv_get_packet(c->dv_demux, pkt); > > - if (size < 0) { > + if (size < 0) > + return AVERROR(ENOMEM); > + > + if (!size) { > int ret; > int64_t pos = avio_tell(s->pb); > if (!c->dv_demux->sys) >
On Fri, Apr 30, 2021 at 03:07:01PM -0300, James Almer wrote: > As avpriv_dv_get_packet can fail now, make it return < 0 on error, 0 on no > packet found, and > 0 on packet found. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavdevice/iec61883.c | 2 +- > libavformat/avidec.c | 4 +++- > libavformat/dv.c | 51 ++++++++++++++++++++++++++---------------- > 3 files changed, 36 insertions(+), 21 deletions(-) crashes, but i guess thats what andreas already reported but if its something else or you need a testcase then ill retest this and provide a better report and test sample, just ask me if you need it thx [avi @ 0x2ced4680] Switching to NI mode, due to poor interleaving ==21022== Invalid read of size 8 ==21022== at 0x10F6A73: av_buffer_ref (in ffmpeg_g) ==21022== by 0x72C6D0: av_packet_ref (in ffmpeg_g) ==21022== by 0x7AA48B: avcodec_send_packet (in ffmpeg_g) ==21022== by 0x6AE90A: try_decode_frame (in ffmpeg_g) ==21022== by 0x6B98D8: avformat_find_stream_info (in ffmpeg_g) ==21022== by 0x2DB293: open_input_file (in ffmpeg_g) ==21022== by 0x2DEBB3: ffmpeg_parse_options (in ffmpeg_g) ==21022== by 0x2D3391: main (in ffmpeg_g) ==21022== Address 0x2d0a8748 is 8 bytes inside a block of size 24 free'd ==21022== at 0x4C32D3B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==21022== by 0x10F6ABC: av_buffer_unref (in ffmpeg_g) ==21022== by 0x72C640: av_packet_unref (in ffmpeg_g) ==21022== by 0x59B371: avpriv_dv_produce_packet (in ffmpeg_g) ==21022== by 0x577F4F: avi_read_packet (in ffmpeg_g) ==21022== by 0x6B3D5A: ff_read_packet (in ffmpeg_g) ==21022== by 0x6B4AFA: read_frame_internal (in ffmpeg_g) ==21022== by 0x6B95FC: avformat_find_stream_info (in ffmpeg_g) ==21022== by 0x2DB293: open_input_file (in ffmpeg_g) ==21022== by 0x2DEBB3: ffmpeg_parse_options (in ffmpeg_g) ==21022== by 0x2D3391: main (in ffmpeg_g) ==21022== Block was alloc'd at ==21022== at 0x4C33E76: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==21022== by 0x4C33F91: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==21022== by 0x110C0E2: av_malloc (in ffmpeg_g) ==21022== by 0x110C308: av_mallocz (in ffmpeg_g) ==21022== by 0x10F6925: av_buffer_create (in ffmpeg_g) ==21022== by 0x10F69E6: av_buffer_alloc (in ffmpeg_g) ==21022== by 0x72BE07: av_grow_packet (in ffmpeg_g) ==21022== by 0x6AEFD9: append_packet_chunked (in ffmpeg_g) ==21022== by 0x577F02: avi_read_packet (in ffmpeg_g) ==21022== by 0x6B3D5A: ff_read_packet (in ffmpeg_g) ==21022== by 0x6B4AFA: read_frame_internal (in ffmpeg_g) ==21022== by 0x6B95FC: avformat_find_stream_info (in ffmpeg_g) ==21022== by 0x2DB293: open_input_file (in ffmpeg_g) ==21022== by 0x2DEBB3: ffmpeg_parse_options (in ffmpeg_g) ==21022== by 0x2D3391: main (in ffmpeg_g) [...]
On 5/2/2021 1:10 AM, Andreas Rheinhardt wrote: > James Almer: >> As avpriv_dv_get_packet can fail now, make it return < 0 on error, 0 on no >> packet found, and > 0 on packet found. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavdevice/iec61883.c | 2 +- >> libavformat/avidec.c | 4 +++- >> libavformat/dv.c | 51 ++++++++++++++++++++++++++---------------- >> 3 files changed, 36 insertions(+), 21 deletions(-) >> >> diff --git a/libavdevice/iec61883.c b/libavdevice/iec61883.c >> index 18ad704066..de9f48b8fc 100644 >> --- a/libavdevice/iec61883.c >> +++ b/libavdevice/iec61883.c >> @@ -191,7 +191,7 @@ static int iec61883_parse_queue_dv(struct iec61883_data *dv, AVPacket *pkt) >> int size; >> >> size = avpriv_dv_get_packet(dv->dv_demux, pkt); >> - if (size > 0) >> + if (size) >> return size; >> >> packet = dv->queue_first; >> diff --git a/libavformat/avidec.c b/libavformat/avidec.c >> index 2d0d2a7389..2f493e42a6 100644 >> --- a/libavformat/avidec.c >> +++ b/libavformat/avidec.c >> @@ -1440,8 +1440,10 @@ static int avi_read_packet(AVFormatContext *s, AVPacket *pkt) >> >> if (CONFIG_DV_DEMUXER && avi->dv_demux) { >> int size = avpriv_dv_get_packet(avi->dv_demux, pkt); >> - if (size >= 0) >> + if (size > 0) >> return size; >> + else if (size < 0) >> + return AVERROR(ENOMEM); >> else >> goto resync; >> } >> diff --git a/libavformat/dv.c b/libavformat/dv.c >> index a948fc0b98..1adc9fdb7b 100644 >> --- a/libavformat/dv.c >> +++ b/libavformat/dv.c >> @@ -45,7 +45,7 @@ struct DVDemuxContext { >> AVFormatContext* fctx; >> AVStream* vst; >> AVStream* ast[4]; >> - AVPacket audio_pkt[4]; >> + AVPacket *audio_pkt[4]; >> uint8_t audio_buf[4][8192]; >> int ach; >> int frames; >> @@ -261,11 +261,11 @@ static int dv_extract_audio_info(DVDemuxContext *c, const uint8_t *frame) >> c->ast[i]->codecpar->codec_type = AVMEDIA_TYPE_AUDIO; >> c->ast[i]->codecpar->codec_id = AV_CODEC_ID_PCM_S16LE; >> >> - av_init_packet(&c->audio_pkt[i]); >> - c->audio_pkt[i].size = 0; >> - c->audio_pkt[i].data = c->audio_buf[i]; >> - c->audio_pkt[i].stream_index = c->ast[i]->index; >> - c->audio_pkt[i].flags |= AV_PKT_FLAG_KEY; >> + av_packet_unref(c->audio_pkt[i]); >> + c->audio_pkt[i]->size = 0; >> + c->audio_pkt[i]->data = c->audio_buf[i]; >> + c->audio_pkt[i]->stream_index = c->ast[i]->index; >> + c->audio_pkt[i]->flags |= AV_PKT_FLAG_KEY; >> } >> c->ast[i]->codecpar->sample_rate = dv_audio_frequency[freq]; >> c->ast[i]->codecpar->channels = 2; >> @@ -327,6 +327,9 @@ void avpriv_dv_close_demux(DVDemuxContext **pc) >> if (!c) >> return; >> >> + for (int i = 0; i < 4; i++) >> + av_packet_free(&c->audio_pkt[i]); >> + >> av_freep(pc); >> } >> >> @@ -336,6 +339,12 @@ static int dv_init_demux(AVFormatContext *s, DVDemuxContext *c) >> if (!c->vst) >> return AVERROR(ENOMEM); >> >> + for (int i = 0; i < 4; i++) { >> + c->audio_pkt[i] = av_packet_alloc(); >> + if (!c->audio_pkt[i]) >> + return AVERROR(ENOMEM); >> + } >> + >> c->fctx = s; >> c->vst->codecpar->codec_type = AVMEDIA_TYPE_VIDEO; >> c->vst->codecpar->codec_id = AV_CODEC_ID_DVVIDEO; >> @@ -361,13 +370,14 @@ DVDemuxContext *avpriv_dv_init_demux(AVFormatContext *s) >> >> int avpriv_dv_get_packet(DVDemuxContext *c, AVPacket *pkt) >> { >> - int size = -1; >> + int size = 0; >> int i; >> >> for (i = 0; i < c->ach; i++) { >> - if (c->ast[i] && c->audio_pkt[i].size) { >> - *pkt = c->audio_pkt[i]; >> - c->audio_pkt[i].size = 0; >> + if (c->ast[i] && c->audio_pkt[i]->size) { >> + if (av_packet_ref(pkt, c->audio_pkt[i]) < 0) >> + return -1; >> + c->audio_pkt[i]->size = 0; >> size = pkt->size; >> break; >> } >> @@ -392,9 +402,9 @@ int avpriv_dv_produce_packet(DVDemuxContext *c, AVPacket *pkt, >> /* FIXME: in case of no audio/bad audio we have to do something */ >> size = dv_extract_audio_info(c, buf); >> for (i = 0; i < c->ach; i++) { >> - c->audio_pkt[i].pos = pos; >> - c->audio_pkt[i].size = size; >> - c->audio_pkt[i].pts = (c->sys->height == 720) ? (c->frames & ~1) : c->frames; >> + c->audio_pkt[i]->pos = pos; >> + c->audio_pkt[i]->size = size; >> + c->audio_pkt[i]->pts = (c->sys->height == 720) ? (c->frames & ~1) : c->frames; >> ppcm[i] = c->audio_buf[i]; >> } >> if (c->ach) >> @@ -404,15 +414,15 @@ int avpriv_dv_produce_packet(DVDemuxContext *c, AVPacket *pkt, >> * channels 0,1 and odd 2,3. */ >> if (c->sys->height == 720) { >> if (buf[1] & 0x0C) { >> - c->audio_pkt[2].size = c->audio_pkt[3].size = 0; >> + c->audio_pkt[2]->size = c->audio_pkt[3]->size = 0; >> } else { >> - c->audio_pkt[0].size = c->audio_pkt[1].size = 0; >> + c->audio_pkt[0]->size = c->audio_pkt[1]->size = 0; >> } >> } >> >> /* Now it's time to return video packet */ >> size = dv_extract_video_info(c, buf); >> - av_init_packet(pkt); >> + av_packet_unref(pkt); > > This code predates the introduction of refcounted AVPackets; it > therefore doesn't handle this case very well. > There are four callers of avpriv_dv_produce_packet(); of these, two have > refcounted packets and therefore store the packet's AVBufferRef* and > reattach it later. Here is the avi demuxer, the mov demuxer does the same: > > AVBufferRef *avbuf = pkt->buf; > size = avpriv_dv_produce_packet(avi->dv_demux, pkt, > pkt->data, pkt->size, pkt->pos); > pkt->buf = avbuf; > pkt->flags |= AV_PKT_FLAG_KEY; > if (size < 0) > av_packet_unref(pkt); > > With your code, the reference (and the underlying buffer) will be freed > in avpriv_dv_produce_packet(), leading to use-after-free. > > The simple fix is of course to reset pkt->buf, but I think whoever > touches this code should make it properly support refcounted packets. > (Btw: Is the av_init_packet() even necessary? None of the packets it > gets is uninitialized.) Probably not, so i can just remove it and prevent what you describe above. > > Furthermore, I don't like that you are adding another avpriv symbol that > could be easily avoided by adding a struct that has exactly the members > of the packet that are actually used. A struct with the subset of AVPacket members used here will have more than half of them. And I don't see introducing a close() avpriv_ symbol here as a problem (its signature isn't going to require changes, and it can do more things in the future if required). But if you really dislike it, i can look into implementing it. > >> pkt->data = buf; >> pkt->pos = pos; >> pkt->size = size; >> @@ -447,8 +457,8 @@ static int64_t dv_frame_offset(AVFormatContext *s, DVDemuxContext *c, >> void ff_dv_offset_reset(DVDemuxContext *c, int64_t frame_offset) >> { >> c->frames = frame_offset; >> - c->audio_pkt[0].size = c->audio_pkt[1].size = 0; >> - c->audio_pkt[2].size = c->audio_pkt[3].size = 0; >> + c->audio_pkt[0]->size = c->audio_pkt[1]->size = 0; >> + c->audio_pkt[2]->size = c->audio_pkt[3]->size = 0; >> } >> >> /************************************************************ >> @@ -547,7 +557,10 @@ static int dv_read_packet(AVFormatContext *s, AVPacket *pkt) >> >> size = avpriv_dv_get_packet(c->dv_demux, pkt); >> >> - if (size < 0) { >> + if (size < 0) >> + return AVERROR(ENOMEM); >> + >> + if (!size) { >> int ret; >> int64_t pos = avio_tell(s->pb); >> if (!c->dv_demux->sys) >> > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On 5/2/2021 6:21 AM, Michael Niedermayer wrote: > On Fri, Apr 30, 2021 at 03:07:01PM -0300, James Almer wrote: >> As avpriv_dv_get_packet can fail now, make it return < 0 on error, 0 on no >> packet found, and > 0 on packet found. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavdevice/iec61883.c | 2 +- >> libavformat/avidec.c | 4 +++- >> libavformat/dv.c | 51 ++++++++++++++++++++++++++---------------- >> 3 files changed, 36 insertions(+), 21 deletions(-) > > crashes, but i guess thats what andreas already reported but > if its something else or you need a testcase then ill retest this and > provide a better report and test sample, just ask me if you need it Can this file (or a portion of it) be added to FATE? Looking at coverage, these avpriv_ functions are only used with the raw DV demuxer. Both the AVI and MOV implementations are untested. I also see three mov files in the FATE samples repo that are not being used. None of them have audio, though. > > thx > > [avi @ 0x2ced4680] Switching to NI mode, due to poor interleaving > ==21022== Invalid read of size 8 > ==21022== at 0x10F6A73: av_buffer_ref (in ffmpeg_g) > ==21022== by 0x72C6D0: av_packet_ref (in ffmpeg_g) > ==21022== by 0x7AA48B: avcodec_send_packet (in ffmpeg_g) > ==21022== by 0x6AE90A: try_decode_frame (in ffmpeg_g) > ==21022== by 0x6B98D8: avformat_find_stream_info (in ffmpeg_g) > ==21022== by 0x2DB293: open_input_file (in ffmpeg_g) > ==21022== by 0x2DEBB3: ffmpeg_parse_options (in ffmpeg_g) > ==21022== by 0x2D3391: main (in ffmpeg_g) > ==21022== Address 0x2d0a8748 is 8 bytes inside a block of size 24 free'd > ==21022== at 0x4C32D3B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > ==21022== by 0x10F6ABC: av_buffer_unref (in ffmpeg_g) > ==21022== by 0x72C640: av_packet_unref (in ffmpeg_g) > ==21022== by 0x59B371: avpriv_dv_produce_packet (in ffmpeg_g) > ==21022== by 0x577F4F: avi_read_packet (in ffmpeg_g) > ==21022== by 0x6B3D5A: ff_read_packet (in ffmpeg_g) > ==21022== by 0x6B4AFA: read_frame_internal (in ffmpeg_g) > ==21022== by 0x6B95FC: avformat_find_stream_info (in ffmpeg_g) > ==21022== by 0x2DB293: open_input_file (in ffmpeg_g) > ==21022== by 0x2DEBB3: ffmpeg_parse_options (in ffmpeg_g) > ==21022== by 0x2D3391: main (in ffmpeg_g) > ==21022== Block was alloc'd at > ==21022== at 0x4C33E76: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > ==21022== by 0x4C33F91: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > ==21022== by 0x110C0E2: av_malloc (in ffmpeg_g) > ==21022== by 0x110C308: av_mallocz (in ffmpeg_g) > ==21022== by 0x10F6925: av_buffer_create (in ffmpeg_g) > ==21022== by 0x10F69E6: av_buffer_alloc (in ffmpeg_g) > ==21022== by 0x72BE07: av_grow_packet (in ffmpeg_g) > ==21022== by 0x6AEFD9: append_packet_chunked (in ffmpeg_g) > ==21022== by 0x577F02: avi_read_packet (in ffmpeg_g) > ==21022== by 0x6B3D5A: ff_read_packet (in ffmpeg_g) > ==21022== by 0x6B4AFA: read_frame_internal (in ffmpeg_g) > ==21022== by 0x6B95FC: avformat_find_stream_info (in ffmpeg_g) > ==21022== by 0x2DB293: open_input_file (in ffmpeg_g) > ==21022== by 0x2DEBB3: ffmpeg_parse_options (in ffmpeg_g) > ==21022== by 0x2D3391: main (in ffmpeg_g) > > > [...] > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
diff --git a/libavdevice/iec61883.c b/libavdevice/iec61883.c index 18ad704066..de9f48b8fc 100644 --- a/libavdevice/iec61883.c +++ b/libavdevice/iec61883.c @@ -191,7 +191,7 @@ static int iec61883_parse_queue_dv(struct iec61883_data *dv, AVPacket *pkt) int size; size = avpriv_dv_get_packet(dv->dv_demux, pkt); - if (size > 0) + if (size) return size; packet = dv->queue_first; diff --git a/libavformat/avidec.c b/libavformat/avidec.c index 2d0d2a7389..2f493e42a6 100644 --- a/libavformat/avidec.c +++ b/libavformat/avidec.c @@ -1440,8 +1440,10 @@ static int avi_read_packet(AVFormatContext *s, AVPacket *pkt) if (CONFIG_DV_DEMUXER && avi->dv_demux) { int size = avpriv_dv_get_packet(avi->dv_demux, pkt); - if (size >= 0) + if (size > 0) return size; + else if (size < 0) + return AVERROR(ENOMEM); else goto resync; } diff --git a/libavformat/dv.c b/libavformat/dv.c index a948fc0b98..1adc9fdb7b 100644 --- a/libavformat/dv.c +++ b/libavformat/dv.c @@ -45,7 +45,7 @@ struct DVDemuxContext { AVFormatContext* fctx; AVStream* vst; AVStream* ast[4]; - AVPacket audio_pkt[4]; + AVPacket *audio_pkt[4]; uint8_t audio_buf[4][8192]; int ach; int frames; @@ -261,11 +261,11 @@ static int dv_extract_audio_info(DVDemuxContext *c, const uint8_t *frame) c->ast[i]->codecpar->codec_type = AVMEDIA_TYPE_AUDIO; c->ast[i]->codecpar->codec_id = AV_CODEC_ID_PCM_S16LE; - av_init_packet(&c->audio_pkt[i]); - c->audio_pkt[i].size = 0; - c->audio_pkt[i].data = c->audio_buf[i]; - c->audio_pkt[i].stream_index = c->ast[i]->index; - c->audio_pkt[i].flags |= AV_PKT_FLAG_KEY; + av_packet_unref(c->audio_pkt[i]); + c->audio_pkt[i]->size = 0; + c->audio_pkt[i]->data = c->audio_buf[i]; + c->audio_pkt[i]->stream_index = c->ast[i]->index; + c->audio_pkt[i]->flags |= AV_PKT_FLAG_KEY; } c->ast[i]->codecpar->sample_rate = dv_audio_frequency[freq]; c->ast[i]->codecpar->channels = 2; @@ -327,6 +327,9 @@ void avpriv_dv_close_demux(DVDemuxContext **pc) if (!c) return; + for (int i = 0; i < 4; i++) + av_packet_free(&c->audio_pkt[i]); + av_freep(pc); } @@ -336,6 +339,12 @@ static int dv_init_demux(AVFormatContext *s, DVDemuxContext *c) if (!c->vst) return AVERROR(ENOMEM); + for (int i = 0; i < 4; i++) { + c->audio_pkt[i] = av_packet_alloc(); + if (!c->audio_pkt[i]) + return AVERROR(ENOMEM); + } + c->fctx = s; c->vst->codecpar->codec_type = AVMEDIA_TYPE_VIDEO; c->vst->codecpar->codec_id = AV_CODEC_ID_DVVIDEO; @@ -361,13 +370,14 @@ DVDemuxContext *avpriv_dv_init_demux(AVFormatContext *s) int avpriv_dv_get_packet(DVDemuxContext *c, AVPacket *pkt) { - int size = -1; + int size = 0; int i; for (i = 0; i < c->ach; i++) { - if (c->ast[i] && c->audio_pkt[i].size) { - *pkt = c->audio_pkt[i]; - c->audio_pkt[i].size = 0; + if (c->ast[i] && c->audio_pkt[i]->size) { + if (av_packet_ref(pkt, c->audio_pkt[i]) < 0) + return -1; + c->audio_pkt[i]->size = 0; size = pkt->size; break; } @@ -392,9 +402,9 @@ int avpriv_dv_produce_packet(DVDemuxContext *c, AVPacket *pkt, /* FIXME: in case of no audio/bad audio we have to do something */ size = dv_extract_audio_info(c, buf); for (i = 0; i < c->ach; i++) { - c->audio_pkt[i].pos = pos; - c->audio_pkt[i].size = size; - c->audio_pkt[i].pts = (c->sys->height == 720) ? (c->frames & ~1) : c->frames; + c->audio_pkt[i]->pos = pos; + c->audio_pkt[i]->size = size; + c->audio_pkt[i]->pts = (c->sys->height == 720) ? (c->frames & ~1) : c->frames; ppcm[i] = c->audio_buf[i]; } if (c->ach) @@ -404,15 +414,15 @@ int avpriv_dv_produce_packet(DVDemuxContext *c, AVPacket *pkt, * channels 0,1 and odd 2,3. */ if (c->sys->height == 720) { if (buf[1] & 0x0C) { - c->audio_pkt[2].size = c->audio_pkt[3].size = 0; + c->audio_pkt[2]->size = c->audio_pkt[3]->size = 0; } else { - c->audio_pkt[0].size = c->audio_pkt[1].size = 0; + c->audio_pkt[0]->size = c->audio_pkt[1]->size = 0; } } /* Now it's time to return video packet */ size = dv_extract_video_info(c, buf); - av_init_packet(pkt); + av_packet_unref(pkt); pkt->data = buf; pkt->pos = pos; pkt->size = size; @@ -447,8 +457,8 @@ static int64_t dv_frame_offset(AVFormatContext *s, DVDemuxContext *c, void ff_dv_offset_reset(DVDemuxContext *c, int64_t frame_offset) { c->frames = frame_offset; - c->audio_pkt[0].size = c->audio_pkt[1].size = 0; - c->audio_pkt[2].size = c->audio_pkt[3].size = 0; + c->audio_pkt[0]->size = c->audio_pkt[1]->size = 0; + c->audio_pkt[2]->size = c->audio_pkt[3]->size = 0; } /************************************************************ @@ -547,7 +557,10 @@ static int dv_read_packet(AVFormatContext *s, AVPacket *pkt) size = avpriv_dv_get_packet(c->dv_demux, pkt); - if (size < 0) { + if (size < 0) + return AVERROR(ENOMEM); + + if (!size) { int ret; int64_t pos = avio_tell(s->pb); if (!c->dv_demux->sys)
As avpriv_dv_get_packet can fail now, make it return < 0 on error, 0 on no packet found, and > 0 on packet found. Signed-off-by: James Almer <jamrial@gmail.com> --- libavdevice/iec61883.c | 2 +- libavformat/avidec.c | 4 +++- libavformat/dv.c | 51 ++++++++++++++++++++++++++---------------- 3 files changed, 36 insertions(+), 21 deletions(-)