On 1/27/2021 2:40 PM, Lynne wrote:
Jan 27, 2021, 16:07 by jamr...@gmail.com:
On 1/27/2021 6:16 AM, Anton Khirnov wrote:
Quoting James Almer (2021-01-26 20:11:16)
On 1/26/2021 1:17 PM, Anton Khirnov wrote:
We could start by adding a field to AVPacket that would be set to a
magic value by av_packet_alloc().
Then have e.g. AVCodecContext/AVFormatContext warn when they see a
packet without this magic value.
I don't like much the idea of adding a public field just to emit a
deprecation warning.
int internal_do_not_touch; // do not touch
is not really public. It is visible in the public headers, but so are
all the AVFooInternal. I agree that it is not the prettiest thing ever,
but it's not too bad.
And I believe it would solve a real problem, since we have few other
ways to let our users know they need to change something. Most of them
do not follow development closely, I'd think.
If sizeof(AVPacket) stops being part of the ABI, then av_init_packet() becomes
unnecessary, right? av_packet_unref() will be the only valid way for the user
to reuse the AVPacket. So that deprecation warning might be enough for stack
users.
I think just a compile-time deprecation on av_init_packet() would be enough.
Regarding a new internal field that lavf could check, i don't think it's enough
since it may be uninitialized for packets on stack. And you can't make
av_init_packet() set it to 0/NULL because then av_packet_unref() will also
reset it, and lavf will start printing bogus warnings for allocated packet
users.
I'd really rather not spam the log of users with API deprecation messages. We're
already guilty enough of doing that with color_range.
Ok, something like this patch, then?
Assuming we go through with it, I don't know what would be better, if to
push it now for ffmpeg 4.4 (Which should be branched out before the
bump), or after the bump.
The former would give a lot more time for downstreams to adapt, since it
will be in a release for the majority of the deprecation period rather
than exclusively on the master branch until the first post-bump release.
There's also a lot of stack usage for AVPacket within libav*, so we will
have to either remove them all alongside the deprecation, or at least
silence the av_init_packet() warnings while we slowly go through all of
them.
From 75d82c56aca9d1905865e0b1e40258ec0da61e6b Mon Sep 17 00:00:00 2001
From: James Almer <jamr...@gmail.com>
Date: Wed, 27 Jan 2021 16:24:10 -0300
Subject: [PATCH] avcodec/packet: deprecate av_init_packet()
Once removed, sizeof(AVPacket) will stop being a part of the public ABI.
Signed-off-by: James Almer <jamr...@gmail.com>
---
libavcodec/avpacket.c | 34 ++++++++++++++++++++++++++--------
libavcodec/packet.h | 19 +++++++++++++++----
libavcodec/version.h | 3 +++
3 files changed, 44 insertions(+), 12 deletions(-)
diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
index e4ba403cf6..e6cae39c81 100644
--- a/libavcodec/avpacket.c
+++ b/libavcodec/avpacket.c
@@ -32,6 +32,7 @@
#include "packet.h"
#include "packet_internal.h"
+#if FF_API_INIT_PACKET
void av_init_packet(AVPacket *pkt)
{
pkt->pts = AV_NOPTS_VALUE;
@@ -49,6 +50,27 @@ FF_ENABLE_DEPRECATION_WARNINGS
pkt->side_data = NULL;
pkt->side_data_elems = 0;
}
+#endif
+
+static void packet_init(AVPacket *pkt)
+{
+ pkt->pts = AV_NOPTS_VALUE;
+ pkt->dts = AV_NOPTS_VALUE;
+ pkt->pos = -1;
+ pkt->duration = 0;
+#if FF_API_CONVERGENCE_DURATION
+FF_DISABLE_DEPRECATION_WARNINGS
+ pkt->convergence_duration = 0;
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
+ pkt->flags = 0;
+ pkt->stream_index = 0;
+ pkt->buf = NULL;
+ pkt->data = NULL;
+ pkt->side_data = NULL;
+ pkt->side_data_elems = 0;
+ pkt->size = 0;
+}
AVPacket *av_packet_alloc(void)
{
@@ -56,7 +78,7 @@ AVPacket *av_packet_alloc(void)
if (!pkt)
return pkt;
- av_init_packet(pkt);
+ packet_init(pkt);
return pkt;
}
@@ -92,7 +114,7 @@ int av_new_packet(AVPacket *pkt, int size)
if (ret < 0)
return ret;
- av_init_packet(pkt);
+ packet_init(pkt);
pkt->buf = buf;
pkt->data = buf->data;
pkt->size = size;
@@ -607,9 +629,7 @@ void av_packet_unref(AVPacket *pkt)
{
av_packet_free_side_data(pkt);
av_buffer_unref(&pkt->buf);
- av_init_packet(pkt);
- pkt->data = NULL;
- pkt->size = 0;
+ packet_init(pkt);
}
int av_packet_ref(AVPacket *dst, const AVPacket *src)
@@ -664,9 +684,7 @@ AVPacket *av_packet_clone(const AVPacket *src)
void av_packet_move_ref(AVPacket *dst, AVPacket *src)
{
*dst = *src;
- av_init_packet(src);
- src->data = NULL;
- src->size = 0;
+ packet_init(src);
}
int av_packet_make_refcounted(AVPacket *pkt)
diff --git a/libavcodec/packet.h b/libavcodec/packet.h
index b9d4c9c2c8..d7f34d109e 100644
--- a/libavcodec/packet.h
+++ b/libavcodec/packet.h
@@ -319,10 +319,6 @@ typedef struct AVPacketSideData {
* packets, with no compressed data, containing only side data
* (e.g. to update some stream parameters at the end of encoding).
*
- * AVPacket is one of the few structs in FFmpeg, whose size is a part of public
- * ABI. Thus it may be allocated on stack and no new fields can be added to it
- * without libavcodec and libavformat major bump.
- *
* The semantics of data ownership depends on the buf field.
* If it is set, the packet data is dynamically allocated and is
* valid indefinitely until a call to av_packet_unref() reduces the
@@ -334,6 +330,12 @@ typedef struct AVPacketSideData {
* The side data is always allocated with av_malloc(), copied by
* av_packet_ref() and freed by av_packet_unref().
*
+ * sizeof(AVPacket) being a part of the public ABI is deprecated. once
+ * av_init_packet() is removed, new packets will only be able to be allocated
+ * with av_packet_alloc(), and new fields may be added to the end of the struct
+ * with a minor bump.
+ *
+ * @see av_packet_alloc
* @see av_packet_ref
* @see av_packet_unref
*/
@@ -460,6 +462,7 @@ AVPacket *av_packet_clone(const AVPacket *src);
*/
void av_packet_free(AVPacket **pkt);
+#if FF_API_INIT_PACKET
/**
* Initialize optional fields of a packet with default values.
*
@@ -467,8 +470,16 @@ void av_packet_free(AVPacket **pkt);
* initialized separately.
*
* @param pkt packet
+ *
+ * @see av_packet_alloc
+ * @see av_packet_unref
+ *
+ * @deprecated This function is deprecated. Once it's removed,
+ sizeof(AVPacket) will not be a part of the ABI anymore.
*/
+attribute_deprecated
void av_init_packet(AVPacket *pkt);
+#endif
/**
* Allocate the payload of a packet and initialize its fields with
diff --git a/libavcodec/version.h b/libavcodec/version.h
index 1e1bedfce6..af8487a4c1 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -153,5 +153,8 @@
#ifndef FF_API_DEBUG_MV
#define FF_API_DEBUG_MV (LIBAVCODEC_VERSION_MAJOR < 60)
#endif
+#ifndef FF_API_INIT_PACKET
+#define FF_API_INIT_PACKET (LIBAVCODEC_VERSION_MAJOR < 60)
+#endif
#endif /* AVCODEC_VERSION_H */
--
2.30.0
_______________________________________________
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".
