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

wm4 nfxjfg at googlemail.com
Thu Oct 1 18:39:09 CEST 2015


On Thu, 1 Oct 2015 18:29:00 +0200
Hendrik Leppkes <h.leppkes at gmail.com> wrote:

> 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?

Well, it worked for the resolution change sample. It _should_ be using
SPS/PPS NALs that occur mid-stream and in-band. The h264 decoder will
reinitialize itself when they change (at least in most cases), and then
it will also reinitialize this hwaccel, which means the code above is
actually run in this situation. The h->sps and h->pps fields will be
set to whatever is actually used by the decoder at this point. So I'm
hoping that this change is actually pretty correct.


More information about the ffmpeg-devel mailing list