[FFmpeg-devel] [PATCH] avcodec/mediacodecdec: Do not abort when H264/HEVC extradata extraction fails

[Jan Ekström]

On Fri, Feb 12, 2021 at 12:36 PM Matthieu Bouron
<matthieu.bouron at gmail.com> wrote:
>
> On Mon, Jan 25, 2021 at 06:38:36PM +0100, sfan5 wrote:
> > Although rare, extradata can be present but empty and extraction will fail.
> > However Android also supports passing codec-specific data inline and
> > will likely play such a stream anyway. So there's no reason to abort
> > initialization before we know for sure.
> > ---
> >  libavcodec/mediacodecdec.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c
> > index ac1725e466..67adefb530 100644
> > --- a/libavcodec/mediacodecdec.c
> > +++ b/libavcodec/mediacodecdec.c
> > @@ -167,8 +167,8 @@ static int h264_set_extradata(AVCodecContext *avctx,
> > FFAMediaFormat *format)
> >          ff_AMediaFormat_setBuffer(format, "csd-1", (void*)data, data_size);
> >          av_freep(&data);
> >      } else {
> > -        av_log(avctx, AV_LOG_ERROR, "Could not extract PPS/SPS from
> > extradata");
> > -        ret = AVERROR_INVALIDDATA;
> > +        av_log(avctx, AV_LOG_WARNING, "Could not extract PPS/SPS from
> > extradata\n");
> > +        ret = 0;
> >      }
> >   done:
> > @@ -254,8 +254,8 @@ static int hevc_set_extradata(AVCodecContext *avctx,
> > FFAMediaFormat *format)
> >           av_freep(&data);
> >      } else {
> > -        av_log(avctx, AV_LOG_ERROR, "Could not extract VPS/PPS/SPS from
> > extradata");
> > -        ret = AVERROR_INVALIDDATA;
> > +        av_log(avctx, AV_LOG_WARNING, "Could not extract VPS/PPS/SPS from
> > extradata\n");
> > +        ret = 0;
> >      }
> >   done:
>
> Hello,
>
> First of all, sorry for the late reply. The original intent with this
> restriction was to make sure that MediaCodec could error out early (at the
> configuration stage) so an implementation on top of libavcodec can decide
> to fall back to a software decoder (without trying to decode anything).
>
> I am fine with removing this restriction (if everyone is OK with that) for
> the following reasons:
> - there are chances that the h264/hevc software decoders have not been
>   enabled (to avoid licensing issues) so the fallback mechanism is not
>   available
> - as you said extradata are not always available (mpegts) and we want to
>   give MediaCodec a chance to decode the stream

I just had a discussion with sfan5 on IRC yesterday when I was looking
at the related code, and yea - I think this is acceptable since to get
to this point ff_h264_decode_extradata / ff_hevc_decode_extradata
would have to return success. They just didn't get anything. This can
happen with f.ex. completely valid MP4 files, since certain
identifiers (which are visible through the codec_tag identifier) do
not required all of the required parameter sets to be within the
extradata (and thus having 0 pps and 0 sps is technically valid with
those).

My idea was thus to make the log message either WARNING or DEBUG
depending on (is_avc || is_nalff) && codec_tag in (list, of, tags) (in
pseudocode). How matroska should be handled depends on how the codec
stuff is currently defined in there.

>
> So LGTM.
>
> If everyone is OK with that, I'll also submit a patch that removes the
> requirements on having the avctx->extradata set (I use such patch at work
> to allow playback of certain hls/mpegts streams).
>
> [...]
> --
> Matthieu B.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".