[FFmpeg-devel] [PATCH 16/50] avformat/dv: use av_packet_alloc() to allocate packets
Andreas Rheinhardt
andreas.rheinhardt at gmail.com
Sun Feb 7 09:47:56 EET 2021
James Almer:
> Signed-off-by: James Almer <jamrial at 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",
> };
>
More information about the ffmpeg-devel
mailing list