[FFmpeg-devel] [PATCH 2/2] avcodec/videotoolbox: fix decoding of some h264 bitstreams

[Hendrik Leppkes]

On Thu, Oct 1, 2015 at 6:13 PM, wm4 <nfxjfg at googlemail.com> wrote:
> This affects Annex B streams (such as demuxed from .ts and others). It
> also handles the format change in reinit-large_420_8-to-small_420_8.h264
> correctly.
>
> Instead of passing through the extradata, create it on the fly it from
> the currently active SPS and PPS. Since reconstructing the PPS and SPS
> NALs would be very complicated and verbose, we use the NALs as they
> originally appeared in the bitstream.
>
> The code for writing the extradata is somewhat derived from
> libavformat/avc.c, but it's small and different enough that sharing it
> is not really worth it.
> ---
> Even though it requires changes in the general h264 decoder (previous
> patch), this solution is much cleaner and more robust than my patch
> from yesterday.
> ---
>  libavcodec/videotoolbox.c | 48 +++++++++++++++++++++++++++++------------------
>  1 file changed, 30 insertions(+), 18 deletions(-)
>
> diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
> index 9dec5fc..cc1e592 100644
> --- a/libavcodec/videotoolbox.c
> +++ b/libavcodec/videotoolbox.c
> @@ -77,28 +77,40 @@ int ff_videotoolbox_alloc_frame(AVCodecContext *avctx, AVFrame *frame)
>      return 0;
>  }
>
> +#define AV_W8(p, v) *(p) = (v)
> +
>  CFDataRef ff_videotoolbox_avcc_extradata_create(AVCodecContext *avctx)
>  {
> +    H264Context *h     = avctx->priv_data;
>      CFDataRef data = NULL;
> +    uint8_t *p;
> +    int vt_extradata_size = 6 + 3 + h->sps.data_size + 4 + h->pps.data_size;
> +    uint8_t *vt_extradata = av_malloc(vt_extradata_size);
> +    if (!vt_extradata)
> +        return NULL;
>
> -    /* Each VCL NAL in the bitstream sent to the decoder
> -     * is preceded by a 4 bytes length header.
> -     * Change the avcC atom header if needed, to signal headers of 4 bytes. */
> -    if (avctx->extradata_size >= 4 && (avctx->extradata[4] & 0x03) != 0x03) {
> -        uint8_t *rw_extradata = av_memdup(avctx->extradata, avctx->extradata_size);
> -
> -        if (!rw_extradata)
> -            return NULL;
> -
> -        rw_extradata[4] |= 0x03;
> -
> -        data = CFDataCreate(kCFAllocatorDefault, rw_extradata, avctx->extradata_size);
> -
> -        av_freep(&rw_extradata);
> -    } else {
> -        data = CFDataCreate(kCFAllocatorDefault, avctx->extradata, avctx->extradata_size);
> -    }
> -
> +    p = vt_extradata;
> +
> +    AV_W8(p + 0, 1); /* version */
> +    AV_W8(p + 1, h->sps.data[0]); /* profile */
> +    AV_W8(p + 2, h->sps.data[1]); /* profile compat */
> +    AV_W8(p + 3, h->sps.data[2]); /* level */
> +    AV_W8(p + 4, 0xff); /* 6 bits reserved (111111) + 2 bits nal size length - 3 (11) */
> +    AV_W8(p + 5, 0xe1); /* 3 bits reserved (111) + 5 bits number of sps (00001) */
> +    AV_WB16(p + 6, h->sps.data_size + 1);
> +    AV_W8(p + 8, NAL_SPS | (3 << 5)); // NAL unit header
> +    memcpy(p + 9, h->sps.data, h->sps.data_size);
> +    p += 9 + h->sps.data_size;
> +    AV_W8(p + 0, 1); /* number of pps */
> +    AV_WB16(p + 1, h->pps.data_size + 1);
> +    AV_W8(p + 3, NAL_PPS | (3 << 5)); // NAL unit header
> +    memcpy(p + 4, h->pps.data, h->pps.data_size);
> +
> +    p += 4 + h->pps.data_size;
> +    av_assert0(p - vt_extradata == vt_extradata_size);
> +
> +    data = CFDataCreate(kCFAllocatorDefault, vt_extradata, vt_extradata_size);
> +    av_free(vt_extradata);
>      return data;
>  }
>

This will still fail spectacularly with a SPS/PPS change mid-stream. I
don't suppose it somehow accepts the SPS/PPS data in-band as well?

- Hendrik