[vlc-devel] [PATCH] d3d11 vout plugin
Martell Malone
martellmalone at gmail.com
Tue Mar 3 13:45:59 CET 2015
>
> >> configure.ac | 7 +-
> >> modules/MODULES_LIST | 1 +
> >> modules/video_output/Makefile.am | 16 +
> >> modules/video_output/msw/common.c | 33 +-
> >> modules/video_output/msw/common.h | 46 +-
> >> modules/video_output/msw/direct3d11.c | 940
> ++++++++++++++++++++++++++++++++++
> >> modules/video_output/msw/events.c | 17 +-
> >> 7 files changed, 1049 insertions(+), 11 deletions(-)
> >> create mode 100644 modules/video_output/msw/direct3d11.c
>
> Missing NEWS and po/POTFILES.in entry
I've no idea what to put in there, suggestions are welcome.
I will look at previous NEWS and POTFILES to get an idea.
>> diff --git a/modules/video_output/msw/common.c
> b/modules/video_output/msw/common.c
> >> index 1ebe08c..802450d 100644
> >> --- a/modules/video_output/msw/common.c
> >> +++ b/modules/video_output/msw/common.c
> >> @@ -4,7 +4,8 @@
> >> * Copyright (C) 2001-2009 VLC authors and VideoLAN
> >> * $Id$
> >> *
> >> - * Authors: Gildas Bazin <gbazin at videolan.org>
> >> + * Authors: Martell Malone <martellmalone at gmail.com>,
> >> + * Gildas Bazin <gbazin at videolan.org>
>
> No, you don't put yourself first.
>
My mistake I copy and pasted it from direct3d11.c
I will move my name to last for these
>> +#ifdef MODULE_NAME_IS_direct3d11
> >> +#if !VLC_WINSTORE_APP
> >> + HINSTANCE hdxgi_dll; /* handle of the opened
> dxgi dll */
> >> + HINSTANCE hd3d11_dll; /* handle of the opened
> d3d11 dll */
> >> + HINSTANCE hd3dcompiler_dll; /* handle of the opened
> d3dcompiler dll */
>
> Those 3 are linked in, for WinRT?
>
Only d3dcompiler is linked in for WinRT.
The other two are not needed.
I have a patch to submit to mingw-w64 to add d3dcompiler_47 the windows 8.1
version.
>> + IDXGIAdapter *dxgiadapter; /* DXGI adapter */
> >> + IDXGIFactory *dxgifactory; /* DXGI factory */
> >> + IDXGISwapChain *dxgiswapChain; /* DXGI 1.0 swap chain
> */
> >> + /* We should find a better way to store this or atleast a shorter
> name */
> >> + PFN_D3D11_CREATE_DEVICE_AND_SWAP_CHAIN
> OurD3D11CreateDeviceAndSwapChain;
> >> + PFN_D3D11_CREATE_DEVICE OurD3D11CreateDevice;
> >> + pD3DCompile OurD3DCompile;
> >> +#else
> >> + IDXGISwapChain1 *dxgiswapChain; /* DXGI 1.1 swap chain
> */
> >> +#endif
> >> + ID3D11Device *d3ddevice; /* D3D device */
> >> + ID3D11DeviceContext *d3dcontext; /* D3D context */
> >> + ID3D11Texture2D *d3dtexture;
>
> Only 1 texture?
>
There is 2
>> + ID3D11ShaderResourceView *d3dresViewY;
>> + ID3D11ShaderResourceView *d3dresViewUV;
the naming in the dx11 api is a bit misleading.
You have a texture which is more of a surface than a texture.
and then this can have any number of resource views which are technically
the textures.
Thank Microsoft here for confusing everyone. :)
>> + DXGI_FORMAT d3dFormat;
> >> + vlc_fourcc_t vlcFormat;
> format of ?
>
>> + vd->sys->vlcFormat = d3d_formats[0].fourcc;
>> + video_format_t fmt;
Many of the if statements that were using fmt were failing in
createresources, update etc if i set fmt directly.
Rather than wasting time on this issue I stored it.
I can have a second look at this if you want?
>> +/* avoided until we can deal with c++/cx in gcc
hahah. no.
I could have explained that with a better comment.
We need a way to pass a pointer to SwapChainPanel from the WinRT app rather
than the cx reference ^ it is doing atm.
I don't know why I even said it that way :)
>> + ID3D11VertexShader *d3dvertexShader;
> >> + ID3D11PixelShader *d3dpixelShader;
> >> + ID3D11InputLayout *d3dvertexLayout;
> >> + ID3D11SamplerState *d3dsampState;
> >> + picture_sys_t *picsys;
>
> picsys seems wrong
>
We need this to pass it to lock. In the dx11 case it is called map.
picture_resource_t resource = { .p_sys = picsys };
This is the same way it is done in the directx9 plugin.
> +#define D3D11_HELP N_("Recommended video output for Windows RT and
> Windows Phone")
>
> Nah, bad description.
>
Have you replacement text?
>> +static const d3d_format_t d3d_formats[] = {
> >> + { "NV12", DXGI_FORMAT_NV12, VLC_CODEC_NV12 },
> >> + { "RGBA", DXGI_FORMAT_R8G8B8A8_UNORM, VLC_CODEC_RGBA },
> >> + { NULL, 0, 0 }
> >> +};
>
> Where is YV12 ?
>
>From under DXGI_FORMAT_420_OPAQUE on
https://msdn.microsoft.com/en-us/library/windows/desktop/bb173059%28v=vs.85%29.aspx
"Applications cannot use the CPU to map the resource and then access the
> data within the resource.
> You cannot use shaders with this format."
>
The format will only become usable when I add ID3D11Video hardware
accelerated decoding.
I will add it at that stage :)
>> +#if VLC_WINSTORE_APP
> >> + DXGI_SWAP_CHAIN_DESC1 scd;
> >> +#else
> >> + DXGI_SWAP_CHAIN_DESC scd;
> >> +#endif
>
> Why not using one define at the top and use that in the code?
>
I'm not quite sure what you mean?
If you give me an example I'll gladly do it.
>> +#if 0
> Why ?
>
I left this comment above the first one in the code explaining them
/* All the #if 0 contain an alternative method to setup dx11
> They both need to be benchmarked to see which performs better */
> #if 0
>
There is 2 ways to setup dx11.
I just don't know which is better and what devices support which, speed of
each etc.
Your indentation is too small (2 instead of 4).
>
Good Spot I dunno how I missed that.
The rest looks good.
:)
On Tue, Mar 3, 2015 at 10:44 AM, Jean-Baptiste Kempf <jb at videolan.org>
wrote:
> On 03 Mar, Martell Malone wrote :
> > Attached is the directx11 vout that I have been working on.
>
> >> configure.ac | 7 +-
> >> modules/MODULES_LIST | 1 +
> >> modules/video_output/Makefile.am | 16 +
> >> modules/video_output/msw/common.c | 33 +-
> >> modules/video_output/msw/common.h | 46 +-
> >> modules/video_output/msw/direct3d11.c | 940
> ++++++++++++++++++++++++++++++++++
> >> modules/video_output/msw/events.c | 17 +-
> >> 7 files changed, 1049 insertions(+), 11 deletions(-)
> >> create mode 100644 modules/video_output/msw/direct3d11.c
>
> Missing NEWS and po/POTFILES.in entry.
>
> >> diff --git a/modules/video_output/msw/common.c
> b/modules/video_output/msw/common.c
> >> index 1ebe08c..802450d 100644
> >> --- a/modules/video_output/msw/common.c
> >> +++ b/modules/video_output/msw/common.c
> >> @@ -4,7 +4,8 @@
> >> * Copyright (C) 2001-2009 VLC authors and VideoLAN
> >> * $Id$
> >> *
> >> - * Authors: Gildas Bazin <gbazin at videolan.org>
> >> + * Authors: Martell Malone <martellmalone at gmail.com>,
> >> + * Gildas Bazin <gbazin at videolan.org>
>
> No, you don't put yourself first.
>
> >> index 5cb8c45..b02fa27 100644
> >> --- a/modules/video_output/msw/common.h
> >> +++ b/modules/video_output/msw/common.h
> >> @@ -4,7 +4,8 @@
> >> * Copyright (C) 2001-2009 VLC authors and VideoLAN
> >> * $Id$
> >> *
> >> - * Authors: Gildas Bazin <gbazin at videolan.org>
> >> + * Authors: Martell Malone <martellmalone at gmail.com>,
> >> + * Gildas Bazin <gbazin at videolan.org>,
>
> idem.
>
> > @@ -25,6 +26,15 @@
> >> +#ifdef MODULE_NAME_IS_direct3d11
> >> +# include <d3d11.h>
> >> +#if VLC_WINSTORE_APP
> >> +# include <dxgi1_2.h>
> >> +#else
> >> +# include <dxgi.h>
> >> +#endif
> >> +# include <d3dcompiler.h>
> >> +#endif
>
> Please indent your ifs,
>
> >> +#ifdef MODULE_NAME_IS_direct3d11
> >> +#if !VLC_WINSTORE_APP
> >> + HINSTANCE hdxgi_dll; /* handle of the opened
> dxgi dll */
> >> + HINSTANCE hd3d11_dll; /* handle of the opened
> d3d11 dll */
> >> + HINSTANCE hd3dcompiler_dll; /* handle of the opened
> d3dcompiler dll */
>
> Those 3 are linked in, for WinRT?
>
> >> + IDXGIAdapter *dxgiadapter; /* DXGI adapter */
> >> + IDXGIFactory *dxgifactory; /* DXGI factory */
> >> + IDXGISwapChain *dxgiswapChain; /* DXGI 1.0 swap chain
> */
> >> + /* We should find a better way to store this or atleast a shorter
> name */
> >> + PFN_D3D11_CREATE_DEVICE_AND_SWAP_CHAIN
> OurD3D11CreateDeviceAndSwapChain;
> >> + PFN_D3D11_CREATE_DEVICE OurD3D11CreateDevice;
> >> + pD3DCompile OurD3DCompile;
> >> +#else
> >> + IDXGISwapChain1 *dxgiswapChain; /* DXGI 1.1 swap chain
> */
> >> +#endif
> >> + ID3D11Device *d3ddevice; /* D3D device */
> >> + ID3D11DeviceContext *d3dcontext; /* D3D context */
> >> + ID3D11Texture2D *d3dtexture;
>
> Only 1 texture?
>
> >> + ID3D11ShaderResourceView *d3dresViewY;
> >> + ID3D11ShaderResourceView *d3dresViewUV;
> >> + ID3D11RenderTargetView *d3drenderTargetView;
> >> + ID3D11DepthStencilView *d3ddepthStencilView;
> >> + ID3D11VertexShader *d3dvertexShader;
> >> + ID3D11PixelShader *d3dpixelShader;
> >> + ID3D11InputLayout *d3dvertexLayout;
> >> + ID3D11SamplerState *d3dsampState;
> >> + picture_sys_t *picsys;
>
> picsys seems wrong
>
> >> + D3D_FEATURE_LEVEL d3dfeaturelevel;
> >> + DXGI_FORMAT d3dFormat;
> >> + vlc_fourcc_t vlcFormat;
>
> format of ?
> >> +#endif
> >> +
>
> >> +/* avoided until we can deal with c++/cx in gcc
> hahah. no.
>
> > +#define D3D11_HELP N_("Recommended video output for Windows RT and
> Windows Phone")
>
> Nah, bad description.
>
> >> +static const d3d_format_t d3d_formats[] = {
> >> + { "NV12", DXGI_FORMAT_NV12, VLC_CODEC_NV12 },
> >> + { "RGBA", DXGI_FORMAT_R8G8B8A8_UNORM, VLC_CODEC_RGBA },
> >> + { NULL, 0, 0 }
> >> +};
>
> Where is YV12 ?
>
> >> +#if VLC_WINSTORE_APP
> >> + DXGI_SWAP_CHAIN_DESC1 scd;
> >> +#else
> >> + DXGI_SWAP_CHAIN_DESC scd;
> >> +#endif
>
> Why not using one define at the top and use that in the code?
>
> >> +#if 0
>
> Why ?
>
> >> +
> >> + /* TODO : list adapters for the user to choose from */
> >> + hr = IDXGIFactory_EnumAdapters(sys->dxgifactory, 0,
> &sys->dxgiadapter);
> >> + if (FAILED(hr)) {
> >> + msg_Err(vd, "Could not create find factory. (hr=0x%lX)", hr);
> >> + return VLC_EGENERIC;
> >> + }
> >> +
> >> + IDXGIOutput* output;
> >> + hr = IDXGIAdapter_EnumOutputs(sys->dxgiadapter, 0, &output);
> >> + if (FAILED(hr)) {
> >> + msg_Err(vd, "Could not Enumerate DXGI Outputs. (hr=0x%lX)", hr);
> >> + return VLC_EGENERIC;
> >> + }
> >> +
> >> + DXGI_MODE_DESC md;
> >> + memset(&md, 0, sizeof(md));
> >> + md.Width = fmt->i_visible_width;
> >> + md.Height = fmt->i_visible_height;
> >> + md.Format = scd.BufferDesc.Format;
> >> + md.Scaling = DXGI_MODE_SCALING_UNSPECIFIED;
> >> +
> >> + hr = IDXGIOutput_FindClosestMatchingMode(output, &md,
> &scd.BufferDesc, NULL);
> >> + if (FAILED(hr)) {
> >> + msg_Err(vd, "Failed to find a supported video mode.
> (hr=0x%lX)", hr);
> >> + return VLC_EGENERIC;
> >> + }
> >> +
> >> + /* mode desc doesn't carry over the width and height*/
> >> + scd.BufferDesc.Width = fmt->i_visible_width;
> >> + scd.BufferDesc.Height = fmt->i_visible_height;
> >> +
> >> + hr = D3D11CreateDeviceAndSwapChain(sys->dxgiadapter,
> >> + D3D_DRIVER_TYPE_UNKNOWN, NULL, creationFlags,
> >> + featureLevels, ARRAYSIZE(featureLevels),
> >> + D3D11_SDK_VERSION, &scd, &sys->dxgiswapChain,
> >> + &sys->d3ddevice, &sys->d3dfeaturelevel,
> &sys->d3dcontext);
> >> + if (FAILED(hr)) {
> >> + msg_Err(vd, "Could not Create the D3D11 device and SwapChain.
> (hr=0x%lX)", hr);
> >> + return VLC_EGENERIC;
> >> + }
> >> +
> >> +#else
> >> +
> >> + static const D3D_DRIVER_TYPE driverAttempts[] = {
> >> + D3D_DRIVER_TYPE_HARDWARE,
> >> + D3D_DRIVER_TYPE_WARP,
> >> + D3D_DRIVER_TYPE_REFERENCE,
> >> + };
> >> +
> >> + for (UINT driver = 0; driver < ARRAYSIZE(driverAttempts);
> driver++) {
> >> + hr = D3D11CreateDevice(NULL, driverAttempts[driver], NULL,
> creationFlags,
> >> + featureLevels, ARRAYSIZE(featureLevels),
> D3D11_SDK_VERSION,
> >> + &sys->d3ddevice, &sys->d3dfeaturelevel,
> &sys->d3dcontext);
> >> + if (SUCCEEDED(hr)) break;
> >> + }
> >> +
> >> + if (FAILED(hr)) {
> >> + msg_Err(vd, "Could not Create the D3D11 device. (hr=0x%lX)",
> hr);
> >> + return VLC_EGENERIC;
> >> + }
> >> +
> >> + IDXGIDevice *pDXGIDevice = NULL;
> >> + hr = ID3D11Device_QueryInterface(sys->d3ddevice, &IID_IDXGIDevice,
> (void **)&pDXGIDevice);
> >> + if (FAILED(hr)) {
> >> + msg_Err(vd, "Could not Query DXGI Interface. (hr=0x%lX)", hr);
> >> + return VLC_EGENERIC;
> >> + }
> >> +
> >> + hr = IDXGIDevice_GetAdapter(pDXGIDevice, &sys->dxgiadapter);
> >> + if (FAILED(hr)) {
> >> + msg_Err(vd, "Could not get the DXGI Adapter. (hr=0x%lX)", hr);
> >> + return VLC_EGENERIC;
> >> + }
> >> +
> >> + hr = IDXGIAdapter_GetParent(sys->dxgiadapter, &IID_IDXGIFactory,
> (void **)&sys->dxgifactory);
> >> + if (FAILED(hr)) {
> >> + msg_Err(vd, "Could not get the DXGI Factory. (hr=0x%lX)", hr);
> >> + return VLC_EGENERIC;
> >> + }
> >> +
> >> +#if VLC_WINSTORE_APP
> >> + hr = IDXGIFactory2_CreateSwapChainForComposition(sys->dxgifactory,
> (IUnknown *)sys->d3ddevice, &scd, NULL, &sys->dxgiswapChain);
> >> +#else
> >> + hr = IDXGIFactory_CreateSwapChain(sys->dxgifactory, (IUnknown
> *)sys->d3ddevice, &scd, &sys->dxgiswapChain);
> >> +#endif
> >> +
> >> + if (FAILED(hr)) {
> >> + msg_Err(vd, "Could not create the SwapChain. (hr=0x%lX)", hr);
> >> + return VLC_EGENERIC;
> >> + }
> >> +
> >> +#if VLC_WINSTORE_APP /* avoided until we can deal with c++/cx in gcc */
> >> + /* TODO: figure out how to get "ISwapChainPanel ^panel" into
> brokenpanel in gcc */
> >> + ISwapChainPanel *brokenpanel;
> >> + ISwapChainPanelNative *panelNative;
> >> + hr = ISwapChainPanelNative_QueryInterface(brokenpanel,
> &IID_ISwapChainPanelNative, (void **)&pDXGIDevice);
> >> + if (FAILED(hr)) {
> >> + msg_Err(vd, "Could not get the Native Panel. (hr=0x%lX)", hr);
> >> + return VLC_EGENERIC;
> >> + }
> >> +
> >> + hr = ISwapChainPanelNative_SetSwapChain(panelNative,
> sys->dxgiswapChain);
> >> + if (FAILED(hr)) {
> >> + msg_Err(vd, "Could not link the SwapChain with the Native
> Panel. (hr=0x%lX)", hr);
> >> + return VLC_EGENERIC;
> >> + }
> >> +
> >> +#endif
> >> +
> >> +#endif
> >> +
> >> +#endif
> >> +
>
> Your indentation is too small (2 instead of 4).
>
> The rest looks good.
>
> --
> Jean-Baptiste Kempf
> http://www.jbkempf.com/ - +33 672 704 734
> Sent from my Electronic Device
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20150303/a35ee65b/attachment.html>
More information about the vlc-devel
mailing list