fix(playback): address embedded mpv recording review

Entire-Checkpoint: f957cd9849e0

apps/electron-backend/native/src/embedded_mpv.mm

@@ -127,6 +127,11 @@ struct Session {
     std::atomic<uint64_t> totalRenderNanoseconds{0};
     std::mutex mutex;
     SessionSnapshot snapshot;
+    uint64_t pendingRecordingStartRequestId = 0;
+    uint64_t pendingRecordingStopRequestId = 0;
+    std::string pendingRecordingTargetPath;
+    std::string pendingRecordingStartedAt;
+    std::string pendingRecordingStopStartedAt;
     std::weak_ptr<Session>* renderCallbackContext = nullptr;
     RenderBackend renderBackend = RenderBackend::OpenGL;
     int renderWidthPixels = 0;
@@ -1030,6 +1035,59 @@ std::string currentUtcTimestamp()
     return buffer;
 }
 
+bool reconcileRecordingPropertyReply(
+    const std::shared_ptr<Session>& session,
+    uint64_t requestId,
+    int error
+)
+{
+    if (requestId == 0) {
+        return false;
+    }
+
+    if (requestId == session->pendingRecordingStartRequestId) {
+        session->pendingRecordingStartRequestId = 0;
+        const std::string targetPath = session->pendingRecordingTargetPath;
+        const std::string startedAt = session->pendingRecordingStartedAt;
+        session->pendingRecordingTargetPath.clear();
+        session->pendingRecordingStartedAt.clear();
+
+        if (error < 0) {
+            session->snapshot.recordingActive = false;
+            session->snapshot.recordingTargetPath = targetPath;
+            session->snapshot.recordingStartedAt.clear();
+            session->snapshot.recordingError = mpv_error_string(error);
+            return true;
+        }
+
+        session->snapshot.recordingActive = true;
+        session->snapshot.recordingTargetPath = targetPath;
+        session->snapshot.recordingStartedAt = startedAt;
+        session->snapshot.recordingError.clear();
+        return true;
+    }
+
+    if (requestId == session->pendingRecordingStopRequestId) {
+        session->pendingRecordingStopRequestId = 0;
+        const std::string startedAt = session->pendingRecordingStopStartedAt;
+        session->pendingRecordingStopStartedAt.clear();
+
+        if (error < 0) {
+            session->snapshot.recordingActive = true;
+            session->snapshot.recordingStartedAt = startedAt;
+            session->snapshot.recordingError = mpv_error_string(error);
+            return true;
+        }
+
+        session->snapshot.recordingActive = false;
+        session->snapshot.recordingStartedAt.clear();
+        session->snapshot.recordingError.clear();
+        return true;
+    }
+
+    return false;
+}
+
 double clampVolumePercent(double volume)
 {
     return std::clamp(volume, 0.0, 1.0) * 100.0;
@@ -1227,6 +1285,13 @@ void runEventLoop(const std::shared_ptr<Session>& session)
             }
             case MPV_EVENT_COMMAND_REPLY:
             case MPV_EVENT_SET_PROPERTY_REPLY:
