[FFmpeg-devel] [PATCH 1/2] avcodec/h264_parse: change prefix to avpriv for usage in avformat mxf muxer

[Mark Thompson]

On 09/04/2019 23:14, Baptiste Coudurier wrote:
> ---
>  libavcodec/h264_parse.c  | 2 +-
>  libavcodec/h264_parser.c | 2 +-
>  libavcodec/h264_ps.c     | 4 ++--
>  libavcodec/h264_ps.h     | 4 ++--
>  libavcodec/h264dec.c     | 6 +++---
>  5 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
> index 17bfa780ce..980b1e189d 100644
> --- a/libavcodec/h264_ps.c
> +++ b/libavcodec/h264_ps.c
> @@ -330,8 +330,8 @@ void ff_h264_ps_uninit(H264ParamSets *ps)
>      ps->sps = NULL;
>  }
>  
> -int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx,
> -                                     H264ParamSets *ps, int ignore_truncation)
> +int avpriv_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx,
> +                                         H264ParamSets *ps, int ignore_truncation)
>  {
>      AVBufferRef *sps_buf;
>      int profile_idc, level_idc, constraint_set_flags = 0;
> diff --git a/libavcodec/h264_ps.h b/libavcodec/h264_ps.h
> index e967b9cbcf..d422ce122e 100644
> --- a/libavcodec/h264_ps.h
> +++ b/libavcodec/h264_ps.h
> @@ -149,8 +149,8 @@ typedef struct H264ParamSets {
>  /**
>   * Decode SPS
>   */
> -int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx,
> -                                     H264ParamSets *ps, int ignore_truncation);
> +int avpriv_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx,
> +                                         H264ParamSets *ps, int ignore_truncation);

Making the H.264 decoder's internal SPS and PPS structures fixed API really doesn't feel like a good idea to me, but I admit I don't have a better answer to the problem you're facing.  Copying everything is also pretty terrible.  Adding a new API to parse the headers and return the information you want might be nicer considering just this change, but extensibility for the inevitable subsequent patch which wants some extra piece of information in future would be a big pain.

If you're going to go with this, please add namespace prefixes to the structures it affects ("SPS" and "PPS", since you're now including them in code which isn't explicitly H.264), and also add big warnings to the header indicating that the structures are now fixed and can't be modified at all except on major bumps.  (Though maybe wait for other comments before actually making any changes.)

- Mark