[FFmpeg-devel] [PATCH 1/3] avcodec/h264, videotoolbox: pass SPS changes into the VT decoder

Aman Gupta ffmpeg at tmm1.net
Thu Mar 2 19:56:53 EET 2017


On Thu, Mar 2, 2017 at 2:04 AM, wm4 <nfxjfg at googlemail.com> wrote:

> On Thu, 2 Mar 2017 10:47:23 +0100
> Hendrik Leppkes <h.leppkes at gmail.com> wrote:
>
> > On Thu, Mar 2, 2017 at 10:35 AM, wm4 <nfxjfg at googlemail.com> wrote:
> > > On Fri, 17 Feb 2017 21:15:56 +0100
> > > Michael Niedermayer <michael at niedermayer.cc> wrote:
> > >
> > >> On Fri, Feb 17, 2017 at 07:02:10AM +0100, wm4 wrote:
> > >> > On Thu, 16 Feb 2017 10:29:36 -0800
> > >> > Aman Gupta <ffmpeg at tmm1.net> wrote:
> > >> >
> > >> > > From: Aman Gupta <aman at tmm1.net>
> > >> > >
> > >> > > This fixes playback of h264 streams with SPS changes. One such
> sample
> > >> > > is available at http://tmm1.s3.amazonaws.com/
> videotoolbox/spschange.ts.
> > >> > > It switches mid-stream from level 4.0 to level 3.2.
> > >> > >
> > >> > > Previously, playing this sample with the VT hwaccel on iOS would
> crash.
> > >> > > After this patch, it plays back as expected.
> > >> > >
> > >> > > On macOS however, feeding in new SPS into an existing
> decompression
> > >> > > session does not always work, so this patch is only a partial fix.
> > >> > > ---
> > >> > >  libavcodec/h264dec.c | 7 +++++++
> > >> > >  1 file changed, 7 insertions(+)
> > >> > >
> > >> > > diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
> > >> > > index 41c0964..e521c52 100644
> > >> > > --- a/libavcodec/h264dec.c
> > >> > > +++ b/libavcodec/h264dec.c
> > >> > > @@ -740,6 +740,13 @@ FF_ENABLE_DEPRECATION_WARNINGS
> > >> > >              break;
> > >> > >          case H264_NAL_SPS: {
> > >> > >              GetBitContext tmp_gb = nal->gb;
> > >> > > +            if (avctx->hwaccel && avctx->hwaccel->pix_fmt ==
> AV_PIX_FMT_VIDEOTOOLBOX) {
> > >> > > +                ret = avctx->hwaccel->decode_slice(avctx,
> > >> > > +                                                   nal->data,
> > >> > > +                                                   nal->size);
> > >> > > +                if (ret < 0)
> > >> > > +                    goto end;
> > >> > > +            }
> > >> > >              if (ff_h264_decode_seq_parameter_set(&tmp_gb,
> avctx, &h->ps, 0) >= 0)
> > >> > >                  break;
> > >> > >              av_log(h->avctx, AV_LOG_DEBUG,
> > >> >
> > >> > A bit ugly but ok IMHO. Maybe it would be better to add a new
> hwaccel
> > >> > callback here, even if it's used by VT only.
> > >> >
> > >> > You should probably wait for approval by michaelni.
> > >>
> > >> i dont really have an oppinion on hwaccel, thats not so much my
> > >> area
> > >> though i find special cases for specific hwaccel a bit ugly, i dont
> > >> object to it, just saying i would be in favor of not having special
> > >> cases if that is possible
> > >
> > > So do you think this is tolerable in the current state or not?
> >
> > I agree with Michael, the number of VT-specific hacks seem to be
> > piling up. Maybe it should be re-designed entirely instead of piling
> > hacks on top of hacks to keep it barely working.
>
> Would that mean reimplementing it as a full-stream decoder?
>
> That's my favorite choice as well, but apparently the VT hwaccel
> wouldn't require much more changes to work reasonably well.
>

FWIW, after these patches the VT hwaccel has been working very reliably for
me across hundreds of test streams.

I agree that re-implementing as a decoder is the correct long term
solution, but the VT api presents several technical hurdles before that can
become a viable alternative.

For this particular patch, I think it would make sense to add a new hwaccel
callback that's only used by VT. If we can agree on a name I can work up a
patch. Maybe hwaccel->upload_nal() ?

Aman


> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


More information about the ffmpeg-devel mailing list