-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
hwdec: detach d3d and d3d9 hwaccel from angle #5440
hwdec: detach d3d and d3d9 hwaccel from angle #5440
Conversation
@myfreeer i just test it and it's working(BIG THANKS).Not sure how well though but at least it's working so i can't complain.What i see is when switching to full screen the VSync Jitter goes up to 0.122 and shortly after goes back to normal(0.006).Will leave log file in case you need to do some more polishing.Thanks again !!! |
This has nothing to do with this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing this. Just a couple comments / questions.
video/out/gpu/hwdec.c
Outdated
&ra_hwdec_d3d11egl, | ||
&ra_hwdec_d3d11eglrgb, | ||
#if HAVE_D3D9_HWACCEL | ||
#endif | ||
#if HAVE_D3D9_HWACCEL && HAVE_EGL_ANGLE_WIN32 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should probably be nested for consistency with HAVE_D3D11, HAVE_D3D9_HWACCEL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HAVE_D3D11
is used for build option d3d11
, meaning 'Direct3D 11 video output', seeming not necessary for ra_hwdec_dxva2egl
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, sorry, I meant that it looked like you could have nested HAVE_D3D9_HWACCEL
under HAVE_EGL_ANGLE
which I didn't distinguish from HAVE_EGL_ANGLE_WIN32
.
wscript_build.py
Outdated
( "video/out/opengl/hwdec_dxva2gldx.c", "gl-dxinterop-d3d9" ), | ||
( "video/out/opengl/hwdec_dxva2egl.c", "d3d9-hwaccel" ), | ||
( "video/out/opengl/hwdec_dxva2egl.c", "d3d9-hwaccel && egl-angle-win32" ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why does this have different dependencies? egl-angle
works here for me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kevmitch egl-angle-win32
is originally required by d3d9-hwaccel
in wscript
before this commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wm4 added this in dd408e6. It no longer appears to be necessary. I tested
--disable-angle
--disable-d3d9-hwaccel
--disable-angle
--disable-d3d9-hwaccel
For the sake of simplifying, let's drop the added egl-angle-win32
dependency. This is front end CLI context creation. Looking at the code, I don't see any clear reason d3d9-hwaccel
interop alone should require that.
It would also simplify the logic in video/out/gpu/hwdec.c
wscript
Outdated
@@ -831,13 +831,13 @@ hwaccel_features = [ | |||
}, { | |||
# (conflated with ANGLE for easier deps) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment should probably be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The commits should be squashed to one.
video/out/gpu/hwdec.c
Outdated
&ra_hwdec_d3d11egl, | ||
&ra_hwdec_d3d11eglrgb, | ||
#if HAVE_D3D9_HWACCEL | ||
#endif | ||
#if HAVE_D3D9_HWACCEL && HAVE_EGL_ANGLE_WIN32 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, sorry, I meant that it looked like you could have nested HAVE_D3D9_HWACCEL
under HAVE_EGL_ANGLE
which I didn't distinguish from HAVE_EGL_ANGLE_WIN32
.
wscript_build.py
Outdated
( "video/out/opengl/hwdec_dxva2gldx.c", "gl-dxinterop-d3d9" ), | ||
( "video/out/opengl/hwdec_dxva2egl.c", "d3d9-hwaccel" ), | ||
( "video/out/opengl/hwdec_dxva2egl.c", "d3d9-hwaccel && egl-angle-win32" ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wm4 added this in dd408e6. It no longer appears to be necessary. I tested
--disable-angle
--disable-d3d9-hwaccel
--disable-angle
--disable-d3d9-hwaccel
For the sake of simplifying, let's drop the added egl-angle-win32
dependency. This is front end CLI context creation. Looking at the code, I don't see any clear reason d3d9-hwaccel
interop alone should require that.
It would also simplify the logic in video/out/gpu/hwdec.c
2d7a200
to
8626cdb
Compare
Fix #5420
Test build: http://0x0.st/sqVY.7z
I agree that my changes can be relicensed to LGPL 2.1 or later.