From: Aman Gupta <aman@tmm1.net> VideoToolbox's support for interlaced H264 is quite poor. On macOS, VTSessionCopySupportedPropertyDictionary() will show that kVTDecompressionPropertyKey_FieldMode is supported. Possible values for this option include DeinterlaceFields and BothFields. However, files that use MBAFF interlacing are not deinterlaced on macOS even if the FieldMode=DeinterlacedFields option is specified. Although it doesn't always deinterlace, the macOS version of VideoToolbox will always decode and return frame data even when the H264 source is interlaced. On iOS, FieldMode is not a valid option and interlaced H264 is not supported at all. You can create a valid decompression session, but no frames are returned and almost every DecodeFrame() call returns a malfunction error. I opened rdar://30669495 about this, and Apple's response was:

Correct and intentional. Please stop using interlaced video.

So this commit forces the VideoToolbox hwaccel to fail early when interlaced H264 is encountered. Thus the API user can easily detect the failure and use a different decoder. Here are some sample files I tested on iOS 10 and 11: https://s3.amazonaws.com/tmm1/videotoolbox/interlaced.ts https://s3.amazonaws.com/tmm1/videotoolbox/interlaced2.ts https://s3.amazonaws.com/tmm1/videotoolbox/interlaced3.ts Decoding interlaced2.ts with VideoToolbox never produces any frames. Decoding interlaced3.ts ocassional produces frames, but most slices cause errors. Decoding interlaced.ts actually works as expected, even though its SPS matches the other two samples which fail. This means my test is not comprehensive, and it is not possible to fully detect if a file is compatible with VideoToolbox using its SPS alone. Still, though this method produces false positives, it does not produce false negatives. --- libavcodec/videotoolbox.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c index 7dfcf14c00..8ed56392cc 100644 --- a/libavcodec/videotoolbox.c +++ b/libavcodec/videotoolbox.c @@ -496,11 +496,18 @@ static CFDictionaryRef videotoolbox_decoder_config_create(CMVideoCodecType codec if (data) CFDictionarySetValue(avc_info, CFSTR("esds"), data); break; - case kCMVideoCodecType_H264 : + case kCMVideoCodecType_H264 : { + H264Context *h = avctx->priv_data; + if (TARGET_OS_IPHONE && h->ps.sps->frame_mbs_only_flag == 0) { + av_log(avctx, AV_LOG_ERROR, "VideoToolbox cannot decode interlaced fields on iOS\n"); + CFRelease(avc_info); + goto fail; + } data = ff_videotoolbox_avcc_extradata_create(avctx); if (data) CFDictionarySetValue(avc_info, CFSTR("avcC"), data); break; + } default: break; } @@ -515,6 +522,10 @@ static CFDictionaryRef videotoolbox_decoder_config_create(CMVideoCodecType codec CFRelease(avc_info); } return config_info; + +fail: + CFRelease(config_info); + return NULL; } static CFDictionaryRef videotoolbox_buffer_attributes_create(int width, -- 2.13.5 (Apple Git-94)

From: Aman Gupta <aman@tmm1.net> --- libavcodec/videotoolbox.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c index 1de556f3e8..f56ab1f8c9 100644 --- a/libavcodec/videotoolbox.c +++ b/libavcodec/videotoolbox.c @@ -425,7 +425,22 @@ static int videotoolbox_common_end_frame(AVCodecContext *avctx, AVFrame *frame) status = videotoolbox_session_decode_frame(avctx); if (status) { - av_log(avctx, AV_LOG_ERROR, "Failed to decode frame (%d)\n", status); + const char *error = NULL; + switch (status) { + case kVTVideoDecoderBadDataErr: + error = "bad data"; + break; + case kVTVideoDecoderMalfunctionErr: + error = "decoder malfunction"; + break; + case kVTInvalidSessionErr: + error = "invalid session"; + break; + default: + error = "unknown"; + break; + } + av_log(avctx, AV_LOG_ERROR, "Failed to decode frame (%s, %d)\n", error, status); return AVERROR_UNKNOWN; } -- 2.13.5 (Apple Git-94)

From: Aman Gupta <aman@tmm1.net> The only reason videotoolbox wouldn't produce frames is if the data fed to it was invalid, so returning AVERROR_INVALIDDATA makes sense here. Further, it means AVERROR_EXTERNAL can be used in further commits to signal fatal VideoToolbox errors, letting the user know that they need to fallback to another decoder. --- libavcodec/h264dec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c index 49ebeca6d8..a8263f2e19 100644 --- a/libavcodec/h264dec.c +++ b/libavcodec/h264dec.c @@ -849,7 +849,7 @@ static int output_frame(H264Context *h, AVFrame *dst, H264Picture *srcp) int ret; if (src->format == AV_PIX_FMT_VIDEOTOOLBOX && src->buf[0]->size == 1) - return AVERROR_EXTERNAL; + return AVERROR_INVALIDDATA; ret = av_frame_ref(dst, src); if (ret < 0) -- 2.13.5 (Apple Git-94)

