[FFmpeg-devel] [PATCH] lavc/packet: deprecate AVPacket.time_base

[Nicolas George]

The contents of AVPacket can only make sense with some extra information.
In FFmpeg, this information is carried by AVStream. Applications can use
AVStream or define their own data structure for that. There is no reason
to carry one of these informations everywhere along with the packet.

Furthermore, since this field is only set by libavformat and never used
by any part of the code, we have absolutely no test coverage. That means
it can become out of sync with what it should be and we would not
notice. As a general principle, having the same redundant information in
several data structures and expecting it to stay in sync is a bad and
risky practice.

Signed-off-by: Nicolas George <george at nsup.org>
---
 libavcodec/avpacket.c | 12 ++++++++++++
 libavcodec/packet.h   |  7 +++++++
 libavcodec/version.h  |  3 +++
 libavformat/mux.c     |  8 ++++++++
 4 files changed, 30 insertions(+)


Please believe that I am not sending this patch to be contrarian. I
genuinely believe it is a bad idea, and the paragraphs above explain
why.

This field has been added only recently, that means we can remove it
without a lot of damage. It also means we have managed without it for
years, and if we could, then applications should be able too. They just
have to do what we do: include a time_base field in their data structure
describing stream.


diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
index d8d8fef3b9..79ff35dd68 100644
--- a/libavcodec/avpacket.c
+++ b/libavcodec/avpacket.c
@@ -47,7 +47,11 @@ void av_init_packet(AVPacket *pkt)
     pkt->side_data_elems      = 0;
     pkt->opaque               = NULL;
     pkt->opaque_ref           = NULL;
+#if FF_API_PACKET_TIMEBASE
+FF_DISABLE_DEPRECATION_WARNINGS
     pkt->time_base            = av_make_q(0, 1);
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
 }
 #endif
 
@@ -58,7 +62,11 @@ static void get_packet_defaults(AVPacket *pkt)
     pkt->pts             = AV_NOPTS_VALUE;
     pkt->dts             = AV_NOPTS_VALUE;
     pkt->pos             = -1;
+#if FF_API_PACKET_TIMEBASE
+FF_DISABLE_DEPRECATION_WARNINGS
     pkt->time_base       = av_make_q(0, 1);
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
 }
 
 AVPacket *av_packet_alloc(void)
@@ -388,7 +396,11 @@ int av_packet_copy_props(AVPacket *dst, const AVPacket *src)
     dst->flags                = src->flags;
     dst->stream_index         = src->stream_index;
     dst->opaque               = src->opaque;
+#if FF_API_PACKET_TIMEBASE
+FF_DISABLE_DEPRECATION_WARNINGS
     dst->time_base            = src->time_base;
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
     dst->opaque_ref           = NULL;
     dst->side_data            = NULL;
     dst->side_data_elems      = 0;
diff --git a/libavcodec/packet.h b/libavcodec/packet.h
index 9baff24635..e411fad06b 100644
--- a/libavcodec/packet.h
+++ b/libavcodec/packet.h
@@ -408,10 +408,17 @@ typedef struct AVPacket {
      */
     AVBufferRef *opaque_ref;
 
+#if FF_API_PACKET_TIMEBASE
     /**
      * Time base of the packet's timestamps.
+     * Deprecated: the time base is specified by AVStream->time_base;
+     * applications need to propagate it if they separate a packet from its
+     * stream.
      */
+    attribute_deprecated
     AVRational time_base;
+#endif
+
 } AVPacket;
 
 #if FF_API_INIT_PACKET
diff --git a/libavcodec/version.h b/libavcodec/version.h
index 4b4fe543ab..c84267243d 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -81,5 +81,8 @@
 #ifndef FF_API_MPEGVIDEO_OPTS
 #define FF_API_MPEGVIDEO_OPTS      (LIBAVCODEC_VERSION_MAJOR < 60)
 #endif
+#ifndef FF_API_PACKET_TIMEBASE
+#define FF_API_PACKET_TIMEBASE     (LIBAVCODEC_VERSION_MAJOR < 60)
+#endif
 
 #endif /* AVCODEC_VERSION_H */
diff --git a/libavformat/mux.c b/libavformat/mux.c
index 6ba1306f2b..6e9b2e7538 100644
--- a/libavformat/mux.c
+++ b/libavformat/mux.c
@@ -1293,7 +1293,11 @@ int ff_write_chained(AVFormatContext *dst, int dst_stream, AVPacket *pkt,
 {
     int64_t pts = pkt->pts, dts = pkt->dts, duration = pkt->duration;
     int stream_index = pkt->stream_index;
+#if FF_API_PACKET_TIMEBASE
+FF_DISABLE_DEPRECATION_WARNINGS
     AVRational time_base = pkt->time_base;
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
     int ret;
 
     pkt->stream_index = dst_stream;
@@ -1311,7 +1315,11 @@ int ff_write_chained(AVFormatContext *dst, int dst_stream, AVPacket *pkt,
         pkt->dts          = dts;
         pkt->duration     = duration;
         pkt->stream_index = stream_index;
+#if FF_API_PACKET_TIMEBASE
+FF_DISABLE_DEPRECATION_WARNINGS
         pkt->time_base    = time_base;
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
     } else
         ret = av_interleaved_write_frame(dst, pkt);
 
-- 
2.33.0