-
Notifications
You must be signed in to change notification settings - Fork 272
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
Support sharing texture from cef on Windows #1217
Support sharing texture from cef on Windows #1217
Conversation
Thanks. |
Please move the dx11 stuff out to a separate file |
I tested with 4 layer html with video mode 1080p50. And results:
|
Ah sorry. I don't know to add files with cmake :( |
Yeah, I have found that Visual studio does not make this easy to do. The best way I have done this is to manually create the file in |
|
||
auto image_size = rowBytes * height; | ||
|
||
memcpy(frame.image_data(0).begin(), reinterpret_cast<uint8_t*>(map.pData), image_size); |
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.
I'm not keen on this memcpy. It would be better if we could do a native gpu copy into a opengl texture, to save having to reupload it before the mixer can use the frame.
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.
We can't do a native gpu copy into opengl texture, because shared texture can't access by cpu
|
||
// Mapping surface data to memory | ||
D3D11_MAPPED_SUBRESOURCE map; | ||
if (SUCCEEDED(dx11_device_->immedidate_context()->context()->Map( |
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.
It looks like we are doing some potentially quite slow work in this callback. It might be better to throw this onto an executor to do, so that we don't block up the cef event loop.
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 shared texture will be used by next render, so, could not throw it onto an executor to do
namespace caspar { namespace html { | ||
|
||
#ifdef WIN32 | ||
|
||
class dx11_device final |
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.
I think this should be made into a singleton, as it is being constructed a few times just to check if it is available.
set(CEF_BIN_PATH "${NUGET_PACKAGES_FOLDER}/cef.redist.x64.3.3239.1723/CEF") | ||
set(CEF_RESOURCE_PATH "${NUGET_PACKAGES_FOLDER}/cef.redist.x64.3.3239.1723/CEF") | ||
link_directories("${NUGET_PACKAGES_FOLDER}/cef.sdk.3.3239.1723/CEF/x64") | ||
set(CEF_INCLUDE_PATH "${NUGET_PACKAGES_FOLDER}/cef.sdk.3.3578.1870/CEF") |
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.
Any particular reason for this version of CEF? There look to be newer on nuget. I ask because I will need to bump to 74 sometime soon for #669
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.
I tested in cef 74, but cef 74 seem have bug with fps, it render very fast
Can we use WGLEW_NV_DX_interop? |
May be. I will try |
I looks like I was using WGLEW_NV_DX_interop when trying this myself, but its been a few months since I was doing this so I don't really remember what I was doing. In case any of it is useful for you, but be warned some of it is a hacky mess: https://github.com/CasparCG/server/compare/master...Julusian:feature/cef_gpu_texture?expand=1 |
I moved the dx11 stuff out to a separate file(dx11.h and dx11.cpp) |
I haven't actually compiled this. But I'm quite impressed and happy with what I see. @Julusian if you can compile and verify I'm ok with this being merged. |
std::lock_guard<std::mutex> lock(frames_mutex_); | ||
|
||
frames_.push(dframe); | ||
while (frames_.size() > 8) { |
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 buffer size should probably be configurable.
I've applied changes and looks like working well, but html text scroll (Ticker) is not smooth, is it possible its not synchronized with requestanimationframe loop ? |
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.
Overall this looks good.
The only things from me are to update CEF to 73, and to fix it so that caspar can still run on non-nvidia gpus.
@@ -74,6 +74,14 @@ struct texture::impl | |||
|
|||
void clear() { GL(glClearTexImage(id_, 0, FORMAT[stride_], TYPE[stride_], nullptr)); } | |||
|
|||
#ifdef WIN32 | |||
void copy_from(int texute_id) |
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.
Is there a reason this method is windows only?
typo in variable name.
GL(glFlush()); | ||
|
||
deadline_timer timer(service_); | ||
for (auto n = 0; true; ++n) { |
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.
Will the opengl thread be blocked while this copy is happening?
io_context service_; | ||
decltype(make_work_guard(service_)) work_; | ||
std::thread thread_; | ||
|
||
impl() | ||
: device_(sf::ContextSettings(0, 0, 0, 4, 5, sf::ContextSettings::Attribute::Core), 1, 1) | ||
#ifdef WIN32 | ||
, d3d_device_(d3d::d3d_device::get_device()) |
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.
We should be able to handle failure when creating this gracefully.
if (enable_gpu) { | ||
auto dev = accelerator::d3d::d3d_device::get_device(); | ||
if (!dev) | ||
CASPAR_LOG(warning) << L"Failed to create directX device"; |
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 log line could be better
set(CEF_INCLUDE_PATH "${NUGET_PACKAGES_FOLDER}/cef.sdk.3.3578.1870/CEF") | ||
set(CEF_BIN_PATH "${NUGET_PACKAGES_FOLDER}/cef.redist.x64.3.3578.1870/CEF") | ||
set(CEF_RESOURCE_PATH "${NUGET_PACKAGES_FOLDER}/cef.redist.x64.3.3578.1870/CEF") | ||
link_directories("${NUGET_PACKAGES_FOLDER}/cef.sdk.3.3578.1870/CEF/x64") |
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.
We should bump this to cef 73. Newer than that has issues, but I havent found any in 73 yet.
@@ -100,6 +113,18 @@ struct device::impl : public std::enable_shared_from_this<impl> | |||
|
|||
device_.setActive(false); | |||
|
|||
#ifdef WIN32 | |||
if (d3d_device_) { | |||
interop_handle_ = std::shared_ptr<void>(wglDXOpenDeviceNV(d3d_device_->device()), [](void* p) { |
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 can segfault as wglDXOpenDeviceNV is not available on intel.
If this happens then just skip the whole shared texture stuff there, we can look into alternative api in a later PR.
This has been manually merged to master. |
Solve the problem at #1177
Need update cef to 3.3578.1870 (for support sharing texture)
Support only Windows