From: Aman Gupta <aman@tmm1.net> This callback will be used by the VideoToolbox H264 hwaccel so that it can receive SPS and PPS NALUs. VideoToolbox requires PPS changes to be fed into the decoder session, and for the session to be recreated when the SPS changes. --- libavcodec/avcodec.h | 12 ++++++++++++ libavcodec/h264dec.c | 14 ++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h index 5c84974e03..14bb509f66 100644 --- a/libavcodec/avcodec.h +++ b/libavcodec/avcodec.h @@ -3921,6 +3921,18 @@ typedef struct AVHWAccel { int (*start_frame)(AVCodecContext *avctx, const uint8_t *buf, uint32_t buf_size); /** + * Callback for parameter data. + * + * Used for SPS/PPS in H264 streams. + * + * @param avctx the codec context + * @param buf the slice data buffer base + * @param buf_size the size of the slice in bytes + * @return zero if successful, a negative value otherwise + */ + int (*decode_params)(AVCodecContext *avctx, const uint8_t *buf, uint32_t buf_size); + + /** * Callback for each slice. * * Meaningful slice information (codec specific) is guaranteed to diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c index a8263f2e19..ab95beb020 100644 --- a/libavcodec/h264dec.c +++ b/libavcodec/h264dec.c @@ -738,6 +738,13 @@ FF_ENABLE_DEPRECATION_WARNINGS break; case H264_NAL_SPS: { GetBitContext tmp_gb = nal->gb; + if (avctx->hwaccel && avctx->hwaccel->decode_params) { + ret = avctx->hwaccel->decode_params(avctx, + nal->data, + nal->size); + if (ret < 0) + goto end; + } if (ff_h264_decode_seq_parameter_set(&tmp_gb, avctx, &h->ps, 0) >= 0) break; av_log(h->avctx, AV_LOG_DEBUG, @@ -749,6 +756,13 @@ FF_ENABLE_DEPRECATION_WARNINGS break; } case H264_NAL_PPS: + if (avctx->hwaccel && avctx->hwaccel->decode_params) { + ret = avctx->hwaccel->decode_params(avctx, + nal->data, + nal->size); + if (ret < 0) + goto end; + } ret = ff_h264_decode_picture_parameter_set(&nal->gb, avctx, &h->ps, nal->size_bits); if (ret < 0 && (h->avctx->err_recognition & AV_EF_EXPLODE)) -- 2.13.5 (Apple Git-94)

From: Aman Gupta <aman@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; + + 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; + } + + // 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, -- 2.13.5 (Apple Git-94)

From: Aman Gupta <aman@tmm1.net> Removes the avctx->extradata_size requirement when creating avcC, since avctx->extradata is only used in the esds code path. This fixes an issue where the VideoToolbox decoder would not work unless avformat_find_stream_info() was called. --- libavcodec/videotoolbox.c | 65 +++++++++++++++++++++++------------------------ 1 file changed, 32 insertions(+), 33 deletions(-) diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c index 6c8477c2ce..de51b9a7c4 100644 --- a/libavcodec/videotoolbox.c +++ b/libavcodec/videotoolbox.c @@ -538,46 +538,45 @@ static CFDictionaryRef videotoolbox_decoder_config_create(CMVideoCodecType codec kVTVideoDecoderSpecification_RequireHardwareAcceleratedVideoDecoder, kCFBooleanTrue); - if (avctx->extradata_size) { - CFMutableDictionaryRef avc_info; - CFDataRef data = NULL; + CFMutableDictionaryRef avc_info; + CFDataRef data = NULL; - avc_info = CFDictionaryCreateMutable(kCFAllocatorDefault, - 1, - &kCFTypeDictionaryKeyCallBacks, - &kCFTypeDictionaryValueCallBacks); + avc_info = CFDictionaryCreateMutable(kCFAllocatorDefault, + 1, + &kCFTypeDictionaryKeyCallBacks, + &kCFTypeDictionaryValueCallBacks); - switch (codec_type) { - case kCMVideoCodecType_MPEG4Video : + switch (codec_type) { + case kCMVideoCodecType_MPEG4Video: + if (avctx->extradata_size) data = videotoolbox_esds_extradata_create(avctx); - if (data) - CFDictionarySetValue(avc_info, CFSTR("esds"), data); - break; - case kCMVideoCodecType_H264 : { - H264Context *h = avctx->priv_data; - if (TARGET_OS_IPHONE && h->ps.sps->frame_mbs_only_flag == 0) { - av_log(avctx, AV_LOG_ERROR, "VideoToolbox cannot decode interlaced fields on iOS\n"); - CFRelease(avc_info); - goto fail; - } - data = ff_videotoolbox_avcc_extradata_create(avctx); - if (data) - CFDictionarySetValue(avc_info, CFSTR("avcC"), data); - break; - } - default: - break; + if (data) + CFDictionarySetValue(avc_info, CFSTR("esds"), data); + break; + case kCMVideoCodecType_H264: { + H264Context *h = avctx->priv_data; + if (TARGET_OS_IPHONE && h->ps.sps->frame_mbs_only_flag == 0) { + av_log(avctx, AV_LOG_ERROR, "VideoToolbox cannot decode interlaced fields on iOS\n"); + CFRelease(avc_info); + goto fail; } + data = ff_videotoolbox_avcc_extradata_create(avctx); + if (data) + CFDictionarySetValue(avc_info, CFSTR("avcC"), data); + break; + } + default: + break; + } - CFDictionarySetValue(config_info, - kCMFormatDescriptionExtension_SampleDescriptionExtensionAtoms, - avc_info); + CFDictionarySetValue(config_info, + kCMFormatDescriptionExtension_SampleDescriptionExtensionAtoms, + avc_info); - if (data) - CFRelease(data); + if (data) + CFRelease(data); - CFRelease(avc_info); - } + CFRelease(avc_info); return config_info; fail: -- 2.13.5 (Apple Git-94)