[FFmpeg-devel] avformat: deprecate another ffserver API leftover

Submitted by wm4 on Jan. 15, 2018, 12:11 p.m.

Details

Message ID 20180115121119.11061-1-nfxjfg@googlemail.com
State New
Headers show

Commit Message

wm4 Jan. 15, 2018, 12:11 p.m.
---
 doc/APIchanges         | 4 ++++
 libavformat/avformat.h | 2 ++
 libavformat/utils.c    | 8 ++++++++
 libavformat/version.h  | 5 ++++-
 4 files changed, 18 insertions(+), 1 deletion(-)

Comments

James Almer Jan. 15, 2018, 2:29 p.m.
On 1/15/2018 9:11 AM, wm4 wrote:
> ---
>  doc/APIchanges         | 4 ++++
>  libavformat/avformat.h | 2 ++
>  libavformat/utils.c    | 8 ++++++++
>  libavformat/version.h  | 5 ++++-
>  4 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index d66c842521..0184815224 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,10 @@ libavutil:     2017-10-21
>  
>  API changes, most recent first:
>  
> +2018-01-xx - xxxxxxx - lavf 58.4.100 - avformat.h
> +  Deprecate AVStream.recommended_encoder_configuration. It was useful only for
> +  FFserver, which has been removed.
> +
>  2018-01-xx - xxxxxxx - lavfi 7.11.101 - avfilter.h
>    Deprecate avfilter_link_get_channels(). Use av_buffersink_get_channels().
>  
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 4f2798a871..6f4b58b14b 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -986,12 +986,14 @@ typedef struct AVStream {
>       */
>      AVRational r_frame_rate;
>  
> +#if FF_API_LAVF_FFSERVER
>      /**
>       * String containing pairs of key and values describing recommended encoder configuration.
>       * Pairs are separated by ','.
>       * Keys are separated from values by '='.
>       */
>      char *recommended_encoder_configuration;

Missing the attribute.

Also, this field was moved to the public portion of AVStream by me in
307c24b32f (at the start of this ABI unstable period), and its accessors
deprecated in b7785d10b0, which should also get wrapped with this new
FF_API_ check you're adding.

I'd say move it back below the notice to make it clear it shouldn't be
used, assuming the ABI unstable period is still going, but it's fine
either way.

> +#endif
>  
>      /**
>       * Codec parameters associated with this stream. Allocated and freed by
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 2185a6f05b..1101c50de2 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -4256,6 +4256,8 @@ int ff_stream_encode_params_copy(AVStream *dst, const AVStream *src)
>          }
>      }
>  
> +#if FF_API_LAVF_FFSERVER
> +FF_DISABLE_DEPRECATION_WARNINGS
>      av_freep(&dst->recommended_encoder_configuration);
>      if (src->recommended_encoder_configuration) {
>          const char *conf_str = src->recommended_encoder_configuration;
> @@ -4263,6 +4265,8 @@ int ff_stream_encode_params_copy(AVStream *dst, const AVStream *src)
>          if (!dst->recommended_encoder_configuration)
>              return AVERROR(ENOMEM);
>      }
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
>  
>      return 0;
>  }
> @@ -4310,7 +4314,11 @@ FF_ENABLE_DEPRECATION_WARNINGS
>      if (st->info)
>          av_freep(&st->info->duration_error);
>      av_freep(&st->info);
> +#if FF_API_LAVF_FFSERVER
> +FF_DISABLE_DEPRECATION_WARNINGS
>      av_freep(&st->recommended_encoder_configuration);
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
>  
>      av_freep(pst);
>  }

You should also wrap the MAKE_ACCESSORS line for it even if it's inside
the FF_API_FORMAT_GET_SET check since b7785d10b0. If anything to add the
disable/enable deprecation warnings macros for it.

> diff --git a/libavformat/version.h b/libavformat/version.h
> index 5ced041f0a..d566e255e5 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   3
> +#define LIBAVFORMAT_VERSION_MINOR   4
>  #define LIBAVFORMAT_VERSION_MICRO 100
>  
>  #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
> @@ -82,6 +82,9 @@
>  #ifndef FF_API_OLD_AVIO_EOF_0
>  #define FF_API_OLD_AVIO_EOF_0           (LIBAVFORMAT_VERSION_MAJOR < 59)
>  #endif
> +#ifndef FF_API_LAVF_FFSERVER
> +#define FF_API_LAVF_FFSERVER            (LIBAVFORMAT_VERSION_MAJOR < 59)
> +#endif
>  
>  
>  #ifndef FF_API_R_FRAME_RATE
> 

LGTM in any case.
wm4 Jan. 16, 2018, 12:01 p.m.
On Mon, 15 Jan 2018 11:29:20 -0300
James Almer <jamrial@gmail.com> wrote:

> On 1/15/2018 9:11 AM, wm4 wrote:
> > ---
> >  doc/APIchanges         | 4 ++++
> >  libavformat/avformat.h | 2 ++
> >  libavformat/utils.c    | 8 ++++++++
> >  libavformat/version.h  | 5 ++++-
> >  4 files changed, 18 insertions(+), 1 deletion(-)
> > 
> > diff --git a/doc/APIchanges b/doc/APIchanges
> > index d66c842521..0184815224 100644
> > --- a/doc/APIchanges
> > +++ b/doc/APIchanges
> > @@ -15,6 +15,10 @@ libavutil:     2017-10-21
> >  
> >  API changes, most recent first:
> >  
> > +2018-01-xx - xxxxxxx - lavf 58.4.100 - avformat.h
> > +  Deprecate AVStream.recommended_encoder_configuration. It was useful only for
> > +  FFserver, which has been removed.
> > +
> >  2018-01-xx - xxxxxxx - lavfi 7.11.101 - avfilter.h
> >    Deprecate avfilter_link_get_channels(). Use av_buffersink_get_channels().
> >  
> > diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> > index 4f2798a871..6f4b58b14b 100644
> > --- a/libavformat/avformat.h
> > +++ b/libavformat/avformat.h
> > @@ -986,12 +986,14 @@ typedef struct AVStream {
> >       */
> >      AVRational r_frame_rate;
> >  
> > +#if FF_API_LAVF_FFSERVER
> >      /**
> >       * String containing pairs of key and values describing recommended encoder configuration.
> >       * Pairs are separated by ','.
> >       * Keys are separated from values by '='.
> >       */
> >      char *recommended_encoder_configuration;  
> 
> Missing the attribute.

Added.

> Also, this field was moved to the public portion of AVStream by me in
> 307c24b32f (at the start of this ABI unstable period), and its accessors
> deprecated in b7785d10b0, which should also get wrapped with this new
> FF_API_ check you're adding.
> 
> I'd say move it back below the notice to make it clear it shouldn't be
> used, assuming the ABI unstable period is still going, but it's fine
> either way.

It turns out I added an explicit note that some fields including
codecpar and this ffserver one are explicitly public. Assuming we're
really strict about public API deprecation, it seems better not to move
it back to the internal region, so I didn't.

(Of course it's likely that only ffserver used it.)

> > +#endif
> >  
> >      /**
> >       * Codec parameters associated with this stream. Allocated and freed by
> > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > index 2185a6f05b..1101c50de2 100644
> > --- a/libavformat/utils.c
> > +++ b/libavformat/utils.c
> > @@ -4256,6 +4256,8 @@ int ff_stream_encode_params_copy(AVStream *dst, const AVStream *src)
> >          }
> >      }
> >  
> > +#if FF_API_LAVF_FFSERVER
> > +FF_DISABLE_DEPRECATION_WARNINGS
> >      av_freep(&dst->recommended_encoder_configuration);
> >      if (src->recommended_encoder_configuration) {
> >          const char *conf_str = src->recommended_encoder_configuration;
> > @@ -4263,6 +4265,8 @@ int ff_stream_encode_params_copy(AVStream *dst, const AVStream *src)
> >          if (!dst->recommended_encoder_configuration)
> >              return AVERROR(ENOMEM);
> >      }
> > +FF_ENABLE_DEPRECATION_WARNINGS
> > +#endif
> >  
> >      return 0;
> >  }
> > @@ -4310,7 +4314,11 @@ FF_ENABLE_DEPRECATION_WARNINGS
> >      if (st->info)
> >          av_freep(&st->info->duration_error);
> >      av_freep(&st->info);
> > +#if FF_API_LAVF_FFSERVER
> > +FF_DISABLE_DEPRECATION_WARNINGS
> >      av_freep(&st->recommended_encoder_configuration);
> > +FF_ENABLE_DEPRECATION_WARNINGS
> > +#endif
> >  
> >      av_freep(pst);
> >  }  
> 
> You should also wrap the MAKE_ACCESSORS line for it even if it's inside
> the FF_API_FORMAT_GET_SET check since b7785d10b0. If anything to add the
> disable/enable deprecation warnings macros for it.

Done.

> > diff --git a/libavformat/version.h b/libavformat/version.h
> > index 5ced041f0a..d566e255e5 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   3
> > +#define LIBAVFORMAT_VERSION_MINOR   4
> >  #define LIBAVFORMAT_VERSION_MICRO 100
> >  
> >  #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
> > @@ -82,6 +82,9 @@
> >  #ifndef FF_API_OLD_AVIO_EOF_0
> >  #define FF_API_OLD_AVIO_EOF_0           (LIBAVFORMAT_VERSION_MAJOR < 59)
> >  #endif
> > +#ifndef FF_API_LAVF_FFSERVER
> > +#define FF_API_LAVF_FFSERVER            (LIBAVFORMAT_VERSION_MAJOR < 59)
> > +#endif
> >  
> >  
> >  #ifndef FF_API_R_FRAME_RATE
> >   
> 
> LGTM in any case.

Pushed.

Patch hide | download patch | download mbox

diff --git a/doc/APIchanges b/doc/APIchanges
index d66c842521..0184815224 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,10 @@  libavutil:     2017-10-21
 
 API changes, most recent first:
 
+2018-01-xx - xxxxxxx - lavf 58.4.100 - avformat.h
+  Deprecate AVStream.recommended_encoder_configuration. It was useful only for
+  FFserver, which has been removed.
+
 2018-01-xx - xxxxxxx - lavfi 7.11.101 - avfilter.h
   Deprecate avfilter_link_get_channels(). Use av_buffersink_get_channels().
 
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 4f2798a871..6f4b58b14b 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -986,12 +986,14 @@  typedef struct AVStream {
      */
     AVRational r_frame_rate;
 
+#if FF_API_LAVF_FFSERVER
     /**
      * String containing pairs of key and values describing recommended encoder configuration.
      * Pairs are separated by ','.
      * Keys are separated from values by '='.
      */
     char *recommended_encoder_configuration;
+#endif
 
     /**
      * Codec parameters associated with this stream. Allocated and freed by
diff --git a/libavformat/utils.c b/libavformat/utils.c
index 2185a6f05b..1101c50de2 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -4256,6 +4256,8 @@  int ff_stream_encode_params_copy(AVStream *dst, const AVStream *src)
         }
     }
 
+#if FF_API_LAVF_FFSERVER
+FF_DISABLE_DEPRECATION_WARNINGS
     av_freep(&dst->recommended_encoder_configuration);
     if (src->recommended_encoder_configuration) {
         const char *conf_str = src->recommended_encoder_configuration;
@@ -4263,6 +4265,8 @@  int ff_stream_encode_params_copy(AVStream *dst, const AVStream *src)
         if (!dst->recommended_encoder_configuration)
             return AVERROR(ENOMEM);
     }
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
 
     return 0;
 }
@@ -4310,7 +4314,11 @@  FF_ENABLE_DEPRECATION_WARNINGS
     if (st->info)
         av_freep(&st->info->duration_error);
     av_freep(&st->info);
+#if FF_API_LAVF_FFSERVER
+FF_DISABLE_DEPRECATION_WARNINGS
     av_freep(&st->recommended_encoder_configuration);
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
 
     av_freep(pst);
 }
diff --git a/libavformat/version.h b/libavformat/version.h
index 5ced041f0a..d566e255e5 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   3
+#define LIBAVFORMAT_VERSION_MINOR   4
 #define LIBAVFORMAT_VERSION_MICRO 100
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
@@ -82,6 +82,9 @@ 
 #ifndef FF_API_OLD_AVIO_EOF_0
 #define FF_API_OLD_AVIO_EOF_0           (LIBAVFORMAT_VERSION_MAJOR < 59)
 #endif
+#ifndef FF_API_LAVF_FFSERVER
+#define FF_API_LAVF_FFSERVER            (LIBAVFORMAT_VERSION_MAJOR < 59)
+#endif
 
 
 #ifndef FF_API_R_FRAME_RATE