[FFmpeg-devel] [PATCH v4 1/1] avutils/hwcontext: When deriving a hwdevice, search for existing device in both directions

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Mark
> Thompson
> Sent: Thursday, December 30, 2021 12:22 PM
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v4 1/1] avutils/hwcontext: When deriving a
> hwdevice, search for existing device in both directions
> 
> On 30/12/2021 00:29, Soft Works wrote:
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Mark
> >> Thompson
> >> Sent: Thursday, December 30, 2021 12:04 AM
> >> To: ffmpeg-devel at ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH v4 1/1] avutils/hwcontext: When
> deriving a
> >> hwdevice, search for existing device in both directions
> >>
> >> On 25/11/2021 02:41, Soft Works wrote:
> >>> The test /libavutil/tests/hwdevice checks that when deriving a device
> >>> from a source device and then deriving back to the type of the source
> >>> device, the result is matching the original source device, i.e. the
> >>> derivation mechanism doesn't create a new device in this case.
> >>>
> >>> Previously, this test was usually passed, but only due to two different
> >>> kind of flaws:
> >>>
> >>> 1. The test covers only a single level of derivation (and back)
> >>>
> >>> It derives device Y from device X and then Y back to the type of X and
> >>> checks whether the result matches X.
> >>>
> >>> What it doesn't check for, are longer chains of derivation like:
> >>>
> >>> CUDA1 > OpenCL2 > CUDA3 and then back to OpenCL4
> >>>
> >>> In that case, the second derivation returns the first device (CUDA3 ==
> >>> CUDA1), but when deriving OpenCL4, hwcontext.c was creating a new
> >>> OpenCL4 context instead of returning OpenCL2, because there was no link
> >>> from CUDA1 to OpenCL2 (only backwards from OpenCL2 to CUDA1)
> >>
> >> Yes, this is exactly what I expect.
> >>
> >> Because of how these APIs work, device derivation is always one-way - you
> can
> >> make an OpenCL device from a D3D11 one, but not the other direction.  I
> don't
> >> think there is any case which allows both directions
> >
> > hwmap=reverse=1
> 
> Indeed, hwmap reverse exists because mapping is one-way and sometimes a
> filter graph wants to use it in the other direction.


=> mapping is not only one-way


> >> Saying that derivation from A should always return the same B is not
> >> intended, nor do I think it should be.
> >
> > Why not?
> >
> > Looking at the reality of API users:
> >
> > - I'm covering a wide range of different processing pipelines and
> >    found that this behavior is crucial to make important and relevant
> >    processing pipelines work
> >
> > - Intel have three different workaround-patches in their backlog/queue
> >    of ffmpeg patches to get certain processing setups working
> >
> > - The developers working on Vulkan have confirmed that this change
> >    is necessary and crucial for certain setups to work
> >
> > - Nobody has named any case or scenario that would be negatively
> >    affected by this change
> >
> > Given that situation, I don't think it's useful to talk about
> > theoretical implications.
> 
> You are not talking about API users at all.  When does an API user ever want
> this patch?  From their point of view it is surprising and unwanted - if they
> want the same device again, they just use the same device again.

No. Not when using libavfilter with a filter graph.

=> It affects API users in the same way

> 
> You are talking about users of the ffmpeg utility.  

No. See above.

> The change is a library
> hack to work around the inability to select devices per-filter in the ffmpeg
> utility.

No, it's not a hack, it's about making libavfilter hw context deriving and reverse
mapping working as expected and needed.

> Please, just implement device selection for filters in ffmpeg rather than
> adding unexpected behaviour elsewhere.  libavfilter has supported it for API
> users for a long time, no library changes should be needed.

I'm afraid but I disagree. I stick to the proposed solution; this change
is welcomed and needed by many which are having problems with the current 
implementation.
I still haven't seen any (real-world and not theoretical constructed) scenario
where this would have a negative effect or break something, neither did I see
anybody else objecting.

Anyway, I have no intentions to propose a different solution. This is what
I'm using and what others want and need. 

Thanks,
softworkz