[FFmpeg-devel] [PATCH] lavc/mediacodecdec_h264: use ff_h264_decode_extradata to extract PPS/SPS

Matthieu Bouron matthieu.bouron at gmail.com
Tue Jun 21 14:22:17 CEST 2016


On Mon, Jun 20, 2016 at 06:25:19PM +0200, Matthieu Bouron wrote:
> On Mon, Jun 20, 2016 at 12:03:58PM +0200, Matthieu Bouron wrote:
> > On Sun, Jun 19, 2016 at 07:44:44PM +0200, Matthieu Bouron wrote:
> > > On Mon, Jun 13, 2016 at 02:37:29PM +0200, Matthieu Bouron wrote:
> > > > On Mon, Jun 13, 2016 at 12:23:07PM +0200, Hendrik Leppkes wrote:
> > > > > On Mon, Jun 13, 2016 at 11:51 AM, Matthieu Bouron
> > > > > <matthieu.bouron at gmail.com> wrote:
> > > > > > On Fri, Jun 10, 2016 at 03:08:48PM +0200, Matthieu Bouron wrote:
> > > > > >> From: Matthieu Bouron <matthieu.bouron at stupeflix.com>
> > > > > >>
> > > > > >> Fixes playback of HLS streams on MediaTek devices which requires PPS/SPS
> > > > > >> to be set in their respective csd-{0,1} buffers.
> > > > > >> ---
> > > > > >>
> > > > > >> Hello,
> > > > > >>
> > > > > >> The attached patch fixes playback of HLS streams on MediaTek devices which
> > > > > >> requires PPS/SPS to be set in their respetive csd-{0,1} buffers (instead of
> > > > > >> having sps+pps in the csd-0 which works on other devices).
> > > > > >>
> > > > > >> I'm not sure if I can use the ff_h264_decode_extradata this way (or at least
> > > > > >> initialize the H264Context with zeroes minus the avctx field).
> > > > > >
> > > > > > Rebased patch (after the h264 ps merged) attached.
> > > > > >
> > > > > > I still have the same question, is my use of
> > > > > > H264Context + ff_h264_decode_extradata correct ?
> > > > > >
> > > > > 
> > > > > Using H264 decoder internals seems to be a rather unfortunate
> > > > > solution, as its prone to breakage, often subtle, as the h264 decoder
> > > > > gets changed and not all inter-module dependencies are known.
> > > > > So if possible at all, not using something that uses H264Context for
> > > > > example would be nice.
> > > > > 
> > > > > For the record, ff_h264_decode_extradata is scheduled for refactoring
> > > > > to make it independent of H264Context so it can be more easily shared
> > > > > with the h264 decoder and the h264 parser.
> > > > > Once that is done, it may give you a cleaner interface to use it from
> > > > > mediacodec as well.
> > > > 
> > > > Ok. I can wait for the refactor to be merged but the MediaCodec decoder
> > > > will remain broken on those devices. I'm not too happy about that if a
> > > > release is to be made in those following days. Do we have an ETA ?  I'm
> > > > also not too happy to write the same parsing code as I did before for the
> > > > AVCC format to split/extract the PPS/SPS.
> > > > 
> > > > Or ..., I can push this code (if its use is valid) and update it when the
> > > > merge lands (I'm helping Clément with the merges, so I will take care
> > > > about this part).
> > > 
> > > Updated patch attached (using the new ff_h264_decode_extradata API).
> > > 
> > 
> > Correct patch attached (the previous one was incorrect).
> > 
> > [...]
> 
> > From a27b3ece788cbbe459226f50fbdaf12ce3fee1bb Mon Sep 17 00:00:00 2001
> > From: Matthieu Bouron <matthieu.bouron at stupeflix.com>
> > Date: Fri, 10 Jun 2016 13:16:09 +0200
> > Subject: [PATCH] lavc/mediacodecdec_h264: use ff_h264_decode_extradata to
> >  extract PPS/SPS
> > 
> > Fixes playback of HLS streams on MediaTek devices which requires PPS/SPS
> > to be set in their respective csd-{0,1} buffers.
> > ---
> >  libavcodec/mediacodecdec_h264.c | 153 +++++++++++-----------------------------
> >  1 file changed, 42 insertions(+), 111 deletions(-)
> > 
> > diff --git a/libavcodec/mediacodecdec_h264.c b/libavcodec/mediacodecdec_h264.c
> > index 0f90606..219649a 100644
> > --- a/libavcodec/mediacodecdec_h264.c
> > +++ b/libavcodec/mediacodecdec_h264.c
> > @@ -32,6 +32,7 @@
> >  #include "libavutil/atomic.h"
> >  
> >  #include "avcodec.h"
> > +#include "h264.h"
> >  #include "internal.h"
> >  #include "mediacodecdec.h"
> >  #include "mediacodec_wrapper.h"
> > @@ -50,104 +51,6 @@ typedef struct MediaCodecH264DecContext {
> >  
> >  } MediaCodecH264DecContext;
> >  
> > -static int h264_extradata_to_annexb_sps_pps(AVCodecContext *avctx,
> > -        uint8_t **extradata_annexb, int *extradata_annexb_size,
> > -        int *sps_offset, int *sps_size,
> > -        int *pps_offset, int *pps_size)
> > -{
> > -    uint16_t unit_size;
> > -    uint64_t total_size = 0;
> > -
> > -    uint8_t i, j, unit_nb;
> > -    uint8_t sps_seen = 0;
> > -    uint8_t pps_seen = 0;
> > -
> > -    const uint8_t *extradata;
> > -    static const uint8_t nalu_header[4] = { 0x00, 0x00, 0x00, 0x01 };
> > -
> > -    if (avctx->extradata_size < 8) {
> > -        av_log(avctx, AV_LOG_ERROR,
> > -            "Too small extradata size, corrupted stream or invalid MP4/AVCC bitstream\n");
> > -        return AVERROR(EINVAL);
> > -    }
> > -
> > -    *extradata_annexb = NULL;
> > -    *extradata_annexb_size = 0;
> > -
> > -    *sps_offset = *sps_size = 0;
> > -    *pps_offset = *pps_size = 0;
> > -
> > -    extradata = avctx->extradata + 4;
> > -
> > -    /* skip length size */
> > -    extradata++;
> > -
> > -    for (j = 0; j < 2; j ++) {
> > -
> > -        if (j == 0) {
> > -            /* number of sps unit(s) */
> > -            unit_nb = *extradata++ & 0x1f;
> > -        } else {
> > -            /* number of pps unit(s) */
> > -            unit_nb = *extradata++;
> > -        }
> > -
> > -        for (i = 0; i < unit_nb; i++) {
> > -            int err;
> > -
> > -            unit_size   = AV_RB16(extradata);
> > -            total_size += unit_size + 4;
> > -
> > -            if (total_size > INT_MAX) {
> > -                av_log(avctx, AV_LOG_ERROR,
> > -                    "Too big extradata size, corrupted stream or invalid MP4/AVCC bitstream\n");
> > -                av_freep(extradata_annexb);
> > -                return AVERROR(EINVAL);
> > -            }
> > -
> > -            if (extradata + 2 + unit_size > avctx->extradata + avctx->extradata_size) {
> > -                av_log(avctx, AV_LOG_ERROR, "Packet header is not contained in global extradata, "
> > -                    "corrupted stream or invalid MP4/AVCC bitstream\n");
> > -                av_freep(extradata_annexb);
> > -                return AVERROR(EINVAL);
> > -            }
> > -
> > -            if ((err = av_reallocp(extradata_annexb, total_size)) < 0) {
> > -                return err;
> > -            }
> > -
> > -            memcpy(*extradata_annexb + total_size - unit_size - 4, nalu_header, 4);
> > -            memcpy(*extradata_annexb + total_size - unit_size, extradata + 2, unit_size);
> > -            extradata += 2 + unit_size;
> > -        }
> > -
> > -        if (unit_nb) {
> > -            if (j == 0) {
> > -                sps_seen = 1;
> > -                *sps_size = total_size;
> > -            } else {
> > -                pps_seen = 1;
> > -                *pps_size = total_size - *sps_size;
> > -                *pps_offset = *sps_size;
> > -            }
> > -        }
> > -    }
> > -
> > -    *extradata_annexb_size = total_size;
> > -
> > -    if (!sps_seen)
> > -        av_log(avctx, AV_LOG_WARNING,
> > -               "Warning: SPS NALU missing or invalid. "
> > -               "The resulting stream may not play.\n");
> > -
> > -    if (!pps_seen)
> > -        av_log(avctx, AV_LOG_WARNING,
> > -               "Warning: PPS NALU missing or invalid. "
> > -               "The resulting stream may not play.\n");
> > -
> > -    return 0;
> > -}
> > -
> >  static av_cold int mediacodec_decode_close(AVCodecContext *avctx)
> >  {
> >      MediaCodecH264DecContext *s = avctx->priv_data;
> > @@ -164,10 +67,20 @@ static av_cold int mediacodec_decode_close(AVCodecContext *avctx)
> >  
> >  static av_cold int mediacodec_decode_init(AVCodecContext *avctx)
> >  {
> > +    int i;
> >      int ret;
> > +
> > +    H264ParamSets ps;
> > +    const PPS *pps = NULL;
> > +    const SPS *sps = NULL;
> > +    int is_avc = 0;
> > +    int nal_length_size = 0;
> > +
> >      FFAMediaFormat *format = NULL;
> >      MediaCodecH264DecContext *s = avctx->priv_data;
> >  
> > +    memset(&ps, 0, sizeof(ps));
> > +
> >      format = ff_AMediaFormat_new();
> >      if (!format) {
> >          av_log(avctx, AV_LOG_ERROR, "Failed to create media format\n");
> > @@ -179,24 +92,32 @@ static av_cold int mediacodec_decode_init(AVCodecContext *avctx)
> >      ff_AMediaFormat_setInt32(format, "width", avctx->width);
> >      ff_AMediaFormat_setInt32(format, "height", avctx->height);
> >  
> > -    if (avctx->extradata[0] == 1) {
> > -        uint8_t *extradata = NULL;
> > -        int extradata_size = 0;
> > -
> > -        int sps_offset, sps_size;
> > -        int pps_offset, pps_size;
> > +    ret = ff_h264_decode_extradata(avctx->extradata, avctx->extradata_size,
> > +            &ps, &is_avc, &nal_length_size, 0, avctx);
> > +    if (ret < 0) {
> > +        goto done;
> > +    }
> >  
> > -        if ((ret = h264_extradata_to_annexb_sps_pps(avctx, &extradata, &extradata_size,
> > -                &sps_offset, &sps_size, &pps_offset, &pps_size)) < 0) {
> > -            goto done;
> > +    for (i = 0; i < MAX_PPS_COUNT; i++) {
> > +        if (ps.pps_list[i]) {
> > +            pps = (const PPS*)ps.pps_list[i]->data;
> > +            break;
> >          }
> > +    }
> >  
> > -        ff_AMediaFormat_setBuffer(format, "csd-0", extradata + sps_offset, sps_size);
> > -        ff_AMediaFormat_setBuffer(format, "csd-1", extradata + pps_offset, pps_size);
> > +    if (pps) {
> > +        if (ps.sps_list[pps->sps_id]) {
> > +            sps = (const SPS*)ps.sps_list[pps->sps_id]->data;
> > +        }
> > +    }
> >  
> > -        av_freep(&extradata);
> > +    if (pps && sps) {
> > +        ff_AMediaFormat_setBuffer(format, "csd-0", (void*)sps->data, sps->data_size);
> > +        ff_AMediaFormat_setBuffer(format, "csd-1", (void*)pps->data, pps->data_size);
> >      } else {
> > -        ff_AMediaFormat_setBuffer(format, "csd-0", avctx->extradata, avctx->extradata_size);
> > +        av_log(avctx, AV_LOG_ERROR, "Could not extract PPS/SPS from extradata");
> > +        ret = AVERROR_EXTERNAL;
> 
> Changed locally to AVERROR_INVALIDDATA;
> 
> > +        goto done;
> >      }
> >  
> >      if ((ret = ff_mediacodec_dec_init(avctx, &s->ctx, CODEC_MIME, format)) < 0) {
> > @@ -236,6 +157,16 @@ done:
> >      if (ret < 0) {
> >          mediacodec_decode_close(avctx);
> >      }
> > +
> > +    for (i = 0; i < MAX_SPS_COUNT; i++)
> > +        av_buffer_unref(&ps.sps_list[i]);
> > +
> > +    for (i = 0; i < MAX_PPS_COUNT; i++)
> > +        av_buffer_unref(&ps.pps_list[i]);
> > +
> > +    av_buffer_unref(&ps.sps_ref);
> > +    av_buffer_unref(&ps.pps_ref);
> 
> Changed locally to ff_h264_ps_uninit().
> 
> If there is no objection, I will push the patch in one day.

Pushed.

[...]


More information about the ffmpeg-devel mailing list