On Thu, 7 Sep 2017 21:04:16 +0100 Mark Thompson <s...@jkqxz.net> wrote:
> On 06/09/17 21:28, wm4 wrote: > > On Wed, 6 Sep 2017 21:07:53 +0100 > > Mark Thompson <s...@jkqxz.net> wrote: > > > >> On 06/09/17 17:27, wm4 wrote: > >>> On Wed, 6 Sep 2017 16:49:11 +0100 > >>> Mark Thompson <s...@jkqxz.net> wrote: > >>> > >>>> On 06/09/17 15:33, wm4 wrote: > >>>>> On Tue, 5 Sep 2017 23:59:30 +0100 > >>>>> Mark Thompson <s...@jkqxz.net> wrote: > >>>>> > >>>>>> --- > >>>>>> Saves the VA config, the frame pool, and the VA context (in that > >>>>>> order). The device reference is held so that freeing the persistent > >>>>>> data is possible (the transient context is already gone when it > >>>>>> happens). > >>>>>> > >>>>>> > >>>>>> libavcodec/vaapi_decode.c | 241 > >>>>>> +++++++++++++++++++++++++++++++++------------- > >>>>>> libavcodec/vaapi_decode.h | 18 ++++ > >>>>>> 2 files changed, 192 insertions(+), 67 deletions(-) > >>>>>> > >>>> For normal cases: > >>>> * Flush with the same extradata (e.g. seeking) reuses everything. > >>>> * Resolution change reuses the VA config and makes new frames and VA > >>>> context. > >>>> > >>>>> Also, it would be possible to implement frames context reuse without > >>>>> having to change each hwaccel implementation. > >>>> > >>>> Maybe? It would need some amount of change around management of > >>>> hw_device_ctx / user-set hw_frames_ctx / legacy hwaccel_context cases, > >>>> since all of the choices between those are currently made by the > >>>> individual hwaccels. > >>> > >>> Not really. The invariant is that you _can_ provide an existing frames > >>> context to a hwaccel, and that frames context could be simply cached by > >>> the shared code. One requirement is that the shared code could get the > >>> needed frame parameter in advance. > >>> > >>> That would still require new per-hwaccel code, but only to the extent > >>> of what dxva_adjust_hwframes() (as well as current API users) need to > >>> know. In particular, this is independent from creating an actual > >>> decoder. > >>> > >>> So why can't we just export something like dxva_adjust_hwframes()? > >>> > >>> The only problem is that this would need a new entry point, as the user > >>> somehow select the AVHWaccel implicitly with the get_format() return. > >> > >> I'm not really sure what you are suggesting here. Can you explain a bit > >> further what you mean? > > > > Exporting a function like dxva_adjust_hwframes() for every hwaccel, > > dispatched in a similar way hwaccel use is. (So I guess something like > > avcodec_adjust_hwframes(ctx, pix_fmt).) > > Forgive my stupidity here, but I'm still not understanding what you mean at > all. > > When does the user call this function? What does it do? > > I think of dxva_adjust_hwframes() as modifying the parameters of the created > frames context so that it works at all, not as something optional which can > meaningfully not be called. Currently (as in git master), dxva_adjust_hwframes() is called only when the user doesn't set a avctx->hw_frames_ctx. dxva2.c uses the function to create the hw_frames_ctx if only avctx->hw_device_ctx is set. If the API user sets avctx->hw_frames_ctx, dxva_adjust_hwframes() is never called. dxva_adjust_hwframes() works on an uninitialized hw_frames_ctx. It sets the frames format, surface size, and pool size, according to the current codec. So e.g. if it's HEVC, it knows to add 16 to the pool size, and to align the frame size to 128. Currently, an API user, who wants to set avctx->hw_frames_ctx on its own has to duplicate all logic in dxva_adjust_hwframes(). This means it has to know the surface alignment, how to map the sw_pix_fmts (e.g. AV_PIX_FMT_YUV420P10 => AV_PIX_FMT_P010), and how many frames the codec needs at most. The API user might want to set a custom hw_frames_ctx to do things like setting special frames_ctx parameters before init, or to cache hw_frames_ctx pools. I don't think many people actually want to duplicate some of the logic dxva_adjust_hwframes() does. Usually you always set a NV12 or P010 surface format. But the other use cases, like caching the hw_frames_ctx, or enabling surface flags for using the surfaces in a shader, are legitimate and probably a common thing to do. It's strange that the hwaccel is doing almost everything automagically, but if you even just want to set special hw_frames_ctx init parameters, you have to duplicate a tiny amount of the logic the hwaccel normally does. I'd like if the user could instead reuse this logic. This could be achieved by exposing a function like dxva_adjust_hwframes(). Your patches also do that do a degree (in an "inverted" way), and the proposed patches would work fine for me, but I think it's less flexible than what I'm suggesting. _______________________________________________ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