+                if (reconcileRecordingPropertyReply(
+                        session,
+                        event->reply_userdata,
+                        event->error
+                    )) {
+                    break;
+                }
                 if (event->error < 0) {
                     session->snapshot.status = SessionStatus::Error;
                     session->snapshot.error = mpv_error_string(event->error);
@@ -2095,9 +2160,22 @@ Napi::Value StartRecording(const Napi::CallbackInfo& info)
 
     const auto session = getSessionOrThrow(env, sessionId);
     const char* targetValue = targetPath.c_str();
+    const uint64_t requestId = nextAsyncRequestId();
+    const std::string startedAt = currentUtcTimestamp();
+    {
+        std::lock_guard<std::mutex> lock(session->mutex);
+        session->pendingRecordingStartRequestId = requestId;
+        session->pendingRecordingTargetPath = targetPath;
+        session->pendingRecordingStartedAt = startedAt;
+        session->snapshot.recordingActive = true;
+        session->snapshot.recordingTargetPath = targetPath;
+        session->snapshot.recordingStartedAt = startedAt;
+        session->snapshot.recordingError.clear();
+    }
+
     const int result = mpv_set_property_async(
         session->handle,
-        nextAsyncRequestId(),
+        requestId,
         "stream-record",
         MPV_FORMAT_STRING,
         const_cast<char**>(&targetValue)
@@ -2106,7 +2184,13 @@ Napi::Value StartRecording(const Napi::CallbackInfo& info)
     if (result < 0) {
         {
             std::lock_guard<std::mutex> lock(session->mutex);
+            if (session->pendingRecordingStartRequestId == requestId) {
+                session->pendingRecordingStartRequestId = 0;
+                session->pendingRecordingTargetPath.clear();
+                session->pendingRecordingStartedAt.clear();
+            }
             session->snapshot.recordingActive = false;
+            session->snapshot.recordingStartedAt.clear();
             session->snapshot.recordingError = mpv_error_string(result);
         }
         throw Napi::Error::New(
@@ -2116,14 +2200,6 @@ Napi::Value StartRecording(const Napi::CallbackInfo& info)
         );
     }
 
-    {
-        std::lock_guard<std::mutex> lock(session->mutex);
-        session->snapshot.recordingActive = true;
-        session->snapshot.recordingTargetPath = targetPath;
-        session->snapshot.recordingStartedAt = currentUtcTimestamp();
-        session->snapshot.recordingError.clear();
-    }
-
     return env.Undefined();
 }
 
@@ -2137,9 +2213,20 @@ Napi::Value StopRecording(const Napi::CallbackInfo& info)
     const std::string sessionId = info[0].As<Napi::String>().Utf8Value();
     const auto session = getSessionOrThrow(env, sessionId);
     const char* disabledValue = "";
+    const uint64_t requestId = nextAsyncRequestId();
+    {
+        std::lock_guard<std::mutex> lock(session->mutex);
+        session->pendingRecordingStopRequestId = requestId;
+        session->pendingRecordingStopStartedAt =
+            session->snapshot.recordingStartedAt;
+        session->snapshot.recordingActive = false;
+        session->snapshot.recordingStartedAt.clear();
+        session->snapshot.recordingError.clear();
+    }
+
     const int result = mpv_set_property_async(
         session->handle,
-        nextAsyncRequestId(),
+        requestId,
         "stream-record",
         MPV_FORMAT_STRING,
         const_cast<char**>(&disabledValue)
@@ -2148,6 +2235,14 @@ Napi::Value StopRecording(const Napi::CallbackInfo& info)
     if (result < 0) {
         {
             std::lock_guard<std::mutex> lock(session->mutex);
+            const std::string startedAt =
+                session->pendingRecordingStopStartedAt;
+            if (session->pendingRecordingStopRequestId == requestId) {
+                session->pendingRecordingStopRequestId = 0;
+                session->pendingRecordingStopStartedAt.clear();
+            }
+            session->snapshot.recordingActive = true;
+            session->snapshot.recordingStartedAt = startedAt;
             session->snapshot.recordingError = mpv_error_string(result);
         }
         throw Napi::Error::New(
@@ -2157,13 +2252,6 @@ Napi::Value StopRecording(const Napi::CallbackInfo& info)
         );
     }
 
-    {
-        std::lock_guard<std::mutex> lock(session->mutex);
-        session->snapshot.recordingActive = false;
-        session->snapshot.recordingStartedAt.clear();
-        session->snapshot.recordingError.clear();
-    }
-
     return env.Undefined();
 }
 

apps/electron-backend/src/app/services/embedded-mpv-native.service.spec.ts

@@ -3,6 +3,9 @@ import type {
     EmbeddedMpvSessionStatus,
     ResolvedPortalPlayback,
 } from 'shared-interfaces';
+import { existsSync, mkdtempSync, rmSync, writeFileSync } from 'fs';
+import { tmpdir } from 'os';
+import path from 'path';
 import type { EmbeddedMpvNativeService as EmbeddedMpvNativeServiceType } from './embedded-mpv-native.service';
 
 const powerSaveBlockerMock = {
@@ -95,6 +98,7 @@ describe('EmbeddedMpvNativeService power blocker', () => {
     let addon: MockAddon;
     let nextBlockerId: number;
     let originalPlatform: NodeJS.Platform;
+    let tempDirs: string[];
 
     beforeEach(async () => {
         jest.resetModules();
@@ -103,6 +107,7 @@ describe('EmbeddedMpvNativeService power blocker', () => {
         powerSaveBlockerMock.isStarted.mockReset();
         mainWindowSendMock.mockReset();
 
+        tempDirs = [];
         nextBlockerId = 1;
         powerSaveBlockerMock.start.mockImplementation(() => nextBlockerId++);
         powerSaveBlockerMock.isStarted.mockReturnValue(true);
@@ -123,11 +128,22 @@ describe('EmbeddedMpvNativeService power blocker', () => {
     afterEach(() => {
         service.shutdown();
         jest.useRealTimers();
+        for (const tempDir of tempDirs) {
+            rmSync(tempDir, { recursive: true, force: true });
+        }
         Object.defineProperty(process, 'platform', {
             value: originalPlatform,
         });
     });
 
+    function createTempDir(): string {
+        const tempDir = mkdtempSync(
+            path.join(tmpdir(), 'iptvnator-recording-')
+        );
+        tempDirs.push(tempDir);
+        return tempDir;
+    }
+
     function startSession(sessionId: string, snapshot: MockSnapshot): void {
         addon.createSession.mockReturnValueOnce(sessionId);
         addon.getSessionSnapshot.mockReturnValueOnce(snapshot);
@@ -249,12 +265,13 @@ describe('EmbeddedMpvNativeService power blocker', () => {
         expect(support.capabilities?.recording).toBe(true);
     });
 
-    it('starts recording to a sanitized unique ts file in the requested directory', () => {
+    it('starts recording to a sanitized reserved unique ts file in the requested directory', () => {
         jest.useFakeTimers();
         jest.setSystemTime(new Date(2026, 4, 9, 10, 11, 12));
+        const directory = createTempDir();
         startSession('s1', snapshot('playing'));
 
-        const targetPath = '/tmp/News_Live-20260509-101112.ts';
+        const targetPath = path.join(directory, 'News_Live-20260509-101112.ts');
         addon.getSessionSnapshot.mockReturnValueOnce(
             snapshot('playing', {
                 recording: {
@@ -266,15 +283,53 @@ describe('EmbeddedMpvNativeService power blocker', () => {
         );
 
         const updated = service.startRecording('s1', {
-            directory: '/tmp',
+            directory,
             title: 'News/Live',
         });
 
         expect(addon.startRecording).toHaveBeenCalledWith('s1', targetPath);
+        expect(existsSync(targetPath)).toBe(true);
         expect(updated?.recording?.active).toBe(true);
         expect(updated?.recording?.targetPath).toBe(targetPath);
     });
 
+    it('reserves the next unique recording path when the first candidate exists', () => {
+        jest.useFakeTimers();
+        jest.setSystemTime(new Date(2026, 4, 9, 10, 11, 12));
+        const directory = createTempDir();
+        const firstCandidate = path.join(
+            directory,
+            'News_Live-20260509-101112.ts'
+        );
+        const reservedCandidate = path.join(
+            directory,
+            'News_Live-20260509-101112-2.ts'
+        );
+        writeFileSync(firstCandidate, 'existing recording');
+        startSession('s1', snapshot('playing'));
+
+        addon.getSessionSnapshot.mockReturnValueOnce(
+            snapshot('playing', {
+                recording: {
+                    active: true,
+                    targetPath: reservedCandidate,
+                },
+            })
+        );
+
+        service.startRecording('s1', {
+            directory,
+            title: 'News/Live',
+        });
+
+        expect(addon.startRecording).toHaveBeenCalledWith(
+            's1',
+            reservedCandidate
+        );
+        expect(existsSync(firstCandidate)).toBe(true);
+        expect(existsSync(reservedCandidate)).toBe(true);
+    });
+
     it('stops recording and keeps the last target path in the session snapshot', () => {
         const targetPath = '/tmp/News_Live-20260509-101112.ts';
         startSession(
@@ -302,4 +357,44 @@ describe('EmbeddedMpvNativeService power blocker', () => {
         expect(updated?.recording?.active).toBe(false);
         expect(updated?.recording?.targetPath).toBe(targetPath);
     });
+
+    it('preserves the recording target path when disposing an active recording session', () => {
+        const targetPath = '/tmp/News_Live-20260509-101112.ts';
+        startSession(
+            's1',
+            snapshot('playing', {
+                recording: {
+                    active: true,
+                    targetPath,
+                    startedAt: '2026-05-09T08:11:12.000Z',
+                },
+            })
+        );
+        addon.getSessionSnapshot.mockReturnValueOnce(
+            snapshot('playing', {
+                recording: {
+                    active: true,
+                    targetPath,
+                    startedAt: '2026-05-09T08:11:12.000Z',
+                },
+            })
+        );
+
+        const disposed = service.disposeSession('s1');
+
+        expect(addon.disposeSession).toHaveBeenCalledWith('s1');
+        expect(disposed?.recording).toEqual({
+            active: false,
+            targetPath,
+        });
+        expect(mainWindowSendMock).toHaveBeenLastCalledWith(
+            'EMBEDDED_MPV_SESSION_UPDATE',
+            expect.objectContaining({
+                recording: {
+                    active: false,
+                    targetPath,
+                },
+            })
+        );
+    });
 });

apps/electron-backend/src/app/services/embedded-mpv-native.service.ts

@@ -1,5 +1,12 @@
 import { app, dialog, powerSaveBlocker } from 'electron';
-import { existsSync, mkdirSync, readFileSync } from 'fs';
+import {
+    closeSync,
+    existsSync,
+    mkdirSync,
+    openSync,
+    readFileSync,
+    unlinkSync,
+} from 'fs';
 import { createRequire } from 'module';
 import path from 'path';
 import App from '../app';
@@ -383,11 +390,16 @@ export class EmbeddedMpvNativeService {
             options.directory?.trim() || this.getDefaultRecordingFolder();
         mkdirSync(directory, { recursive: true });
 
-        const targetPath = this.resolveRecordingTargetPath(
+        const targetPath = this.reserveRecordingTargetPath(
             directory,
             options.title || session.title || 'IPTVnator recording'
         );
-        addon.startRecording(sessionId, targetPath);
+        try {
+            addon.startRecording(sessionId, targetPath);
+        } catch (error) {
+            this.releaseReservedRecordingTargetPath(targetPath);
+            throw error;
+        }
         return this.refreshSession(sessionId);
     }
 
@@ -427,8 +439,15 @@ export class EmbeddedMpvNativeService {
             return null;
         }
 
+        let lastRecording: EmbeddedMpvRecordingState | undefined;
         try {
-            this.getAddon().disposeSession(sessionId);
+            const addon = this.getAddon();
+            try {
+                lastRecording = addon.getSessionSnapshot(sessionId)?.recording;
+            } catch {
+                lastRecording = undefined;
+            }
+            addon.disposeSession(sessionId);
         } finally {
             this.sessions.delete(sessionId);
             const payload: EmbeddedMpvSession = {
@@ -445,7 +464,7 @@ export class EmbeddedMpvNativeService {
                 selectedSubtitleTrackId: null,
                 playbackSpeed: 1,
                 aspectOverride: 'no',
-                recording: { active: false },
+                recording: this.createClosedRecordingState(lastRecording),
                 startedAt: session.startedAt,
                 updatedAt: new Date().toISOString(),
             };
@@ -614,7 +633,7 @@ export class EmbeddedMpvNativeService {
         return App.mainWindow.getNativeWindowHandle();
     }
 
-    private resolveRecordingTargetPath(
+    private reserveRecordingTargetPath(
         directory: string,
         title: string
     ): string {
@@ -623,15 +642,44 @@ export class EmbeddedMpvNativeService {
         let candidate = path.join(directory, `${baseName}-${timestamp}.ts`);
         let suffix = 2;
 
-        while (existsSync(candidate)) {
-            candidate = path.join(
-                directory,
-                `${baseName}-${timestamp}-${suffix}.ts`
-            );
-            suffix += 1;
-        }
+        while (true) {
+            try {
+                const fd = openSync(candidate, 'wx');
+                closeSync(fd);
+                return candidate;
+            } catch (error) {
+                if ((error as NodeJS.ErrnoException).code === 'EEXIST') {
+                    candidate = path.join(
+                        directory,
+                        `${baseName}-${timestamp}-${suffix}.ts`
+                    );
+                    suffix += 1;
+                    continue;
+                }
 
-        return candidate;
+                throw error;
+            }
+        }
+    }
+
+    private releaseReservedRecordingTargetPath(targetPath: string): void {
+        try {
+            unlinkSync(targetPath);
+        } catch {
+            // Ignore cleanup failures; the start error is more useful.
+        }
+    }
+
+    private createClosedRecordingState(
+        recording?: EmbeddedMpvRecordingState
+    ): EmbeddedMpvRecordingState {
+        return {
+            active: false,
+            ...(recording?.targetPath
+                ? { targetPath: recording.targetPath }
+                : {}),
+            ...(recording?.error ? { error: recording.error } : {}),
+        };
     }
 
     private sanitizeRecordingFileName(title: string): string {