On Sun, Oct 23, 2016 at 11:20 PM, Aman Gupta <a...@tmm1.net> wrote:
>
>
> On Sunday, October 23, 2016, Richard Kern <ker...@gmail.com> wrote:
>
>>
>> > On Oct 19, 2016, at 8:45 PM, Aman Gupta <ffm...@tmm1.net> wrote:
>> >
>> > From: Aman Gupta <a...@tmm1.net>
>> >
>> > 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.
>
Looks like missing extradata has nothing to do with the platform, but
rather only happens when you skip calling avformat_find_stream_info().
I am skipping the find_stream_info() call to reduce time spent probing and
begin playback more quickly, since the initial avformat_open_input() is
enough to discover available streams on mpegts containers.
That means this issue is not as widespread as I initially thought, but I
still think it's an optimization worth making.
>
>
>>
>> >
>> > 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_R
>> equireHardwareAcceleratedVideoDecoder,
>> > 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.
>
Let me know if you'd like me to submit the alternative version of this
patch.
>
>
>>
>> > 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@ffmpeg.org
>> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
