diff mbox series

[FFmpeg-devel,6/6] lavc: un-avpriv avpriv_bprint_to_extradata()

Message ID 20201026134159.24101-6-anton@khirnov.net
State Accepted
Commit 04385218885935df99ec9f98ab73bb461775d28b
Headers show
Series [FFmpeg-devel,1/6] lavf/latmenc: fix units mismatch | expand

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

Anton Khirnov Oct. 26, 2020, 1:41 p.m. UTC
It has not been used outside of lavc since 6f69f7a8bf6.

Also, move it to the only place where it is used.
---
 libavcodec/dvdsubenc.c | 25 ++++++++++++++++++++++++-
 libavcodec/internal.h  |  5 -----
 libavcodec/utils.c     | 23 -----------------------
 3 files changed, 24 insertions(+), 29 deletions(-)

Comments

James Almer Oct. 27, 2020, 12:10 a.m. UTC | #1
On 10/26/2020 10:41 AM, Anton Khirnov wrote:
> It has not been used outside of lavc since 6f69f7a8bf6.
> 
> Also, move it to the only place where it is used.

Shouldn't you keep the symbol around until the bump? Even though it was
not used by other libraries, it was nonetheless still exported by lavc.
A simple version define preprocessor check like Andreas mentioned is
enough, no need to add a new FF_API define for avpriv functions.

Alternatively, just postpone applying this until after the bump.

LGTM either way.

> ---
>  libavcodec/dvdsubenc.c | 25 ++++++++++++++++++++++++-
>  libavcodec/internal.h  |  5 -----
>  libavcodec/utils.c     | 23 -----------------------
>  3 files changed, 24 insertions(+), 29 deletions(-)
> 
> diff --git a/libavcodec/dvdsubenc.c b/libavcodec/dvdsubenc.c
> index e54b5f0d7b..9fa9d5b6d7 100644
> --- a/libavcodec/dvdsubenc.c
> +++ b/libavcodec/dvdsubenc.c
> @@ -424,6 +424,29 @@ fail:
>      return ret;
>  }
>  
> +static int bprint_to_extradata(AVCodecContext *avctx, struct AVBPrint *buf)
> +{
> +    int ret;
> +    char *str;
> +
> +    ret = av_bprint_finalize(buf, &str);
> +    if (ret < 0)
> +        return ret;
> +    if (!av_bprint_is_complete(buf)) {
> +        av_free(str);
> +        return AVERROR(ENOMEM);
> +    }
> +
> +    avctx->extradata = str;
> +    /* Note: the string is NUL terminated (so extradata can be read as a
> +     * string), but the ending character is not accounted in the size (in
> +     * binary formats you are likely not supposed to mux that character). When
> +     * extradata is copied, it is also padded with AV_INPUT_BUFFER_PADDING_SIZE
> +     * zeros. */
> +    avctx->extradata_size = buf->len;
> +    return 0;
> +}
> +
>  static int dvdsub_init(AVCodecContext *avctx)
>  {
>      DVDSubtitleContext *dvdc = avctx->priv_data;
> @@ -451,7 +474,7 @@ static int dvdsub_init(AVCodecContext *avctx)
>          av_bprintf(&extradata, " %06"PRIx32"%c",
>                     dvdc->global_palette[i] & 0xFFFFFF, i < 15 ? ',' : '\n');
>  
> -    ret = avpriv_bprint_to_extradata(avctx, &extradata);
> +    ret = bprint_to_extradata(avctx, &extradata);
>      if (ret < 0)
>          return ret;
>  
> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> index ce4dbbc2b9..17defb9b50 100644
> --- a/libavcodec/internal.h
> +++ b/libavcodec/internal.h
> @@ -313,11 +313,6 @@ int avpriv_h264_has_num_reorder_frames(AVCodecContext *avctx);
>   */
>  int ff_codec_open2_recursive(AVCodecContext *avctx, const AVCodec *codec, AVDictionary **options);
>  
> -/**
> - * Finalize buf into extradata and set its size appropriately.
> - */
> -int avpriv_bprint_to_extradata(AVCodecContext *avctx, struct AVBPrint *buf);
> -
>  const uint8_t *avpriv_find_start_code(const uint8_t *p,
>                                        const uint8_t *end,
>                                        uint32_t *state);
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index 93ac1cd9f0..fb40962c47 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -1946,29 +1946,6 @@ int avcodec_is_open(AVCodecContext *s)
>      return !!s->internal;
>  }
>  
> -int avpriv_bprint_to_extradata(AVCodecContext *avctx, struct AVBPrint *buf)
> -{
> -    int ret;
> -    char *str;
> -
> -    ret = av_bprint_finalize(buf, &str);
> -    if (ret < 0)
> -        return ret;
> -    if (!av_bprint_is_complete(buf)) {
> -        av_free(str);
> -        return AVERROR(ENOMEM);
> -    }
> -
> -    avctx->extradata = str;
> -    /* Note: the string is NUL terminated (so extradata can be read as a
> -     * string), but the ending character is not accounted in the size (in
> -     * binary formats you are likely not supposed to mux that character). When
> -     * extradata is copied, it is also padded with AV_INPUT_BUFFER_PADDING_SIZE
> -     * zeros. */
> -    avctx->extradata_size = buf->len;
> -    return 0;
> -}
> -
>  const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
>                                        const uint8_t *end,
>                                        uint32_t *av_restrict state)
>
Anton Khirnov Oct. 27, 2020, 9:51 a.m. UTC | #2
Quoting James Almer (2020-10-27 01:10:47)
> On 10/26/2020 10:41 AM, Anton Khirnov wrote:
> > It has not been used outside of lavc since 6f69f7a8bf6.
> > 
> > Also, move it to the only place where it is used.
> 
> Shouldn't you keep the symbol around until the bump? Even though it was
> not used by other libraries, it was nonetheless still exported by lavc.
> A simple version define preprocessor check like Andreas mentioned is
> enough, no need to add a new FF_API define for avpriv functions.

