[FFmpeg-devel] [PATCH v2 1/5] avutil/hwcontext_d3d11va: fix the uninitialized texture bindflag

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Hendrik Leppkes
> Sent: Friday, May 6, 2022 7:38 AM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v2 1/5] avutil/hwcontext_d3d11va:
> fix the uninitialized texture bindflag
> 
> On Fri, May 6, 2022 at 3:11 AM Soft Works <softworkz at hotmail.com>
> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Wu,
> > > Tong1
> > > Sent: Thursday, May 5, 2022 11:50 AM
> > > To: FFmpeg development discussions and patches <ffmpeg-
> > > devel at ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH v2 1/5]
> avutil/hwcontext_d3d11va:
> > > fix the uninitialized texture bindflag
> > >
> > > > > -----Original Message-----
> > > > > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf
> Of
> > > > > Hendrik Leppkes
> > > > > Sent: Sunday, May 1, 2022 5:54 PM
> > > > > To: FFmpeg development discussions and patches <ffmpeg-
> > > > > devel at ffmpeg.org>
> > > > > Subject: Re: [FFmpeg-devel] [PATCH v2 1/5]
> > > avutil/hwcontext_d3d11va:
> > > > > fix the uninitialized texture bindflag
> > > > >
> > > > > On Sun, May 1, 2022 at 5:48 PM Hendrik Leppkes
> > > <h.leppkes at gmail.com>
> > > > > wrote:
> > > > > >
> > > > > > On Sun, May 1, 2022 at 7:09 AM Soft Works
> > > <softworkz at hotmail.com>
> > > > > wrote:
> > > > > > > I think that's what Hendrik wanted to point out as far as
> I
> > > > > understood.
> > > > > > >
> > > > > >
> > > > > > Basically, I want explicit behavior, not implicit defaults.
> > > Anytime
> > > > > a
> > > > > > hwcontext is created, something should tell it what its
> going to
> > > be
> > > > > > used for, so we can determine which flags make sense (or
> > > ideally, it
> > > > > > should just specify the flags).
> > > > > >
> > > > > > This patch creates an implicit default for one use-case, is
> this
> > > > > going
> > > > > > to work with all others? No, of course not, because it has
> to
> > > know
> > > > > > what flags to set. Thats why explicitly setting those flags
> is
> > > > > > important, instead of just fixing one implicit case.
> > > >
> > > > I said I agree with you - basically, just that we need to
> > > differentiate between
> > > > the use case:
> > > >
> > > > 1. Use via API
> > > >    => No defaults should be applied, caller is responsible for
> > > specifying
> > > >       the flags
> > > >
> > > > 2. Use via ffmpeg cli
> > > >    => Applying the render target flag would be safe here.
> > > >       We could require this to set via parameter, but there
> won't
> > > ever
> > > >       be a case to specify something different than the render
> > > target flag
> > > >
> > > > Why? Let's look at the possible flags:
> > > >
> > > > D3D11_BIND_DECODER
> > > > In all decoding cases, this flag is set explicitly, so it
> overrides
> > > any default we
> > > > would set
> > > >
> > > > D3D11_BIND_VIDEO_ENCODER
> > > > Set explicitly when required, so it overrides any default we
> would
> > > set
> > > >
> > > > D3D11_BIND_RENDER_TARGET
> > > > All other cases require this flag (e.g. video processing)
> > > >
> > > > No Flag
> > > > Dead end, texture would be unusable for any kind of video
> processing
> > > >
> > > >
> > > > > On that note, the example commandline it fixes just does
> hwupload
> > > and
> > > > > nothing else - does that even require render target to be
> flagged?
> > > > > From what I can tell it uses a simple
> > > > > ID3D11DeviceContext::CopySubresourceRegion to copy from the
> > > staging
> > > > > texture, which should not require any particular bind flags.
> Bind
> > > > > Flags in turn would then depend on what you are actually
> trying to
> > > do
> > > > > with the texture (shader input, etc), in this example...
> nothing?
> > > >
> > > > We are still in the context of ffmpeg cli - you know that there
> are
> > > no shaders
> > > > or 3d operations and no etc.
> > > >
> > > > But at this point, you can derive to another context or you can
> > > hwmap.
> > > > For all of those things, you need D3D11_BIND_RENDER_TARGET.
> > > >
> > > >
> > > > Summary
> > > >
> > > > As mentioned I see two possibilities:
> > > >
> > > > 1. Use D3D11_BIND_RENDER_TARGET as a default when used in
> context of
> > > >    ffmpeg cli, otherwise no default flags
> > > >
> > > > 2. Require a device initialization parameter in the command line
> > > >    (but it would always be about setting the render target flag
> > > >    and there's no case where you would NOT want to set it)
> > > >
> > >
> > > Thanks for the possible solutions for this issue. The No.1 seems
> more
> > > reasonable because
> > > No.2 just seems more unnecessary. But I will still need to find a
> > > better place to set the flag.
> >
> > I would favor #1 as well.
> >
> > Regarding "better place to set the flag":
> >
> > The BindFlag needs to be set when initializing a FRAMES context.
> > But at this point, there's no way to determine whether the code is
> running
> > in an ffmpeg cli process or used by an API consumer.
> >
> > But there would be an opportunity to convey this information on
> > device init. The device (D3D11VA) could then set an internal flag
> > from device init and use this on frames init to determine which
> default
> > to use for the BindFlags value.
> >
> > Remains the question how ffmpeg cli can convey this information to
> > the device (for use during device init).
> >
> > What I thought would be to have ffmpeg.c (or rather ffmpeg_hw.c)
> > to ALWAYS (for all hwaccels) add an entry to the options dictionary,
> > something like "cli" or similar.
> > This would avoid introducing a "special-case" mechanism just for
> > this case (=> by making it a common behavior).
> >
> > There are other cases where this might be useful in order to
> > discriminate API usage from cli usage.
> >
> > But let's see what the others think first..
> >
> 
> Think of the CLI applications as an API user, and design for that,
> because thats really what they are, and thats how you actually end up
> with a good API that covers more cases.
> If special CLI logic is needed, it should be in the CLI applications,
> and not in the libraries.
> 
> - Hendrik

Thanks. When I translate that, it would be a BindFlags entry in the options
dictionary, which any API user _could_ set and which ffmpeg cli _does_
set in ffmpeg_hw.c (to render target). The D3D11VA hw context stores this
on device init and uses it as a default in frames init. 
The default of the default is 0 of course.

Does that sound ok to you?

Kind regards,
softworkz