[FFmpeg-devel] [PATCH] avformat: add apic to AVStream
Andreas Rheinhardt
andreas.rheinhardt at outlook.com
Mon Mar 29 12:46:36 EEST 2021
James Almer:
> As a replacement for attached_pic, which is in turn deprecated and scheduled
> for removal.
>
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
> TODO: APIChanges entry and version bump.
>
> Decided to use the name apic for the field, since it's how id3v2 and other
> formats call it.
>
> Also, in a fortunate coincidence, the first "non public" field in AVStream is a
> unused void pointer that had to be left in place when the field was moved to
> AVStreamInternal to keep every offset intact for the sake of ffmpeg accessing
> fields after it, so I'm going to take advantage of it so this doesn't have to
> wait until the major bump.
>
> libavformat/apetag.c | 15 ++++++++++++---
> libavformat/asfdec_f.c | 21 +++++++++++++++------
> libavformat/asfdec_o.c | 21 +++++++++++++++------
> libavformat/avformat.h | 21 ++++++++++++++++-----
> libavformat/flac_picture.c | 21 +++++++++++++++------
> libavformat/hls.c | 6 +++---
> libavformat/id3v2.c | 23 +++++++++++++++++------
> libavformat/matroskadec.c | 9 ++++++++-
> libavformat/mov.c | 31 +++++++++++++++++++++++--------
> libavformat/utils.c | 14 ++++++++++++--
> libavformat/version.h | 3 +++
> libavformat/wtvdec.c | 13 ++++++++++---
> 12 files changed, 149 insertions(+), 49 deletions(-)
>
> diff --git a/libavformat/apetag.c b/libavformat/apetag.c
> index 23ee6b516d..e0e96c3f4c 100644
> --- a/libavformat/apetag.c
> +++ b/libavformat/apetag.c
> @@ -81,7 +81,7 @@ static int ape_tag_read_field(AVFormatContext *s)
> if ((id = ff_guess_image2_codec(filename)) != AV_CODEC_ID_NONE) {
> int ret;
>
> - ret = av_get_packet(s->pb, &st->attached_pic, size);
> + ret = av_get_packet(s->pb, st->apic, size);
> if (ret < 0) {
> av_log(s, AV_LOG_ERROR, "Error reading cover art.\n");
> return ret;
> @@ -91,8 +91,17 @@ static int ape_tag_read_field(AVFormatContext *s)
> st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
> st->codecpar->codec_id = id;
>
> - st->attached_pic.stream_index = st->index;
> - st->attached_pic.flags |= AV_PKT_FLAG_KEY;
> + st->apic->stream_index = st->index;
> + st->apic->flags |= AV_PKT_FLAG_KEY;
> +#if FF_API_ATTACHED_PIC
> +FF_DISABLE_DEPRECATION_WARNINGS
> + ret = av_packet_ref(&st->attached_pic, st->apic);
> + if (ret < 0) {
> + av_packet_unref(st->apic);
> + return ret;
> + }
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
> } else {
> if ((ret = ff_get_extradata(s, st->codecpar, s->pb, size)) < 0)
> return ret;
> diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
> index 2fae528f4d..838fc924d5 100644
> --- a/libavformat/asfdec_f.c
> +++ b/libavformat/asfdec_f.c
> @@ -223,7 +223,7 @@ static int get_value(AVIOContext *pb, int type, int type2_size)
> * but in reality this is only loosely similar */
> static int asf_read_picture(AVFormatContext *s, int len)
> {
> - AVPacket pkt = { 0 };
> + AVPacket *pkt = s->internal->parse_pkt;
> const CodecMime *mime = ff_id3v2_mime_tags;
> enum AVCodecID id = AV_CODEC_ID_NONE;
> char mimetype[64];
> @@ -277,7 +277,7 @@ static int asf_read_picture(AVFormatContext *s, int len)
> return AVERROR(ENOMEM);
> len -= avio_get_str16le(s->pb, len - picsize, desc, desc_len);
>
> - ret = av_get_packet(s->pb, &pkt, picsize);
> + ret = av_get_packet(s->pb, pkt, picsize);
> if (ret < 0)
> goto fail;
>
> @@ -286,12 +286,21 @@ static int asf_read_picture(AVFormatContext *s, int len)
> ret = AVERROR(ENOMEM);
> goto fail;
> }
> + av_packet_move_ref(st->apic, pkt);
> st->disposition |= AV_DISPOSITION_ATTACHED_PIC;
> st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
> st->codecpar->codec_id = id;
> - st->attached_pic = pkt;
> - st->attached_pic.stream_index = st->index;
> - st->attached_pic.flags |= AV_PKT_FLAG_KEY;
> + st->apic->stream_index = st->index;
> + st->apic->flags |= AV_PKT_FLAG_KEY;
> +#if FF_API_ATTACHED_PIC
> +FF_DISABLE_DEPRECATION_WARNINGS
> + ret = av_packet_ref(&st->attached_pic, st->apic);
> + if (ret < 0) {
> + av_packet_unref(st->apic);
> + goto fail;
> + }
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
>
> if (*desc)
> av_dict_set(&st->metadata, "title", desc, AV_DICT_DONT_STRDUP_VAL);
> @@ -304,7 +313,7 @@ static int asf_read_picture(AVFormatContext *s, int len)
>
> fail:
> av_freep(&desc);
> - av_packet_unref(&pkt);
> + av_packet_unref(pkt);
> return ret;
> }
>
> diff --git a/libavformat/asfdec_o.c b/libavformat/asfdec_o.c
> index 34ae541934..51577064b8 100644
> --- a/libavformat/asfdec_o.c
> +++ b/libavformat/asfdec_o.c
> @@ -360,7 +360,7 @@ static int asf_set_metadata(AVFormatContext *s, const uint8_t *name,
> * but in reality this is only loosely similar */
> static int asf_read_picture(AVFormatContext *s, int len)
> {
> - AVPacket pkt = { 0 };
> + AVPacket *pkt = s->internal->parse_pkt;
> const CodecMime *mime = ff_id3v2_mime_tags;
> enum AVCodecID id = AV_CODEC_ID_NONE;
> char mimetype[64];
> @@ -414,7 +414,7 @@ static int asf_read_picture(AVFormatContext *s, int len)
> return AVERROR(ENOMEM);
> len -= avio_get_str16le(s->pb, len - picsize, desc, desc_len);
>
> - ret = av_get_packet(s->pb, &pkt, picsize);
> + ret = av_get_packet(s->pb, pkt, picsize);
> if (ret < 0)
> goto fail;
>
> @@ -424,12 +424,21 @@ static int asf_read_picture(AVFormatContext *s, int len)
> goto fail;
> }
>
> + av_packet_move_ref(st->apic, pkt);
> st->disposition |= AV_DISPOSITION_ATTACHED_PIC;
> st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
> st->codecpar->codec_id = id;
> - st->attached_pic = pkt;
> - st->attached_pic.stream_index = st->index;
> - st->attached_pic.flags |= AV_PKT_FLAG_KEY;
> + st->apic->stream_index = st->index;
> + st->apic->flags |= AV_PKT_FLAG_KEY;
> +#if FF_API_ATTACHED_PIC
> +FF_DISABLE_DEPRECATION_WARNINGS
> + ret = av_packet_ref(&st->attached_pic, st->apic);
> + if (ret < 0) {
> + av_packet_unref(st->apic);
> + goto fail;
> + }
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
>
> if (*desc) {
> if (av_dict_set(&st->metadata, "title", desc, AV_DICT_DONT_STRDUP_VAL) < 0)
> @@ -444,7 +453,7 @@ static int asf_read_picture(AVFormatContext *s, int len)
>
> fail:
> av_freep(&desc);
> - av_packet_unref(&pkt);
> + av_packet_unref(pkt);
> return ret;
> }
>
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 765bc3b6f5..769d27d84a 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -947,14 +947,19 @@ typedef struct AVStream {
> */
> AVRational avg_frame_rate;
>
> +#if FF_API_ATTACHED_PIC
> /**
> * For streams with AV_DISPOSITION_ATTACHED_PIC disposition, this packet
> * will contain the attached picture.
> *
> * decoding: set by libavformat, must not be modified by the caller.
> * encoding: unused
> + *
> + * @deprecated Use apic instead.
> */
> + attribute_deprecated
> AVPacket attached_pic;
> +#endif
>
> /**
> * An array of side data that applies to the whole stream (i.e. the
> @@ -1039,6 +1044,17 @@ typedef struct AVStream {
> */
> AVCodecParameters *codecpar;
>
> + /**
> + * For streams with AV_DISPOSITION_ATTACHED_PIC disposition, this packet
> + * will contain the attached picture. It is allocated and freed by
> + * libavformat in avformat_new_stream() and avformat_free_context()
> + * respectively.
> + *
> + * - demuxing: set by libavformat, must not be modified by the caller.
> + * - muxing: unused
Does this actually allow using apic internally? (I intend to use this
for muxers to store their attached pics; and eventually users should
also be allowed to set this before avformat_init_output() (this can
reduce delay and might be easier for users that already have the
attached pics). Here:
https://github.com/mkver/FFmpeg/commits/matroska_muxer is a branch (not
based upon master; probably doesn't apply to master at all) that
contains this.)
> + */
> + AVPacket *apic;
> +
> /*****************************************************************
> * All fields below this line are not part of the public API. They
> * may not be used outside of libavformat and can be changed and
> @@ -1049,11 +1065,6 @@ typedef struct AVStream {
> *****************************************************************
> */
>
> -#if LIBAVFORMAT_VERSION_MAJOR < 59
> - // kept for ABI compatibility only, do not access in any way
> - void *unused;
> -#endif
> -
> int pts_wrap_bits; /**< number of bits in pts (used for wrapping control) */
>
> // Timestamp generation support:
> diff --git a/libavformat/flac_picture.c b/libavformat/flac_picture.c
> index f15cfa877a..f771df3fc2 100644
> --- a/libavformat/flac_picture.c
> +++ b/libavformat/flac_picture.c
> @@ -165,12 +165,20 @@ int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size, int tr
> RETURN_ERROR(AVERROR(ENOMEM));
> }
>
> - av_packet_unref(&st->attached_pic);
> - st->attached_pic.buf = data;
> - st->attached_pic.data = data->data;
> - st->attached_pic.size = len;
> - st->attached_pic.stream_index = st->index;
> - st->attached_pic.flags |= AV_PKT_FLAG_KEY;
> + av_packet_unref(st->apic);
> + st->apic->buf = data;
> + st->apic->data = data->data;
> + st->apic->size = len;
> + st->apic->stream_index = st->index;
> + st->apic->flags |= AV_PKT_FLAG_KEY;
> + data = NULL;
> +#if FF_API_ATTACHED_PIC
> +FF_DISABLE_DEPRECATION_WARNINGS
> + ret = av_packet_ref(&st->attached_pic, st->apic);
> + if (ret < 0)
> + goto fail;
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
>
> st->disposition |= AV_DISPOSITION_ATTACHED_PIC;
> st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
> @@ -185,6 +193,7 @@ int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size, int tr
>
> fail:
> av_buffer_unref(&data);
> + av_packet_unref(st->apic);
> av_freep(&desc);
>
> return ret;
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index 597bea7f25..2b18385496 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -1068,15 +1068,15 @@ static int id3_has_changed_values(struct playlist *pls, AVDictionary *metadata,
> }
>
> /* check if apic appeared */
> - if (apic && (pls->ctx->nb_streams != 2 || !pls->ctx->streams[1]->attached_pic.data))
> + if (apic && (pls->ctx->nb_streams != 2 || !pls->ctx->streams[1]->apic->data))
> return 1;
>
> if (apic) {
> - int size = pls->ctx->streams[1]->attached_pic.size;
> + int size = pls->ctx->streams[1]->apic->size;
> if (size != apic->buf->size - AV_INPUT_BUFFER_PADDING_SIZE)
> return 1;
>
> - if (memcmp(apic->buf->data, pls->ctx->streams[1]->attached_pic.data, size) != 0)
> + if (memcmp(apic->buf->data, pls->ctx->streams[1]->apic->data, size) != 0)
> return 1;
> }
>
> diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c
> index f33b7ba93a..f8c10ab90c 100644
> --- a/libavformat/id3v2.c
> +++ b/libavformat/id3v2.c
> @@ -1142,6 +1142,7 @@ int ff_id3v2_parse_apic(AVFormatContext *s, ID3v2ExtraMeta *extra_meta)
> for (cur = extra_meta; cur; cur = cur->next) {
> ID3v2ExtraMetaAPIC *apic;
> AVStream *st;
> + int ret;
>
> if (strcmp(cur->tag, "APIC"))
> continue;
> @@ -1162,14 +1163,24 @@ int ff_id3v2_parse_apic(AVFormatContext *s, ID3v2ExtraMeta *extra_meta)
>
> av_dict_set(&st->metadata, "comment", apic->type, 0);
>
> - av_packet_unref(&st->attached_pic);
> - st->attached_pic.buf = apic->buf;
> - st->attached_pic.data = apic->buf->data;
> - st->attached_pic.size = apic->buf->size - AV_INPUT_BUFFER_PADDING_SIZE;
> - st->attached_pic.stream_index = st->index;
> - st->attached_pic.flags |= AV_PKT_FLAG_KEY;
> + av_packet_unref(st->apic);
> + st->apic->buf = apic->buf;
> + st->apic->data = apic->buf->data;
> + st->apic->size = apic->buf->size - AV_INPUT_BUFFER_PADDING_SIZE;
> + st->apic->stream_index = st->index;
> + st->apic->flags |= AV_PKT_FLAG_KEY;
>
> apic->buf = NULL;
> +
> +#if FF_API_ATTACHED_PIC
> +FF_DISABLE_DEPRECATION_WARNINGS
> + ret = av_packet_ref(&st->attached_pic, st->apic);
> + if (ret < 0) {
> + av_packet_unref(st->apic);
> + return ret;
> + }
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
> }
>
> return 0;
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 1dc188c946..92e3b8f183 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -3007,7 +3007,7 @@ static int matroska_read_header(AVFormatContext *s)
> attachments[j].stream = st;
>
> if (st->codecpar->codec_id != AV_CODEC_ID_NONE) {
> - AVPacket *pkt = &st->attached_pic;
> + AVPacket *pkt = st->apic;
>
> st->disposition |= AV_DISPOSITION_ATTACHED_PIC;
> st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
> @@ -3019,6 +3019,13 @@ static int matroska_read_header(AVFormatContext *s)
> pkt->size = attachments[j].bin.size;
> pkt->stream_index = st->index;
> pkt->flags |= AV_PKT_FLAG_KEY;
> +#if FF_API_ATTACHED_PIC
> +FF_DISABLE_DEPRECATION_WARNINGS
> + res = av_packet_ref(&st->attached_pic, pkt);
> + if (res < 0)
> + goto fail;
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
> } else {
> st->codecpar->codec_type = AVMEDIA_TYPE_ATTACHMENT;
> if (ff_alloc_extradata(st->codecpar, attachments[j].bin.size))
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index cb818ebe0e..d99f922709 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -204,12 +204,21 @@ static int mov_read_covr(MOVContext *c, AVIOContext *pb, int type, int len)
> return AVERROR(ENOMEM);
> st->priv_data = sc;
>
> - ret = av_get_packet(pb, &st->attached_pic, len);
> + ret = av_get_packet(pb, st->apic, len);
> if (ret < 0)
> return ret;
> +#if FF_API_ATTACHED_PIC
> +FF_DISABLE_DEPRECATION_WARNINGS
> + ret = av_packet_ref(&st->attached_pic, st->apic);
> + if (ret < 0) {
> + av_packet_unref(st->apic);
> + return ret;
> + }
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
>
> - if (st->attached_pic.size >= 8 && id != AV_CODEC_ID_BMP) {
> - if (AV_RB64(st->attached_pic.data) == 0x89504e470d0a1a0a) {
> + if (st->apic->size >= 8 && id != AV_CODEC_ID_BMP) {
> + if (AV_RB64(st->apic->data) == 0x89504e470d0a1a0a) {
> id = AV_CODEC_ID_PNG;
> } else {
> id = AV_CODEC_ID_MJPEG;
> @@ -218,8 +227,8 @@ static int mov_read_covr(MOVContext *c, AVIOContext *pb, int type, int len)
>
> st->disposition |= AV_DISPOSITION_ATTACHED_PIC;
>
> - st->attached_pic.stream_index = st->index;
> - st->attached_pic.flags |= AV_PKT_FLAG_KEY;
> + st->apic->stream_index = st->index;
> + st->apic->flags |= AV_PKT_FLAG_KEY;
>
> st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
> st->codecpar->codec_id = id;
> @@ -7229,11 +7238,17 @@ static void mov_read_chapters(AVFormatContext *s)
> goto finish;
> }
>
> - if (av_get_packet(sc->pb, &st->attached_pic, sample->size) < 0)
> + if (av_get_packet(sc->pb, st->apic, sample->size) < 0)
> goto finish;
>
> - st->attached_pic.stream_index = st->index;
> - st->attached_pic.flags |= AV_PKT_FLAG_KEY;
> + st->apic->stream_index = st->index;
> + st->apic->flags |= AV_PKT_FLAG_KEY;
> +#if FF_API_ATTACHED_PIC
> +FF_DISABLE_DEPRECATION_WARNINGS
> + if (av_packet_ref(&st->attached_pic, st->apic) < 0)
> + goto finish;
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
> }
> } else {
> st->codecpar->codec_type = AVMEDIA_TYPE_DATA;
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 524765aeb4..9308b53106 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -457,7 +457,7 @@ int avformat_queue_attached_pictures(AVFormatContext *s)
> for (i = 0; i < s->nb_streams; i++)
> if (s->streams[i]->disposition & AV_DISPOSITION_ATTACHED_PIC &&
> s->streams[i]->discard < AVDISCARD_ALL) {
> - if (s->streams[i]->attached_pic.size <= 0) {
> + if (s->streams[i]->apic->size <= 0) {
> av_log(s, AV_LOG_WARNING,
> "Attached picture on stream %d has invalid size, "
> "ignoring\n", i);
> @@ -466,7 +466,7 @@ int avformat_queue_attached_pictures(AVFormatContext *s)
>
> ret = avpriv_packet_list_put(&s->internal->raw_packet_buffer,
> &s->internal->raw_packet_buffer_end,
> - &s->streams[i]->attached_pic,
> + s->streams[i]->apic,
> av_packet_ref, 0);
> if (ret < 0)
> return ret;
> @@ -4371,8 +4371,14 @@ static void free_stream(AVStream **pst)
> if (st->parser)
> av_parser_close(st->parser);
>
> +#if FF_API_ATTACHED_PIC
> +FF_DISABLE_DEPRECATION_WARNINGS
> if (st->attached_pic.data)
> av_packet_unref(&st->attached_pic);
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
> +
> + av_packet_free(&st->apic);
>
> if (st->internal) {
> avcodec_free_context(&st->internal->avctx);
> @@ -4530,6 +4536,10 @@ FF_ENABLE_DEPRECATION_WARNINGS
> if (!st->codecpar)
> goto fail;
>
> + st->apic = av_packet_alloc();
> + if (!st->apic)
> + goto fail;
> +
Does this have to be always allocated? It feels wrong to have this set
for streams that don't have the ATTACHED_PIC disposition.
> st->internal->avctx = avcodec_alloc_context3(NULL);
> if (!st->internal->avctx)
> goto fail;
> diff --git a/libavformat/version.h b/libavformat/version.h
> index ced5600034..dce936794a 100644
> --- a/libavformat/version.h
> +++ b/libavformat/version.h
> @@ -115,6 +115,9 @@
> #ifndef FF_API_LAVF_PRIV_OPT
> #define FF_API_LAVF_PRIV_OPT (LIBAVFORMAT_VERSION_MAJOR < 60)
> #endif
> +#ifndef FF_API_ATTACHED_PIC
> +#define FF_API_ATTACHED_PIC (LIBAVFORMAT_VERSION_MAJOR < 60)
> +#endif
>
>
> #ifndef FF_API_R_FRAME_RATE
> diff --git a/libavformat/wtvdec.c b/libavformat/wtvdec.c
> index 7def9d2348..05386add76 100644
> --- a/libavformat/wtvdec.c
> +++ b/libavformat/wtvdec.c
> @@ -454,11 +454,18 @@ static void get_attachment(AVFormatContext *s, AVIOContext *pb, int length)
> st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
> st->codecpar->codec_id = AV_CODEC_ID_MJPEG;
> st->id = -1;
> - ret = av_get_packet(pb, &st->attached_pic, filesize);
> + ret = av_get_packet(pb, st->apic, filesize);
> if (ret < 0)
> goto done;
> - st->attached_pic.stream_index = st->index;
> - st->attached_pic.flags |= AV_PKT_FLAG_KEY;
> + st->apic->stream_index = st->index;
> + st->apic->flags |= AV_PKT_FLAG_KEY;
> +#if FF_API_ATTACHED_PIC
> +FF_DISABLE_DEPRECATION_WARNINGS
> + ret = av_packet_ref(&st->attached_pic, st->apic);
> + if (ret < 0)
> + goto done;
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
> st->disposition |= AV_DISPOSITION_ATTACHED_PIC;
> done:
> avio_seek(pb, pos + length, SEEK_SET);
>
More information about the ffmpeg-devel
mailing list