Just being exported doesn't mean we have to maintain compatibility for
it, IMO. The only code that could have legally used it is in other libs,
but there is no such code since before last bump.
diff mbox series

Patch

diff --git a/libavcodec/dvdsubenc.c b/libavcodec/dvdsubenc.c
index e54b5f0d7b..9fa9d5b6d7 100644
--- a/libavcodec/dvdsubenc.c
+++ b/libavcodec/dvdsubenc.c
@@ -424,6 +424,29 @@  fail:
     return ret;
 }
 
+static int bprint_to_extradata(AVCodecContext *avctx, struct AVBPrint *buf)
+{
+    int ret;
+    char *str;
+
+    ret = av_bprint_finalize(buf, &str);
+    if (ret < 0)
+        return ret;
+    if (!av_bprint_is_complete(buf)) {
+        av_free(str);
+        return AVERROR(ENOMEM);
+    }
+
+    avctx->extradata = str;
+    /* Note: the string is NUL terminated (so extradata can be read as a
+     * string), but the ending character is not accounted in the size (in
+     * binary formats you are likely not supposed to mux that character). When
+     * extradata is copied, it is also padded with AV_INPUT_BUFFER_PADDING_SIZE
+     * zeros. */
+    avctx->extradata_size = buf->len;
+    return 0;
+}
+
 static int dvdsub_init(AVCodecContext *avctx)
 {
     DVDSubtitleContext *dvdc = avctx->priv_data;
@@ -451,7 +474,7 @@  static int dvdsub_init(AVCodecContext *avctx)
         av_bprintf(&extradata, " %06"PRIx32"%c",
                    dvdc->global_palette[i] & 0xFFFFFF, i < 15 ? ',' : '\n');
 
-    ret = avpriv_bprint_to_extradata(avctx, &extradata);
+    ret = bprint_to_extradata(avctx, &extradata);
     if (ret < 0)
         return ret;
 
diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index ce4dbbc2b9..17defb9b50 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -313,11 +313,6 @@  int avpriv_h264_has_num_reorder_frames(AVCodecContext *avctx);
  */
 int ff_codec_open2_recursive(AVCodecContext *avctx, const AVCodec *codec, AVDictionary **options);
 
-/**
- * Finalize buf into extradata and set its size appropriately.
- */
-int avpriv_bprint_to_extradata(AVCodecContext *avctx, struct AVBPrint *buf);
-
 const uint8_t *avpriv_find_start_code(const uint8_t *p,
                                       const uint8_t *end,
                                       uint32_t *state);
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 93ac1cd9f0..fb40962c47 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -1946,29 +1946,6 @@  int avcodec_is_open(AVCodecContext *s)
     return !!s->internal;
 }
 
-int avpriv_bprint_to_extradata(AVCodecContext *avctx, struct AVBPrint *buf)
-{
-    int ret;
-    char *str;
-
-    ret = av_bprint_finalize(buf, &str);
-    if (ret < 0)
-        return ret;
-    if (!av_bprint_is_complete(buf)) {
-        av_free(str);
-        return AVERROR(ENOMEM);
-    }
-
-    avctx->extradata = str;
-    /* Note: the string is NUL terminated (so extradata can be read as a
-     * string), but the ending character is not accounted in the size (in
-     * binary formats you are likely not supposed to mux that character). When
-     * extradata is copied, it is also padded with AV_INPUT_BUFFER_PADDING_SIZE
-     * zeros. */
-    avctx->extradata_size = buf->len;
-    return 0;
-}
-
 const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
                                       const uint8_t *end,
                                       uint32_t *av_restrict state)