[vlc-devel] [PATCH 3/3] nVidia affinity

[Rémi Denis-Courmont]

   Hello,

I think all three patches should be merged.

More comments in-line...

On Tue, 27 Aug 2013 09:38:33 -0400, David R Robison
<drrobison at openroadsconsulting.com> wrote:
> @@ -48,11 +50,18 @@
>   static int  Open (vlc_object_t *);
>   static void Close(vlc_object_t *);
> 
> +#define HW_NVIDIA_AFFINITY_TEXT N_("Set the nVidia affinity to this 
> adapter number")

Most probably too long for the GUI.

> +#define HW_NVIDIA_AFFINITY_LONGTEXT N_(\
> +    "Identifies nVidia addapter number to use for hardware
acceleration.")
> +
>   vlc_module_begin()
>       set_category(CAT_VIDEO)
>       set_subcategory(SUBCAT_VIDEO_VOUT)
>       set_shortname("OpenGL")
>       set_description(N_("OpenGL video output"))
> +
> +    add_integer("nvidia-affinity", -1, HW_NVIDIA_AFFINITY_TEXT, 
> HW_NVIDIA_AFFINITY_LONGTEXT, true)
> +
>       set_capability("vout display", 160)
>       add_shortcut("glwin32", "opengl")
>       set_callbacks(Open, Close)
> @@ -70,6 +79,70 @@ static void           Manage (vout_display_t *);
>   static void           Swap   (vlc_gl_t *);
>   static void          *OurGetProcAddress(vlc_gl_t *, const char *);
> 
> +#undef wglEnumGpusNV
> +#undef wglCreateAffinityDCNV
> +#undef wglDeleteDCNV
> +static PFNWGLENUMGPUSNVPROC wglEnumGpusNV;
> +static PFNWGLCREATEAFFINITYDCNVPROC wglCreateAffinityDCNV;
> +static PFNWGLDELETEDCNVPROC wglDeleteDCNV;
> +static HDC AffinityHDC;

Static RW data => re-entrancy hell => bad idea.

> +
> +static void AssignNVidiaAffinity(vout_display_t *vd, UINT
nVidiaAffinity)
> {
> +    PIXELFORMATDESCRIPTOR pfd;
> +    memset(&pfd, 0, sizeof(pfd));
> +    pfd.nSize = sizeof(pfd);
> +    pfd.nVersion = 1;
> +    pfd.dwFlags = PFD_DRAW_TO_WINDOW | PFD_SUPPORT_OPENGL | 
> PFD_DOUBLEBUFFER;
> +    pfd.iPixelType = PFD_TYPE_RGBA;
> +    pfd.cColorBits = 24;
> +    pfd.cDepthBits = 16;
> +    pfd.iLayerType = PFD_MAIN_PLANE;

Named initializer, please. No need for memset().

> +
> +    // initialize the wgl context
> +    if (wglEnumGpusNV == NULL) {
> +        msg_Dbg(vd, "Initialize wgl context" );
> +
> +    // create a temporary GL context
> +        HDC winDC = GetDC(vd->sys->hvideownd);
> +        SetPixelFormat(winDC, ChoosePixelFormat(winDC, &pfd), &pfd);
> +        HGLRC hGLRC = wglCreateContext(winDC);
> +        wglMakeCurrent(winDC, hGLRC);
> +
> +    // get function pointers
> +        wglEnumGpusNV = 
> (PFNWGLENUMGPUSNVPROC)wglGetProcAddress("wglEnumGpusNV");
> +        wglCreateAffinityDCNV = 
>
(PFNWGLCREATEAFFINITYDCNVPROC)wglGetProcAddress("wglCreateAffinityDCNV");
> +        wglDeleteDCNV = 
> (PFNWGLDELETEDCNVPROC)wglGetProcAddress("wglDeleteDCNV");

And indeed, this seems not reentrant.

> +
> +    // delete the temporary GL context
> +        wglDeleteContext(hGLRC);
> +    }
> +
> +    // see if we have the extensions
> +    if (!wglEnumGpusNV || !wglCreateAffinityDCNV) {
> +        msg_Dbg(vd, "OpenGL nVidia extensions do not seem to be
present"
> );
> +        return;
> +    }
> +
> +    // find the graphics card
> +    HGPUNV GpuMask[2];
> +    GpuMask[0] = NULL;
> +    GpuMask[1] = NULL;
> +    HGPUNV hGPU;
> +    if (!wglEnumGpusNV(nVidiaAffinity, &hGPU)) {
> +        msg_Warn(vd, "Could not enumerate nVidia adapter %d", 
> nVidiaAffinity );
> +     return;
> +    }
> +
> +    // make the new context
> +    GpuMask[0] = hGPU;
> +    AffinityHDC = wglCreateAffinityDCNV(GpuMask);
> +    if (AffinityHDC == NULL) {
> +        msg_Warn(vd, "Could not create nVidia afinity DV %d", 
> GetLastError() );
> +        return;
> +    }
> +    SetPixelFormat(AffinityHDC, ChoosePixelFormat(AffinityHDC, &pfd), 
> &pfd);
> +}
> +
>   /**
>    * It creates an OpenGL vout display.
>    */
> @@ -89,7 +162,13 @@ static int Open(vlc_object_t *object)
> 
>       EventThreadUpdateTitle(sys->event, VOUT_TITLE " (OpenGL output)");
> 
> -    /* */
> +    // process card specific affinities
> +    int nVidiaAffinity = var_InheritInteger(vd, "nvidia-affinity");
> +    if (nVidiaAffinity >= 0) {
> +        msg_Dbg(vd, "nVidia affinity using adapter %d", nVidiaAffinity
);
> +        AssignNVidiaAffinity(vd, nVidiaAffinity);
> +    }
> +
>       sys->hGLDC = GetDC(sys->hvideownd);
> 
>       /* Set the pixel format for the DC */
> @@ -98,6 +177,7 @@ static int Open(vlc_object_t *object)
>       pfd.nSize = sizeof(pfd);
>       pfd.nVersion = 1;
>       pfd.dwFlags = PFD_DRAW_TO_WINDOW | PFD_SUPPORT_OPENGL | 
> PFD_DOUBLEBUFFER;
> +    //pfd.dwFlags = PFD_DRAW_TO_WINDOW | PFD_SUPPORT_OPENGL;

