[FFmpeg-devel] [PATCH 8/9] avcodec/videotoolbox: use decode_params to propagate PPS changes and restart on SPS changes

[wm4]

On Mon, 25 Sep 2017 17:36:30 -0700
Aman Gupta <ffmpeg at tmm1.net> wrote:

> From: Aman Gupta <aman at tmm1.net>
> 
> If the VideoToolbox session needs to be restarted, and
> videotoolbox_start() fails for some reason (for instance, if the video
> is interlaced and the decoder is running on iOS), avcodec will return
> AVERROR_EXTERNAL. This can be used by the API user to switch to another
> decoder.
> ---
>  libavcodec/vda_vt_internal.h |  6 ++++++
>  libavcodec/videotoolbox.c    | 45 +++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/vda_vt_internal.h b/libavcodec/vda_vt_internal.h
> index e55a813899..49c91791ee 100644
> --- a/libavcodec/vda_vt_internal.h
> +++ b/libavcodec/vda_vt_internal.h
> @@ -47,6 +47,12 @@ typedef struct VTContext {
>      // Non-NULL if the new hwaccel API is used. This is only a separate struct
>      // to ease compatibility with the old API.
>      struct AVVideotoolboxContext *vt_ctx;
> +
> +    // Current H264 parameters (used to trigger decoder restart on SPS changes).
> +    uint8_t                     *sps;
> +    uint32_t                    sps_len;
> +    unsigned int                sps_capa;
> +    bool                        reconfig_needed;
>  } VTContext;
>  
>  int ff_videotoolbox_alloc_frame(AVCodecContext *avctx, AVFrame *frame);
> diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
> index f56ab1f8c9..6c8477c2ce 100644
> --- a/libavcodec/videotoolbox.c
> +++ b/libavcodec/videotoolbox.c
> @@ -41,6 +41,9 @@
>  
>  #define VIDEOTOOLBOX_ESDS_EXTRADATA_PADDING  12
>  
> +static void videotoolbox_stop(AVCodecContext *avctx);
> +static int videotoolbox_start(AVCodecContext *avctx);
> +
>  static void videotoolbox_buffer_release(void *opaque, uint8_t *data)
>  {
>      CVPixelBufferRef cv_buffer = (CVImageBufferRef)data;
> @@ -148,6 +151,33 @@ int ff_videotoolbox_h264_start_frame(AVCodecContext *avctx,
>      return 0;
>  }
>  
> +static int videotoolbox_h264_decode_params(AVCodecContext *avctx,
> +                                           const uint8_t *buffer,
> +                                           uint32_t size)
> +{
> +    VTContext *vtctx = avctx->internal->hwaccel_priv_data;
> +    H264Context *h  = avctx->priv_data;
> +
> +    if (h->is_avc == 1)
> +        return 0;

Should that matter?

> +    switch (buffer[0] & 0x1f) {
> +        case H264_NAL_SPS:
> +            if (!vtctx->sps || vtctx->sps_len != size || memcmp(buffer, vtctx->sps, size) != 0) {
> +                vtctx->sps = av_fast_realloc(vtctx->sps, &vtctx->sps_capa, size);
> +                if (vtctx->sps)
> +                    memcpy(vtctx->sps, buffer, size);
> +                if (vtctx->sps_len)
> +                    vtctx->reconfig_needed = true;
> +                vtctx->sps_len = size;
> +            }
> +            break;
> +    }

To be honest, I think just rebuilding the avcc and testing for a change
would be simpler. Unless there's a good reason not to reinit when only
the PPS changes.

Might also make sense to pass the NAL type as parameter to the
AVHWAccel function?

Otherwise I'm fine with it.

> +    // pass-through new PPS to the decoder
> +    return ff_videotoolbox_h264_decode_slice(avctx, buffer, size);
> +}
> +
>  int ff_videotoolbox_h264_decode_slice(AVCodecContext *avctx,
>                                        const uint8_t *buffer,
>                                        uint32_t size)
> @@ -180,6 +210,7 @@ int ff_videotoolbox_uninit(AVCodecContext *avctx)
>      VTContext *vtctx = avctx->internal->hwaccel_priv_data;
>      if (vtctx) {
>          av_freep(&vtctx->bitstream);
> +        av_freep(&vtctx->sps);
>          if (vtctx->frame)
>              CVPixelBufferRelease(vtctx->frame);
>      }
> @@ -419,7 +450,16 @@ static int videotoolbox_common_end_frame(AVCodecContext *avctx, AVFrame *frame)
>      AVVideotoolboxContext *videotoolbox = videotoolbox_get_context(avctx);
>      VTContext *vtctx = avctx->internal->hwaccel_priv_data;
>  
> -    if (!videotoolbox->session || !vtctx->bitstream)
> +    if (vtctx->reconfig_needed == true) {
> +        vtctx->reconfig_needed = false;
> +        av_log(avctx, AV_LOG_VERBOSE, "VideoToolbox decoder needs reconfig, restarting..\n");
> +        videotoolbox_stop(avctx);
> +        if (videotoolbox_start(avctx) != 0) {
> +            return AVERROR_EXTERNAL;
> +        }
> +    }
> +
> +    if (!videotoolbox->session || !vtctx->bitstream || !vtctx->bitstream_size)
>          return AVERROR_INVALIDDATA;
>  
>      status = videotoolbox_session_decode_frame(avctx);
> @@ -432,9 +472,11 @@ static int videotoolbox_common_end_frame(AVCodecContext *avctx, AVFrame *frame)
>                  break;
>              case kVTVideoDecoderMalfunctionErr:
>                  error = "decoder malfunction";
> +                vtctx->reconfig_needed = true;
>                  break;
>              case kVTInvalidSessionErr:
>                  error = "invalid session";
> +                vtctx->reconfig_needed = true;
>                  break;
>              default:
>                  error = "unknown";
> @@ -824,6 +866,7 @@ AVHWAccel ff_h264_videotoolbox_hwaccel = {
>      .alloc_frame    = ff_videotoolbox_alloc_frame,
>      .start_frame    = ff_videotoolbox_h264_start_frame,
>      .decode_slice   = ff_videotoolbox_h264_decode_slice,
> +    .decode_params  = videotoolbox_h264_decode_params,
>      .end_frame      = videotoolbox_h264_end_frame,
>      .init           = videotoolbox_common_init,
>      .uninit         = videotoolbox_uninit,