James Almer: > Signed-off-by: James Almer <jamr...@gmail.com> > --- > This one must be committed after the upcoming major bump, as it changes > the return value of an avpriv function. > > libavdevice/iec61883.c | 2 +- > libavformat/dv.c | 56 ++++++++++++++++++++++++++++-------------- > 2 files changed, 38 insertions(+), 20 deletions(-) > > diff --git a/libavdevice/iec61883.c b/libavdevice/iec61883.c > index cafafb2672..4582ada151 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/dv.c b/libavformat/dv.c > index 26a78139f5..f5c0c44afc 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,10 @@ 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; > + 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; > @@ -332,6 +331,12 @@ static int dv_init_demux(AVFormatContext *s, > DVDemuxContext *c) > c->vst->codecpar->bit_rate = 25000000; > c->vst->start_time = 0; > > + for (int i = 0; i < 4; i++) { > + c->audio_pkt[i] = av_packet_alloc(); > + if (!c->audio_pkt[i]) > + return AVERROR(ENOMEM);
This leads to lots of leaks on error, for both avpriv_dv_init_demux (for which a free function will be necessary) as well as the dv demuxer (whose read_close function is not called automatically on error). > + } > + > return 0; > } > > @@ -353,13 +358,15 @@ 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) { > + int ret = av_packet_ref(pkt, c->audio_pkt[i]); > + if (ret < 0) > + return ret; > + c->audio_pkt[i]->size = 0; > size = pkt->size; > break; > } > @@ -384,9 +391,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) > @@ -396,15 +403,14 @@ 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); This function here is a bit nuts: It predates reference-counted buffers and so it simply resets pkt->buf (it in fact treats pkt purely as something for output as if it were already blank). The mov and avi demuxers work around this by copying pkt->buf and putting it in the returned packet afterwards (which is probably unnecessary when the above call is ; whereas the iec61883 device/demuxer really puts a blank packet in there and makes it refcounted afterwards. It would be nice if this whole mess were to be cleaned up. > pkt->data = buf; > pkt->pos = pos; > pkt->size = size; > @@ -439,8 +445,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; > } > > /************************************************************ > @@ -539,7 +545,7 @@ static int dv_read_packet(AVFormatContext *s, AVPacket > *pkt) > > size = avpriv_dv_get_packet(&c->dv_demux, pkt); > > - if (size < 0) { > + if (!size) { > int ret; > int64_t pos = avio_tell(s->pb); > if (!c->dv_demux.sys) > @@ -614,6 +620,17 @@ static int dv_probe(const AVProbeData *p) > return 0; > } > > +static int dv_read_close(AVFormatContext *s) > +{ > + RawDVContext *r = s->priv_data; > + DVDemuxContext *c = &r->dv_demux; > + > + for (int i = 0; i < 4; i++) > + av_packet_free(&c->audio_pkt[i]); > + > + return 0; > +} > + > AVInputFormat ff_dv_demuxer = { > .name = "dv", > .long_name = NULL_IF_CONFIG_SMALL("DV (Digital Video)"), > @@ -622,5 +639,6 @@ AVInputFormat ff_dv_demuxer = { > .read_header = dv_read_header, > .read_packet = dv_read_packet, > .read_seek = dv_read_seek, > + .read_close = dv_read_close, > .extensions = "dv,dif", > }; > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".