[FFmpeg-devel] [PATCH] Try to make new VDPAU usable by adding context to callback.

[wm4]

On Thu, 8 Aug 2013 19:26:58 +0200
Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:

> On Thu, Aug 08, 2013 at 11:28:53AM +0200, wm4 wrote:
> > On Thu, 8 Aug 2013 03:53:33 +0200
> > Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:
> > > I have no idea what you propose.
> > > How does any of this allow me to free the old resources, create the new decoder and call the decode function again within the decode function?
> > 
> > The new API doesn't allow you to free old resources (AVFrame references
> > inside the decoder embed vdpau surface handles), but maybe it doesn't
> > matter too much: vdpau preemption implicitly marks all allocated vdpau
> > objects as deallocated/invalid.
> 
> Sure it does, you just have to store a generation counter in the
> opaque pointer of the AVFrame. When the generation counter is behind
> you know you need to update the surface.
> Of course it's even simpler if you ignore what it was "supposed" to be
> and just pass an array index instead of the actual surface number to
> FFmpeg. You then just need to update all the surfaces in your array once
> to handle preemption.

I suppose that is possible, feels a bit too much like API-abuse,
though. If you do something differently than the API demands, you
might get in trouble with future API changes or extensions too.

Storing an additional generation counter might be a good idea.

To be honest, I think the new vdpau API is completely messed up,
because it doesn't allow handling this stuff in a nice way. Maybe we
should change the AV_PIX_FMT_VDPAU description so that an AVFrame is
completely opaque, instead of mandating that a VdpVideoSurface is
stored in data[3]? I don't really like casting integer handles to
pointers either. (This wouldn't even be an incompatible API change.)

> > You have a point about calling the decode function, but that helps only
> > if the preemption is for a short moment. Preemption recovery can take a
> > longer period of time, for example when switching away from X. At least
> > in my tests vdpau will refuse to reinitialize until you switch back to
> > X, so what you want to do would block the player completely. Moreover,
> > calling the real decode function again will not make much sense, since
> > the surface contents are lost anyway.
> 
> Huh? I don't know what happens to the reference frames, but even if they
> should be destroyed that is not an issue for e.g. intra-only content.

If it's intra-only, then the frame after that will resume decoding
properly anyway. Unless the source video is very low FPS. I'm just
questioning whether that is really worth an API change. (But I'll
probably post your patch to Libav once you get it working - it's much
easier to handle preemption properly with it.)

> > > How would I even be able to handle the return value of the render function in general, no matter the cause of error? (if you don't care about losing more frames than necessary I guess you coukd handle preemption outside the decode loop, but that seems at least suboptimal).
> > > lavc ignores the return value, and any wrapper would have nowhere to store it.
> > 
> > Maybe lavc should check the return value, and return it from the
> > libavcodec decoding function?
> 
> Remapping all possible VDPAU errors to FFmpeg ones? That's going to be
> fun...

Well, OK, but maybe at least render2 should be able to return a ffmpeg
error code?