diff mbox series

[FFmpeg-devel,6/6] avformat: deprecate some mpegts details from AVStream

Message ID 20201228224929.21299-6-cus@passwd.hu
State New
Headers show
Series [FFmpeg-devel,1/6] avformat/utils: do not overwrite already existing program with defaults in av_new_program
Related show

Checks

Context Check Description
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

Commit Message

Marton Balint Dec. 28, 2020, 10:49 p.m. UTC
These fields were added to support -merge_pmt_versions, but the mpegts demuxer
is also keeping track its programs internally, so that should be a better place
to handle it.

Also it is not a very good API to provide fields like program_num or
pmt_stream_idx in an AVStream, because a single stream can be part of multiple
programs, multiple PMTs, so the stream attributes can refer to any program the
stream is part of.

Considering that these were added only 2 years ago, and their usefulness was
limited in the first place, I think the best approach is to deprecate them. If
users should come forward wanting such functionality, then adding it to
AVProgram seems the better place to be able to better support having the same
stream in multiple programs.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 doc/APIchanges         |  3 +++
 libavformat/avformat.h |  5 +++++
 libavformat/mpegts.c   | 12 ++++++++++++
 libavformat/version.h  |  5 ++++-
 4 files changed, 24 insertions(+), 1 deletion(-)

Comments

James Almer Dec. 29, 2020, 1:11 p.m. UTC | #1
On 12/28/2020 7:49 PM, Marton Balint wrote:
> These fields were added to support -merge_pmt_versions, but the mpegts demuxer
> is also keeping track its programs internally, so that should be a better place
> to handle it.
> 
> Also it is not a very good API to provide fields like program_num or
> pmt_stream_idx in an AVStream, because a single stream can be part of multiple
> programs, multiple PMTs, so the stream attributes can refer to any program the
> stream is part of.
> 
> Considering that these were added only 2 years ago, and their usefulness was
> limited in the first place, I think the best approach is to deprecate them.

Just remove them. They are not public (See the notice immediately after 
the codecpar field). No need to deprecate them, add any kind of entry to 
APIChanges, or bump the library version.

