fix(electron): address embedded mpv review feedback

This commit is contained in:
4gray committed 2026-06-08 17:19:06 +02:00
1 parent 12758e3775
commit b5b495f0f5
7 files changed
+180 -26

No files matched your search

@@ -51,6 +51,9 @@ function log(message) {
function cleanOutput() {
fs.rmSync(outputFile, { force: true });
fs.rmSync(outputLibDir, { recursive: true, force: true });
for (const windowsDllName of ['mpv-2.dll', 'mpv.dll']) {
fs.rmSync(path.join(outputDir, windowsDllName), { force: true });
}
fs.rmSync(path.join(outputDir, '.deps'), { recursive: true, force: true });
fs.rmSync(path.join(outputDir, 'obj.target'), {
recursive: true,
@@ -233,6 +236,17 @@ function copyGenericRuntimeToNativeBuild(runtime) {
copiedFiles.add(fileName);
}
if (targetPlatform === 'win32') {
for (const fileName of copiedFiles) {
if (fileName.endsWith('.dll')) {
copyFile(
path.join(outputLibDir, fileName),
path.join(outputDir, fileName)
);
}
}
}
if (targetPlatform === 'linux') {
const libMpvPath = findLinuxLibMpv(outputLibDir);
if (libMpvPath && path.basename(libMpvPath) !== 'libmpv.so') {
@@ -94,6 +94,7 @@ public:
XMoveResizeWindow(display_, window_, x, y, width, height);
XRaiseWindow(display_, window_);
XFlush(display_);
drainEvents();
}
std::string wid() const
@@ -117,6 +118,18 @@ public:
}
private:
void drainEvents()
{
if (!display_) {
return;
}
while (XPending(display_) > 0) {
XEvent event{};
XNextEvent(display_, &event);
}
}
void clearInputShape()
{
if (!display_ || !window_) {
@@ -76,6 +76,7 @@ struct Session {
std::string pendingRecordingTargetPath;
std::string pendingRecordingStartedAt;
std::string pendingRecordingStopStartedAt;
uint64_t pendingPlaybackLoadRequestId = 0;
};
std::atomic<uint64_t> gNextSessionId{1};
@@ -468,6 +469,26 @@ bool reconcileRecordingPropertyReply(
return false;
}
bool reconcilePlaybackLoadReply(
const std::shared_ptr<Session>& session,
uint64_t requestId,
int error)
{
if (
requestId == 0 ||
requestId != session->pendingPlaybackLoadRequestId
) {
return false;
}
session->pendingPlaybackLoadRequestId = 0;
if (error < 0) {
session->snapshot.status = SessionStatus::Error;
session->snapshot.error = mpv_error_string(error);
}
return true;
}
void runEventLoop(std::shared_ptr<Session> session)
{
while (session->running.load()) {
@@ -563,6 +584,13 @@ void runEventLoop(std::shared_ptr<Session> session)
}
case MPV_EVENT_COMMAND_REPLY:
case MPV_EVENT_SET_PROPERTY_REPLY:
if (reconcilePlaybackLoadReply(
session,
event->reply_userdata,
event->error
)) {
break;
}
if (reconcileRecordingPropertyReply(
session,
event->reply_userdata,
@@ -571,7 +599,6 @@ void runEventLoop(std::shared_ptr<Session> session)
break;
}
if (event->error < 0) {
session->snapshot.status = SessionStatus::Error;
session->snapshot.error = mpv_error_string(event->error);
}
break;
@@ -844,20 +871,7 @@ Napi::Value LoadPlayback(const Napi::CallbackInfo& info)
command.format = MPV_FORMAT_NODE_ARRAY;
command.u.list = &commandList;
const int commandResult = mpv_command_node_async(
session->handle,
nextAsyncRequestId(),
&command
);
if (commandResult < 0) {
updateSessionError(session, mpv_error_string(commandResult));
throw Napi::Error::New(
env,
std::string("Failed to load playback: ") +
mpv_error_string(commandResult)
);
}
const uint64_t loadPlaybackRequestId = nextAsyncRequestId();
{
std::lock_guard<std::mutex> lock(session->mutex);
session->snapshot.streamUrl = streamUrl;
@@ -867,6 +881,25 @@ Napi::Value LoadPlayback(const Napi::CallbackInfo& info)
session->snapshot.recordingTargetPath.clear();
session->snapshot.recordingStartedAt.clear();
session->snapshot.recordingError.clear();
session->pendingPlaybackLoadRequestId = loadPlaybackRequestId;
}
const int commandResult = mpv_command_node_async(
session->handle,
loadPlaybackRequestId,
&command
);
if (commandResult < 0) {
{
std::lock_guard<std::mutex> lock(session->mutex);
session->pendingPlaybackLoadRequestId = 0;
}
updateSessionError(session, mpv_error_string(commandResult));
throw Napi::Error::New(
env,
std::string("Failed to load playback: ") +
mpv_error_string(commandResult)
);
}
return env.Undefined();
@@ -18,10 +18,21 @@ public:
}
static std::string lastError()
{
if (!lastErrorMessage_.empty()) {
return lastErrorMessage_;
}
return formatLastError(
"Failed to create embedded MPV child window."
);
}
static std::string formatLastError(const char* fallbackMessage)
{
DWORD error = GetLastError();
if (error == 0) {
return "Failed to create embedded MPV child window.";
return fallbackMessage;
}
LPSTR messageBuffer = nullptr;
@@ -37,7 +48,7 @@ public:
nullptr
);
std::string message = "Failed to create embedded MPV child window.";
std::string message = fallbackMessage;
if (length > 0 && messageBuffer) {
message += " ";
message += messageBuffer;
@@ -50,12 +61,17 @@ public:
bool create(uintptr_t parentHandle, const Bounds& bounds)
{
lastErrorMessage_.clear();
parentWindow_ = reinterpret_cast<HWND>(parentHandle);
if (!parentWindow_ || !IsWindow(parentWindow_)) {
lastErrorMessage_ =
"Unable to resolve Electron native window handle.";
return false;
}
registerWindowClass();
if (!registerWindowClass()) {
return false;
}
window_ = CreateWindowExW(
WS_EX_TRANSPARENT,
windowClassName(),
@@ -71,6 +87,9 @@ public:
nullptr
);
if (!window_) {
lastErrorMessage_ = formatLastError(
"Failed to create embedded MPV child window."
);
return false;
}
@@ -127,11 +146,11 @@ private:
return L"IPTVnatorEmbeddedMpvHostWindow";
}
static void registerWindowClass()
static bool registerWindowClass()
{
static bool registered = false;
if (registered) {
return;
return true;
}
WNDCLASSEXW windowClass{};
@@ -142,8 +161,19 @@ private:
windowClass.hCursor = LoadCursorW(nullptr, IDC_ARROW);
windowClass.hbrBackground =
reinterpret_cast<HBRUSH>(GetStockObject(BLACK_BRUSH));
RegisterClassExW(&windowClass);
const ATOM classAtom = RegisterClassExW(&windowClass);
if (classAtom == 0) {
const DWORD error = GetLastError();
if (error != ERROR_CLASS_ALREADY_EXISTS) {
lastErrorMessage_ = formatLastError(
"Failed to register embedded MPV child window class."
);
return false;
}
}
registered = true;
return true;
}
static LRESULT CALLBACK windowProc(
@@ -165,6 +195,7 @@ private:
HWND parentWindow_ = nullptr;
HWND window_ = nullptr;
static inline std::string lastErrorMessage_;
};
} // namespace
@@ -6,6 +6,25 @@ describe('Embedded MPV native source recording invariants', () => {
path.resolve(__dirname, '../../../native/src/embedded_mpv.mm'),
'utf8'
);
const widCommonSource = readFileSync(
path.resolve(
__dirname,
'../../../native/src/embedded_mpv_wid_common.h'
),
'utf8'
);
const win32Source = readFileSync(
path.resolve(__dirname, '../../../native/src/embedded_mpv_win32.cc'),
'utf8'
);
const linuxSource = readFileSync(
path.resolve(__dirname, '../../../native/src/embedded_mpv_linux.cc'),
'utf8'
);
const buildScriptSource = readFileSync(
path.resolve(__dirname, '../../../build-embedded-mpv.js'),
'utf8'
);
function functionBody(name: string): string {
const start = nativeSource.indexOf(`Napi::Value ${name}(`);
@@ -67,6 +86,44 @@ describe('Embedded MPV native source recording invariants', () => {
expect(endFileCase).toContain('SessionStatus::Ended');
expect(endFileCase).toContain('MPV_END_FILE_REASON_EOF');
});
it('keeps Windows/Linux non-load async MPV command failures non-fatal', () => {
expect(widCommonSource).toContain('pendingPlaybackLoadRequestId');
expect(widCommonSource).toContain('reconcilePlaybackLoadReply');
expect(widCommonSource).toContain(
'session->snapshot.error = mpv_error_string(event->error);'
);
expect(widCommonSource).not.toContain(
'if (event->error < 0) {\n' +
' session->snapshot.status = SessionStatus::Error;'
);
});
it('copies Windows runtime DLLs next to the addon for Windows loader lookup', () => {
expect(buildScriptSource).toContain(
"for (const windowsDllName of ['mpv-2.dll', 'mpv.dll'])"
);
expect(buildScriptSource).toContain(
'path.join(outputDir, windowsDllName)'
);
expect(buildScriptSource).toContain("fileName.endsWith('.dll')");
expect(buildScriptSource).toContain('path.join(outputDir, fileName)');
});
it('checks Win32 window class registration failures explicitly', () => {
expect(win32Source).toContain('const ATOM classAtom = RegisterClassExW');
expect(win32Source).toContain('ERROR_CLASS_ALREADY_EXISTS');
expect(win32Source).toContain(
'Failed to register embedded MPV child window class.'
);
});
it('drains Linux X11 events after resizing the embedded window', () => {
expect(linuxSource).toContain('void drainEvents()');
expect(linuxSource).toContain('XPending(display_)');
expect(linuxSource).toContain('XNextEvent(display_, &event)');
expect(linuxSource).toContain('drainEvents();');
});
});
describe('Embedded MPV native build configuration', () => {
@@ -190,7 +190,8 @@ test('embedded MPV package validation accepts Windows and Linux runtime files',
try {
for (const [platform, runtimeFile] of [
['windows', join('lib', 'mpv-2.dll')],
['windows', 'mpv-2.dll'],
['windows', join('lib', 'mpv.dll')],
['linux', join('lib', 'libmpv.so.2')],
]) {
const resourceDir = join(tempDir, platform);
+9 -4
View File
@@ -399,7 +399,7 @@ function validateNoForbiddenRuntimeLinks(binaryPaths) {
return errors;
}
function getPackagedRuntimeCandidates(libDir, platform) {
function getPackagedRuntimeCandidates(libDir, platform, nativeDir) {
switch (platform) {
case 'darwin':
return [
@@ -407,11 +407,12 @@ function getPackagedRuntimeCandidates(libDir, platform) {
path.join(libDir, 'libmpv.dylib'),
];
case 'win32':
case 'windows':
return [
nativeDir ? path.join(nativeDir, 'mpv-2.dll') : null,
nativeDir ? path.join(nativeDir, 'mpv.dll') : null,
path.join(libDir, 'mpv-2.dll'),
path.join(libDir, 'mpv.dll'),
];
].filter(Boolean);
case 'linux':
return [
path.join(libDir, 'libmpv.so.2'),
@@ -467,7 +468,11 @@ function validatePackagedEmbeddedMpv(resourceDir, options = {}) {
}
}
const runtimeCandidates = getPackagedRuntimeCandidates(libDir, platform);
const runtimeCandidates = getPackagedRuntimeCandidates(
libDir,
platform,
unpackedNativeDir
);
if (
runtimeCandidates.length > 0 &&
!runtimeCandidates.some((candidate) => fs.existsSync(candidate))