| Message ID | 20210328155831.54434-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avformat: add apic to AVStream | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make_warn | warning | New warnings during build |
| 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 |
On Sun, 28 Mar 2021, James Almer wrote: > As a replacement for attached_pic, which is in turn deprecated and scheduled > for removal. > > Signed-off-by: James Almer <jamrial@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. Fine by me. > 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 If st->apic is unreffed above, shouldn't you also unref st->attached_pic here? Thanks, Marton > + ret = av_packet_ref(&st->attached_pic, st->apic); > + if (ret < 0) > + goto fail; > +FF_ENABLE_DEPRECATION_WARNINGS > +#endif >
On 3/28/2021 5:32 PM, Marton Balint wrote: > > > On Sun, 28 Mar 2021, James Almer wrote: > >> As a replacement for attached_pic, which is in turn deprecated and >> scheduled >> for removal. >> >> Signed-off-by: James Almer <jamrial@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. > > Fine by me. > >> 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 > > If st->apic is unreffed above, shouldn't you also unref st->attached_pic > here? No, i can in fact just remove it, since the stream was allocated right before it. I think this unref is there because it was previously an av_init_packet(), which i replaced as part of my deprecation work to achieve the same effect of initializing st->attached_pic (a zeroed packet), for the sake of setting fields like pts to no_pts (This is not needed for st->apic, which is properly initialized when it's allocated). The av_packet_ref() below will set all st->attached_pic fields. > > Thanks, > Marton > >> + ret = av_packet_ref(&st->attached_pic, st->apic); >> + if (ret < 0) >> + goto fail; >> +FF_ENABLE_DEPRECATION_WARNINGS >> +#endif >> > _______________________________________________ > 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".
Mar 28, 2021, 17:58 by jamrial@gmail.com: > As a replacement for attached_pic, which is in turn deprecated and scheduled > for removal. > > Signed-off-by: James Almer <jamrial@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. > Personally I don't really like 'apic'. Maybe 'pic_attachment' or 'attachment_pic'?
Lynne (12021-03-29):
> Personally I don't really like 'apic'. Maybe 'pic_attachment' or 'attachment_pic'?
By the way, do we want it to be a field?
Attached pictures can be quite big. Do we want them to stay in AVStream
forever, even when the application has already finished using them, or
does not use them at all?
Regards,
James Almer: > As a replacement for attached_pic, which is in turn deprecated and scheduled > for removal. > > Signed-off-by: James Almer <jamrial@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); >
On 3/29/2021 4:38 AM, Nicolas George wrote: > Lynne (12021-03-29): >> Personally I don't really like 'apic'. Maybe 'pic_attachment' or 'attachment_pic'? > > By the way, do we want it to be a field? > > Attached pictures can be quite big. Do we want them to stay in AVStream > forever, even when the application has already finished using them, or > does not use them at all? > > Regards, The idea for attached_pic afaik was that it's always available and easily accessible to the user from the start, instead of just the one time the demuxing process would return it in an av_read_frame() call. The doxy for AV_DISPOSITION_ATTACHED_PIC even mentions that if you seek the relevant packet may not be returned at all.
On 3/29/2021 6:46 AM, Andreas Rheinhardt wrote: > James Almer: >> As a replacement for attached_pic, which is in turn deprecated and scheduled >> for removal. >> >> Signed-off-by: James Almer <jamrial@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.) By saying "unused" in the context of muxing the user shouldn't really care or look what's on it and assume it's effectively empty, but ideally, ffmpeg would not use it internally either in that case when it can use an st->internal AVPacket that's not exposed instead. That being said, your suggestion to let the user set them on muxers sounds good. > >> + */ >> + 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. The demuxer sets the disposition after having allocated the stream, so that means they will also have to allocate this packet manually. I guess that with your ff_add_attached_pic() refactor it will make no difference, so i can wait for you to push that set and rebase this patch on top of it with this change included. > >> 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); >> > > _______________________________________________ > 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". >
James Almer (12021-03-29): > The idea for attached_pic afaik was that it's always available and easily > accessible to the user from the start, instead of just the one time the > demuxing process would return it in an av_read_frame() call. > The doxy for AV_DISPOSITION_ATTACHED_PIC even mentions that if you seek the > relevant packet may not be returned at all. Yes, but was it a GOOD idea? For the convenience of the few applications that actually use the attached pictures, libavformat will use memory in all applications. It does not seem like a good compromise to me. I think a better design would be some kind of: int avformat_get_attached_pic(AVStream *st, AVPacket **packet, unsigned flags); In the short run, it could just access the field, but in the long run it could seek back to load the packet only when required. Since applications need to update their API anyway, it is a good time to decide if we want to change something. Regards,
On 3/29/2021 9:57 AM, Nicolas George wrote: > James Almer (12021-03-29): >> The idea for attached_pic afaik was that it's always available and easily >> accessible to the user from the start, instead of just the one time the >> demuxing process would return it in an av_read_frame() call. >> The doxy for AV_DISPOSITION_ATTACHED_PIC even mentions that if you seek the >> relevant packet may not be returned at all. > > Yes, but was it a GOOD idea? For the convenience of the few applications > that actually use the attached pictures, libavformat will use memory in > all applications. It does not seem like a good compromise to me. > > I think a better design would be some kind of: > > int avformat_get_attached_pic(AVStream *st, AVPacket **packet, unsigned flags); > > In the short run, it could just access the field, but in the long run it > could seek back to load the packet only when required. Can this be done? id3v2 attached pics for many formats are handled by the generic demux code in avformat_open_input() and not by the actual demuxer. And in demuxers like asf, it seems to be done in read_header(). Would seeking be able to fetch these pictures again? The comment in the AV_DISPOSITION_ATTACHED_PIC doxy makes me think it's not possible. Personally, even if possible i think triggering a seek just to fetch an attachment is inefficient and disruptive to the user's demuxing/decoding process, and a step backwards considering it used to always be available, for both seekable and non-seekable input, the latter which will no longer be able to access the picture past the original packet. > > Since applications need to update their API anyway, it is a good time to > decide if we want to change something. Patches welcome. > > Regards, > > > _______________________________________________ > 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". >
James Almer (12021-03-29): > Can this be done? id3v2 attached pics for many formats are handled by the > generic demux code in avformat_open_input() and not by the actual demuxer. > And in demuxers like asf, it seems to be done in read_header(). > Would seeking be able to fetch these pictures again? The comment in the > AV_DISPOSITION_ATTACHED_PIC doxy makes me think it's not possible. > > Personally, even if possible i think triggering a seek just to fetch an > attachment is inefficient and disruptive to the user's demuxing/decoding > process, and a step backwards considering it used to always be available, > for both seekable and non-seekable input, the latter which will no longer be > able to access the picture past the original packet. As I said, at worse it accesses a field just like now. So of course it can be done. The point is that having a function to get the packet rather than just a pointer gives us more freedom to extend the API later. Regards,
On 3/29/2021 10:20 AM, Nicolas George wrote: > James Almer (12021-03-29): >> Can this be done? id3v2 attached pics for many formats are handled by the >> generic demux code in avformat_open_input() and not by the actual demuxer. >> And in demuxers like asf, it seems to be done in read_header(). >> Would seeking be able to fetch these pictures again? The comment in the >> AV_DISPOSITION_ATTACHED_PIC doxy makes me think it's not possible. >> >> Personally, even if possible i think triggering a seek just to fetch an >> attachment is inefficient and disruptive to the user's demuxing/decoding >> process, and a step backwards considering it used to always be available, >> for both seekable and non-seekable input, the latter which will no longer be >> able to access the picture past the original packet. > > As I said, at worse it accesses a field just like now. So of course it > can be done. > > The point is that having a function to get the packet rather than just a > pointer gives us more freedom to extend the API later. Not against it in that case, even if it will probably never stop just being an abstraction layer to fetch an always available packet, but know that this will kill Andreas' idea of defining how the (until now) public packet can be used to simplify attached pics in muxing scenarios, or make it considerably more complex to the point it's no longer worth it. > > Regards, > > > _______________________________________________ > 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 3/29/2021 10:46 AM, James Almer wrote: > On 3/29/2021 10:20 AM, Nicolas George wrote: >> James Almer (12021-03-29): >>> Can this be done? id3v2 attached pics for many formats are handled by >>> the >>> generic demux code in avformat_open_input() and not by the actual >>> demuxer. >>> And in demuxers like asf, it seems to be done in read_header(). >>> Would seeking be able to fetch these pictures again? The comment in the >>> AV_DISPOSITION_ATTACHED_PIC doxy makes me think it's not possible. >>> >>> Personally, even if possible i think triggering a seek just to fetch an >>> attachment is inefficient and disruptive to the user's demuxing/decoding >>> process, and a step backwards considering it used to always be >>> available, >>> for both seekable and non-seekable input, the latter which will no >>> longer be >>> able to access the picture past the original packet. >> >> As I said, at worse it accesses a field just like now. So of course it >> can be done. >> >> The point is that having a function to get the packet rather than just a >> pointer gives us more freedom to extend the API later. > > Not against it in that case, even if it will probably never stop just > being an abstraction layer to fetch an always available packet, but know > that this will kill Andreas' idea of defining how the (until now) public > packet can be used to simplify attached pics in muxing scenarios, or > make it considerably more complex to the point it's no longer worth it. Some people on IRC mentioned the possibility of adding a demuxer option to disable fetching/storing attached pictures. The streams would not be allocated at all in that case. Would that be ok?
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 + */ + 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; + 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);
As a replacement for attached_pic, which is in turn deprecated and scheduled for removal. Signed-off-by: James Almer <jamrial@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(-)