[FFmpeg-devel] [PATCH v3 1/2] dxva: wait until D3D11 buffer copies are done before submitting them

[Steve Lhomme]

On 2020-08-11 3:52, Soft Works wrote:
> 
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
>> Steve Lhomme
>> Sent: Monday, August 10, 2020 2:15 PM
>> To: ffmpeg-devel at ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH v3 1/2] dxva: wait until D3D11 buffer
>> copies are done before submitting them
>>
> 
>>> @Steven
>>
>> My name is Steve.
> 
> I apologize for that.
> 
>> My change in e3d4784eb31b3ea4a97f2d4c698a75fab9bf3d86 is optional. So
>> much that it even requires to use the creator helper function to make sure
>> the mutex is properly initialized to an empty value and retain backward
>> compatibility. If you don't want to use many threads you can safely ignore
>> this field.
> 
> I thought you had reviewed my code, because you said that it was you
> who had added the locks that I have removed. I just took a look at your commit

I said "This is how I ended up adding the mutex to the old API 
(e3d4784eb31b3ea4a97f2d4c698a75fab9bf3d86)."
I never claimed I added the locks.

> e3d4784eb31b3ea4a97f2d4c698a75fab9bf3d86 and I can say that those
> are not the locks that I have removed.
> So you are actually the wrong address for the lock removal, you've just
> thrown yourself in the line ;-)

No. You misread what I said. I was explaining that my experience on 
multiple devices lead me to add this mutex otherwise it would not work 
properly or even crash.

> But the DX11 subject still needs some clarification...
> 
>>>>>> The documentation for ID3D11DeviceContext is very clear about that
>> [2]:
>>>>>> "Because each ID3D11DeviceContext is single threaded, only one
>>>>>> thread can call a ID3D11DeviceContext at a time. If multiple
>>>>>> threads must access a single ID3D11DeviceContext, they must use
>>>>>> some synchronization mechanism, such as critical sections, to
>>>>>> synchronize
>>>> access to that ID3D11DeviceContext."
>>>>>
>>>>> Yes, but this doesn't apply to accessing staging textures IIRC.
>>>>
>>>> It does. To copy to a staging texture you need to use
>>>> ID3D11DeviceContext::CopySubresourceRegion().
>>>
>>> Correct. And with DX11 and using SetMultithreadProtected it is legal
>>> to call this from multiple threads without synchronization.
>>
>> No. I already explained it and pointed to the Microsoft documentation [1].
>> SetMultithreadProtected relates to ID3D11Device.
> 
> No, that's exactly wrong. You need to re-read. ID3D11Device is thread-safe
> by default. You can read that everywhere throughout the documentation.

I read the documentation. That's not how I understood it. ID3D11Device 
is safe "by default" meaning it can be turned off. I assumed 
SetMultithreadProtected() is the way to do that. It seems 
D3D11_CREATE_DEVICE_SINGLETHREADED is the way to make it not thread safe.

> SetMultithreadProtected initially didn't even have a D3D11 interface.
> The documentation also didn't tell anything about it. There was only
> the DX10 documentation for this.
> 
> But SetMultithreadProtected in fact has the effect of protecting
> the methods from ID3D11DeviceContext (not device).
> Like I explained several times already.
> 
> 
> Luckily I've just seen that there has been a change meanwhile.
> The undocumented behavior has finally been made public by
> adding a DX11 version of SetMultithreadProtected
> (which does the same as what happens when you use the DX10
> version with DX11)
> 
> https://docs.microsoft.com/en-us/windows/win32/api/d3d11_4/nn-d3d11_4-id3d11multithread

OK, this is very interesting. It certainly clarifies that it's 
ID3D11DeviceContext related. The documentation is still sketchy though. 
For example ::Enter() [1] says that it enters a device critical section, 
not a device context critical section. So if you have multiple device 
context everything is blocked. This kinda defeats the use of multiple 
threads.

Also this interface is not available until Windows 10 Creator Update. So 
for example it cannot be used on Windows 7. Maybe the D3D10 version has 
the same effect but there's no guarantee. In fact it wasn't sufficient 
on mobile devices.

I'll try without the mutex (old API) on Windows 7 and see the effect.

Anyway this thread/patch is about waiting for data to be fully copied 
between the CPU and the GPU before submitting the decoding command. Not 
about thread/command synchronization.

> Now: Read the docs!
> 
> PS: I need to say that your way of discrediting my statements
> was not very respectful.

Likewise.

> Kind regards,
> softworkz
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> 

[1] 
https://docs.microsoft.com/en-us/windows/win32/api/d3d11_4/nf-d3d11_4-id3d11multithread-enter