> If users should come forward wanting such functionality, then adding it to
> AVProgram seems the better place to be able to better support having the same
> stream in multiple programs.
> 
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>   doc/APIchanges         |  3 +++
>   libavformat/avformat.h |  5 +++++
>   libavformat/mpegts.c   | 12 ++++++++++++
>   libavformat/version.h  |  5 ++++-
>   4 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 3fb9e12525..b901133f63 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,9 @@ libavutil:     2017-10-21
>   
>   API changes, most recent first:
>   
> +2021-01-xx - xxxxxxxxxx - lavf 58.66.100 - avformat.h
> +  Deprecate AVStream program_num, pmt_version, pmt_stream_idx.
> +
>   2020-12-03 - xxxxxxxxxx - lavu 56.62.100 - timecode.h
>     Add av_timecode_init_from_components.
>   
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 4865c56cc3..89a53056f2 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -1096,12 +1096,17 @@ typedef struct AVStream {
>        */
>       int stream_identifier;
>   
> +#if FF_API_MPEGTS_AVSTREAM
>       /**
>        * Details of the MPEG-TS program which created this stream.
>        */
> +    attribute_deprecated
>       int program_num;
> +    attribute_deprecated
>       int pmt_version;
> +    attribute_deprecated
>       int pmt_stream_idx;
> +#endif

Like i said above, you'd normally just remove this kind of field and 
that's it, since sizeof(AVStream) is not part of the ABI and none of 
these fields are to be accessed outside lavf. But since lavf is 
currently in an unholy relationship with lavd, we need to keep the size 
of the struct unchanged until the soname bump.

So what you need to do is the same thing Anton did for other fields 
above these. Rename them to unused#, and wrap them in a simple 
LIBAVFORMAT_VERSION_MAJOR < 59 check.

You can remove all the uses in mpegts.c safely.

>   
>       /**
>        * An opaque field for libavformat internal usage.
> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> index 9a3fd56ece..6d9bdbc485 100644
> --- a/libavformat/mpegts.c
> +++ b/libavformat/mpegts.c
> @@ -2406,9 +2406,13 @@ static void pmt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len
>                   if (!pes->st)
>                       goto out;
>                   pes->st->id = pes->pid;
> +#if FF_API_MPEGTS_AVSTREAM
> +FF_DISABLE_DEPRECATION_WARNINGS
>                   pes->st->program_num = h->id;
>                   pes->st->pmt_version = h->version;
>                   pes->st->pmt_stream_idx = i;
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
>               }
>               st = pes->st;
>           } else if (is_pes_stream(stream_type, prog_reg_desc)) {
> @@ -2428,9 +2432,13 @@ static void pmt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len
>                   if (!st)
>                       goto out;
>                   st->id = pes->pid;
> +#if FF_API_MPEGTS_AVSTREAM
> +FF_DISABLE_DEPRECATION_WARNINGS
>                   st->program_num = h->id;
>                   st->pmt_version = h->version;
>                   st->pmt_stream_idx = i;
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
>               }
>           } else {
>               int idx = ff_find_stream_index(ts->stream, pid);
> @@ -2445,9 +2453,13 @@ static void pmt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len
>                   if (!st)
>                       goto out;
>                   st->id = pid;
> +#if FF_API_MPEGTS_AVSTREAM
> +FF_DISABLE_DEPRECATION_WARNINGS
>                   st->program_num = h->id;
>                   st->pmt_version = h->version;
>                   st->pmt_stream_idx = i;
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
>                   st->codecpar->codec_type = AVMEDIA_TYPE_DATA;
>                   if (stream_type == 0x86 && prog_reg_desc == AV_RL32("CUEI")) {
>                       mpegts_find_stream_type(st, stream_type, SCTE_types);
> diff --git a/libavformat/version.h b/libavformat/version.h
> index b43193bcb1..c211c13477 100644
> --- a/libavformat/version.h
> +++ b/libavformat/version.h
> @@ -32,7 +32,7 @@
>   // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
>   // Also please add any ticket numbers that you believe might be affected here
>   #define LIBAVFORMAT_VERSION_MAJOR  58
> -#define LIBAVFORMAT_VERSION_MINOR  65
> +#define LIBAVFORMAT_VERSION_MINOR  66
>   #define LIBAVFORMAT_VERSION_MICRO 100
>   
>   #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
> @@ -106,6 +106,9 @@
>   #ifndef FF_API_AVIOFORMAT
>   #define FF_API_AVIOFORMAT               (LIBAVFORMAT_VERSION_MAJOR < 59)
>   #endif
> +#ifndef FF_API_MPEGTS_AVSTREAM
> +#define FF_API_MPEGTS_AVSTREAM          (LIBAVFORMAT_VERSION_MAJOR < 59)
> +#endif
>   
>   
>   #ifndef FF_API_R_FRAME_RATE
>
diff mbox series

Patch

diff --git a/doc/APIchanges b/doc/APIchanges
index 3fb9e12525..b901133f63 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,9 @@  libavutil:     2017-10-21
 
 API changes, most recent first:
 
+2021-01-xx - xxxxxxxxxx - lavf 58.66.100 - avformat.h
+  Deprecate AVStream program_num, pmt_version, pmt_stream_idx.
+
 2020-12-03 - xxxxxxxxxx - lavu 56.62.100 - timecode.h
   Add av_timecode_init_from_components.
 
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 4865c56cc3..89a53056f2 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -1096,12 +1096,17 @@  typedef struct AVStream {
      */
     int stream_identifier;
 
+#if FF_API_MPEGTS_AVSTREAM
     /**
      * Details of the MPEG-TS program which created this stream.
      */
+    attribute_deprecated
     int program_num;
+    attribute_deprecated
     int pmt_version;
+    attribute_deprecated
     int pmt_stream_idx;
+#endif
 
     /**
      * An opaque field for libavformat internal usage.
diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
index 9a3fd56ece..6d9bdbc485 100644
--- a/libavformat/mpegts.c
+++ b/libavformat/mpegts.c
@@ -2406,9 +2406,13 @@  static void pmt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len
                 if (!pes->st)
                     goto out;
                 pes->st->id = pes->pid;
+#if FF_API_MPEGTS_AVSTREAM
+FF_DISABLE_DEPRECATION_WARNINGS
                 pes->st->program_num = h->id;
                 pes->st->pmt_version = h->version;
                 pes->st->pmt_stream_idx = i;
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
             }
             st = pes->st;
         } else if (is_pes_stream(stream_type, prog_reg_desc)) {
@@ -2428,9 +2432,13 @@  static void pmt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len
                 if (!st)
                     goto out;
                 st->id = pes->pid;
+#if FF_API_MPEGTS_AVSTREAM
+FF_DISABLE_DEPRECATION_WARNINGS
                 st->program_num = h->id;
                 st->pmt_version = h->version;
                 st->pmt_stream_idx = i;
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
             }
         } else {
             int idx = ff_find_stream_index(ts->stream, pid);
@@ -2445,9 +2453,13 @@  static void pmt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len
                 if (!st)
                     goto out;
                 st->id = pid;
+#if FF_API_MPEGTS_AVSTREAM
+FF_DISABLE_DEPRECATION_WARNINGS
                 st->program_num = h->id;
                 st->pmt_version = h->version;
                 st->pmt_stream_idx = i;
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
                 st->codecpar->codec_type = AVMEDIA_TYPE_DATA;
                 if (stream_type == 0x86 && prog_reg_desc == AV_RL32("CUEI")) {
                     mpegts_find_stream_type(st, stream_type, SCTE_types);
diff --git a/libavformat/version.h b/libavformat/version.h
index b43193bcb1..c211c13477 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -32,7 +32,7 @@ 
 // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
 // Also please add any ticket numbers that you believe might be affected here
 #define LIBAVFORMAT_VERSION_MAJOR  58
-#define LIBAVFORMAT_VERSION_MINOR  65
+#define LIBAVFORMAT_VERSION_MINOR  66
 #define LIBAVFORMAT_VERSION_MICRO 100
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
@@ -106,6 +106,9 @@ 
 #ifndef FF_API_AVIOFORMAT
 #define FF_API_AVIOFORMAT               (LIBAVFORMAT_VERSION_MAJOR < 59)
 #endif
+#ifndef FF_API_MPEGTS_AVSTREAM
+#define FF_API_MPEGTS_AVSTREAM          (LIBAVFORMAT_VERSION_MAJOR < 59)
+#endif
 
 
 #ifndef FF_API_R_FRAME_RATE