[FFmpeg-devel] [PATCH 10/18] cbs: Remove superfluous checks for ff_cbs_delete_unit

[James Almer]

On 6/17/2019 12:42 AM, Andreas Rheinhardt wrote:
> ff_cbs_delete_unit never fails if the index of the unit to delete is
> valid; document this behaviour explicitly and remove the checks for
> whether ff_cbs_delete_unit failed, because all the callers of
> ff_cbs_delete_unit already make sure that the index is valid.

Add a comment about why you're ignoring the return values in all three
filters. If for whatever reason the filters are changed in the future
and it can no longer be ensured the call will never fail, the developer
should be aware of it.

> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
> Several callers already ignored to check the return value.
> 
>  libavcodec/av1_metadata_bsf.c       | 9 ++-------
>  libavcodec/cbs.h                    | 2 ++
>  libavcodec/h264_metadata_bsf.c      | 7 +------
>  libavcodec/h264_redundant_pps_bsf.c | 4 +---
>  4 files changed, 6 insertions(+), 16 deletions(-)
> 
> diff --git a/libavcodec/av1_metadata_bsf.c b/libavcodec/av1_metadata_bsf.c
> index 842b80c201..dafddced63 100644
> --- a/libavcodec/av1_metadata_bsf.c
> +++ b/libavcodec/av1_metadata_bsf.c
> @@ -161,13 +161,8 @@ static int av1_metadata_filter(AVBSFContext *bsf, AVPacket *pkt)
>  
>      if (ctx->delete_padding) {
>          for (i = frag->nb_units - 1; i >= 0; i--) {
> -            if (frag->units[i].type == AV1_OBU_PADDING) {
> -                err = ff_cbs_delete_unit(ctx->cbc, frag, i);
> -                if (err < 0) {
> -                    av_log(bsf, AV_LOG_ERROR, "Failed to delete Padding OBU.\n");
> -                    goto fail;
> -                }
> -            }
> +            if (frag->units[i].type == AV1_OBU_PADDING)
> +                ff_cbs_delete_unit(ctx->cbc, frag, i);
>          }
>      }
>  
> diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
> index 5260a39c63..e1e6055ceb 100644
> --- a/libavcodec/cbs.h
> +++ b/libavcodec/cbs.h
> @@ -380,6 +380,8 @@ int ff_cbs_insert_unit_data(CodedBitstreamContext *ctx,
>  
>  /**
>   * Delete a unit from a fragment and free all memory it uses.
> + *
> + * Never returns failure if position is >= 0 and < frag->nb_units.
>   */
>  int ff_cbs_delete_unit(CodedBitstreamContext *ctx,
>                         CodedBitstreamFragment *frag,
> diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
> index f7ca1f0f09..d05b75be14 100644
> --- a/libavcodec/h264_metadata_bsf.c
> +++ b/libavcodec/h264_metadata_bsf.c
> @@ -428,12 +428,7 @@ static int h264_metadata_filter(AVBSFContext *bsf, AVPacket *pkt)
>          for (i = au->nb_units - 1; i >= 0; i--) {
>              if (au->units[i].type == H264_NAL_FILLER_DATA) {
>                  // Filler NAL units.
> -                err = ff_cbs_delete_unit(ctx->cbc, au, i);
> -                if (err < 0) {
> -                    av_log(bsf, AV_LOG_ERROR, "Failed to delete "
> -                           "filler NAL.\n");
> -                    goto fail;
> -                }
> +                ff_cbs_delete_unit(ctx->cbc, au, i);
>                  continue;
>              }
>  
> diff --git a/libavcodec/h264_redundant_pps_bsf.c b/libavcodec/h264_redundant_pps_bsf.c
> index db8717d69a..8526b5b4c4 100644
> --- a/libavcodec/h264_redundant_pps_bsf.c
> +++ b/libavcodec/h264_redundant_pps_bsf.c
> @@ -95,9 +95,7 @@ static int h264_redundant_pps_filter(AVBSFContext *bsf, AVPacket *out)
>              if (!au_has_sps) {
>                  av_log(bsf, AV_LOG_VERBOSE, "Deleting redundant PPS "
>                         "at %"PRId64".\n", in->pts);
> -                err = ff_cbs_delete_unit(ctx->input, au, i);
> -                if (err < 0)
> -                    goto fail;
> +                ff_cbs_delete_unit(ctx->input, au, i);
>              }
>          }
>          if (nal->type == H264_NAL_SLICE ||
>