[FFmpeg-devel] [PATCH] lavc/videotoolbox: fix avcc creation for h264 streams missing extradata

[Aman Gupta]

On Sunday, October 23, 2016, Richard Kern <kernrj at gmail.com> wrote:

>
> > On Oct 19, 2016, at 8:45 PM, Aman Gupta <ffmpeg at tmm1.net <javascript:;>>
> wrote:
> >
> > From: Aman Gupta <aman at tmm1.net <javascript:;>>
> >
> > ff_videotoolbox_avcc_extradata_create() was never being called if
> > avctx->extradata_size==0, even though the function does not need or use
> > the avctx->extradata.
>
> Could this be a bug in another part of the code? It seems like extradata
> should be set.


Yes it definitely could be. I can verify if extradata is present on macOS
where the same sample and ffmpeg version worked.


>
> >
> > This manifested itself only on h264 streams in specific containers, and
> > only on iOS. I guess the macOS version of VideoToolbox is more
> > forgiving, atleast on my specific combination of OS version and hardware.
>
> Which container has this issue?


I saw it with an mpegts file, but only on iOS.


>
> >
> > I also added an error log message when VTDecompressionSessionCreate()
> > fails, to help the next poor soul who runs into a bug in this area of the
> > code. The native OSStatus error codes are much easier to google than
> > their AVERROR counterparts (especially in this case, with
> AVERROR_UNKNOWN).
> > ---
> > libavcodec/videotoolbox.c | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
> > index 1288aa5..b21eccb 100644
> > --- a/libavcodec/videotoolbox.c
> > +++ b/libavcodec/videotoolbox.c
> > @@ -413,7 +413,7 @@ static CFDictionaryRef videotoolbox_decoder_config_create(CMVideoCodecType
> codec
> >                          kVTVideoDecoderSpecification_
> RequireHardwareAcceleratedVideoDecoder,
> >                          kCFBooleanTrue);
> >
> > -    if (avctx->extradata_size) {
> > +    if (avctx->extradata_size || codec_type == kCMVideoCodecType_H264) {
>
> This is somewhat confusing. The extradata_size check is only needed for
> videotoolbox_esds_extradata_create(). It should check the sps and pps
> sizes are valid before creating the avcc atom.


I can remove this outer if statement altogether, and add a check either
around or inside the esds_create if that makes more sense.


>
> >         CFMutableDictionaryRef avc_info;
> >         CFDataRef data = NULL;
> >
> > @@ -572,13 +572,16 @@ static int videotoolbox_default_init(AVCodecContext
> *avctx)
> >     if (buf_attr)
> >         CFRelease(buf_attr);
> >
> > +    if (status != 0)
> > +        av_log(avctx, AV_LOG_ERROR, "Error creating videotoolbox
> decompression session: %d\n", status);
> > +
> >     switch (status) {
> >     case kVTVideoDecoderNotAvailableNowErr:
> >     case kVTVideoDecoderUnsupportedDataFormatErr:
> >         return AVERROR(ENOSYS);
> >     case kVTVideoDecoderMalfunctionErr:
> >         return AVERROR(EINVAL);
> > -    case kVTVideoDecoderBadDataErr :
> > +    case kVTVideoDecoderBadDataErr:
> >         return AVERROR_INVALIDDATA;
> >     case 0:
> >         return 0;
> > --
> > 2.8.1
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org <javascript:;>
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>