Left over??

>       pfd.iPixelType = PFD_TYPE_RGBA;
>       pfd.cColorBits = 24;
>       pfd.cDepthBits = 16;
> @@ -106,10 +186,11 @@ static int Open(vlc_object_t *object)
>                      ChoosePixelFormat(sys->hGLDC, &pfd), &pfd);
> 
>       /* Create and enable the render context */
> -    sys->hGLRC = wglCreateContext(sys->hGLDC);
> -    wglMakeCurrent(sys->hGLDC, sys->hGLRC);
> +    sys->hGLRC = wglCreateContext((AffinityHDC != NULL) ? AffinityHDC :

> sys->hGLDC);
> +    if (!wglMakeCurrent(sys->hGLDC, sys->hGLRC)) msg_Warn(vd, 
> "wglMakeCurrent failed = %d", GetLastError() );

Please wrap and indent long lines.

> 
>       const char *extensions = (const char*)glGetString(GL_EXTENSIONS);
> +    msg_Dbg(vd, "GL Extensions = %s", extensions );
>   #ifdef WGL_EXT_swap_control
>       if (HasExtension(extensions, "WGL_EXT_swap_control")) {
>           PFNWGLSWAPINTERVALEXTPROC SwapIntervalEXT = 
> (PFNWGLSWAPINTERVALEXTPROC)wglGetProcAddress("wglSwapIntervalEXT");
> @@ -169,9 +250,10 @@ static void Close(vlc_object_t *object)
>           wglMakeCurrent(NULL, NULL);
>       if (sys->hGLRC)
>           wglDeleteContext(sys->hGLRC);
> +    if (AffinityHDC)
> +    wglDeleteDCNV(AffinityHDC);

Indent.

>       if (sys->hGLDC)
>           ReleaseDC(sys->hvideownd, sys->hGLDC);
> -
>       CommonClean(vd);
> 
>       free(sys);
> @@ -191,6 +273,25 @@ static void Prepare(vout_display_t *vd, picture_t 
> *picture, subpicture_t *subpic
>   {
>       vout_display_sys_t *sys = vd->sys;
> 
> +    // Create a Frame Buffer Object & bind it
> +/*
> +    if (glGenFramebuffersEXT == NULL || glBindFramebufferEXT == NULL) {
> +        msg_Dbg(vd, "OpenGL Frame Buffer extensions do not seem to be 
> present" );
> +    } else {
> +        const int width  = sys->rect_dest.right  - sys->rect_dest.left;
> +        const int height = sys->rect_dest.bottom - sys->rect_dest.top;
> +msg_Dbg("Image %d %d", width, height);
> +        glGenFramebuffersEXT(1, &MyFBO);
> +        glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, MyFBO);
> +        glGenRenderbuffersEXT(1, &MyRBO);
> +        glBindRenderbufferEXT(GL_RENDERBUFFER_EXT, MyRBO);
> +        glRenderbufferStorageEXT(GL_RENDERBUFFER_EXT, GL_RGBA8, width, 
> height);
> +        glFramebufferRenderbufferEXT(GL_FRAMEBUFFER_EXT, 
> GL_COLOR_ATTACHMENT0_EXT, GL_RENDERBUFFER_EXT, MyRBO);
> +        int rc = glCheckFramebufferStatusEXT(GL_FRAMEBUFFER_EXT);
> +        if (rc != GL_FRAMEBUFFER_COMPLETE_EXT) msg_Warn(vd, "Could not 
> create FBO: %d", rc);
> +    }
> +*/

More left over?

> +
>       vout_display_opengl_Prepare(sys->vgl, picture, subpicture);
>   }
> 
> @@ -237,6 +338,11 @@ static void Swap(vlc_gl_t *gl)
>   {
>       vout_display_t *vd = gl->sys;
> 
> +/*
> +    glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, 0);
> +    glDeleteFramebuffersEXT(1, &MyFBO);
> +    glDeleteRenderbuffersEXT(1, &MyRBO);
> +*/

Ditto?

>       SwapBuffers(vd->sys->hGLDC);
>   }

-- 
Rémi Denis-Courmont
Sent from my collocated server