From 8e105ecb651f3b00b5c826aac78c050c44208da5 Mon Sep 17 00:00:00 2001 From: joaner <1726541+joaner@users.noreply.github.com> Date: Sat, 17 Oct 2026 23:21:11 +0900 Subject: [PATCH] fix(player): refill every panel after stepping back one message Stepping one message back while paused left image panels black with "Waiting for image data" until reload. stepMessage delivered the adjacent message (one topic) before announcing the new time; panels treat a time that moves backwards as a rewind and clear, so the frame that had just arrived was wiped and the other topics never received one. stepMessage now announces the new time first, as seek does, and then backfills every subscribed topic at that time. If the backfill fails the stepped message is still delivered. --- src/core/players/IterablePlayer.test.ts | 65 +++++++++++++++++++++++++ src/core/players/IterablePlayer.ts | 21 +++++++- tests/step-back-images.spec.ts | 30 ++++++++++++ 3 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 tests/step-back-images.spec.ts diff --git a/src/core/players/IterablePlayer.test.ts b/src/core/players/IterablePlayer.test.ts index 89b2f5b..b03743d 100644 --- a/src/core/players/IterablePlayer.test.ts +++ b/src/core/players/IterablePlayer.test.ts @@ -1445,3 +1445,68 @@ describe('IterablePlayer initialize logging', () => { expect(logs.error).toContain('IterablePlayer: initialization failed'); }); }); + +describe('IterablePlayer stepMessage', () => { + const OTHER_TOPIC = '/camera/rear/image/compressed'; + + function imageAt(topic: string, ms: number): MessageEvent { + return { ...makeImageMessageAtMs(ms), topic }; + } + + it('stepping back while paused announces the new time first, then refreshes every topic at it', async () => { + const source = makeSource([]); + vi.mocked(source.initialize).mockImplementation(async () => ({ + ...makeInitialization(), + topics: [ + { name: TOPIC, type: 'sensor_msgs/CompressedImage' }, + { name: OTHER_TOPIC, type: 'sensor_msgs/CompressedImage' }, + ], + })); + // The latest message per topic at or before the requested time. + vi.mocked(source.getBackfillMessages).mockImplementation(async ({ time, topics }) => { + const ms = time.sec * 1000 + Math.floor(time.nsec / 1_000_000); + const frames = [imageAt(TOPIC, 0), imageAt(TOPIC, 400), imageAt(OTHER_TOPIC, 0), imageAt(OTHER_TOPIC, 390)]; + return topics.flatMap((topic) => { + const latest = frames + .filter((f) => f.topic === topic && f.receiveTime.sec * 1000 + f.receiveTime.nsec / 1_000_000 <= ms) + .at(-1); + return latest ? [latest] : []; + }); + }); + vi.mocked(source.getAdjacentMessage).mockImplementation(async () => imageAt(TOPIC, 400)); + + const player = new IterablePlayer(source); + await player.initialize({}); + const events: string[] = []; + for (const topic of [TOPIC, OTHER_TOPIC]) { + player.registerHighFrequencyConsumer(`panel:${topic}`, { + topic, + lane: 'video', + onLatestMessage: (message) => { + events.push(`frame ${message.topic} ${message.receiveTime.nsec / 1_000_000}`); + }, + }); + } + player.seek({ sec: 0, nsec: 500_000_000 }); + await flushAsyncWork(); + await flushAsyncWork(); + player.subscribeCurrentTime((time) => { + events.push(`time ${time.nsec / 1_000_000}`); + }); + events.length = 0; + + player.stepMessage(-1); + await flushAsyncWork(); + await flushAsyncWork(); + + // Panels clear on a rewind, so the new time must come before the frames + // that fill them, and every topic (not only the stepped one) must get one. + const timeIndex = events.indexOf('time 400'); + expect(timeIndex).toBeGreaterThanOrEqual(0); + const after = events.slice(timeIndex + 1); + expect(after).toContain(`frame ${TOPIC} 400`); + expect(after).toContain(`frame ${OTHER_TOPIC} 390`); + + player.close(); + }); +}); diff --git a/src/core/players/IterablePlayer.ts b/src/core/players/IterablePlayer.ts index c72fa47..e4e8d9b 100644 --- a/src/core/players/IterablePlayer.ts +++ b/src/core/players/IterablePlayer.ts @@ -629,8 +629,27 @@ export class IterablePlayer implements Player { this._currentTime = this._clampToRange(msg.receiveTime); this._clock.seek(this._currentTime, performance.now()); this._topicLastMessageNs.clear(); - this._distributeMessages([msg], this._currentTime); + // Announce the new time before delivering frames, as seek does: panels + // clear on a rewind, and frames delivered first would be wiped. this._notifyTimeSubscribers(this._currentTime); + // Refresh every subscribed topic at the new time, not only the topic of + // the adjacent message, so a step back leaves no panel empty. + let backfill: MessageEvent[] = []; + try { + backfill = await this._withSourceTimeout( + this._source.getBackfillMessages({ time: this._currentTime, topics }), + "playback backfill timed out", + ); + this._noteSourceSuccess(); + } catch (err) { + // Still show the stepped message; other panels refresh on the next step or play. + console.warn("IterablePlayer: stepMessage backfill failed", err); + } + if (!this._isPlaybackEpochCurrent(epoch)) return; + const hasSteppedMessage = backfill.some( + (event) => event.topic === msg.topic && toNano(event.receiveTime) === toNano(msg.receiveTime), + ); + this._distributeMessages(hasSteppedMessage ? backfill : [...backfill, msg], this._currentTime); this._lastPipelineEmitMs = 0; this._emitState(); this._scheduleNextTick(); diff --git a/tests/step-back-images.spec.ts b/tests/step-back-images.spec.ts new file mode 100644 index 0000000..dadc21b --- /dev/null +++ b/tests/step-back-images.spec.ts @@ -0,0 +1,30 @@ +import { test, expect } from '@playwright/test'; +import { MCAP_3CAM_URL, requireExamplesDir } from './fixturePaths'; +import { openFixtureByUrl } from './helpers/rosview'; + +test.beforeAll(() => { + requireExamplesDir(); +}); + +test('stepping back one message while paused keeps every image panel showing a frame', async ({ page }) => { + await page.setViewportSize({ width: 1440, height: 900 }); + await openFixtureByUrl(page, MCAP_3CAM_URL); + const waiting = page.getByText('Waiting for image data'); + const panels = page.locator('canvas'); + await expect(panels).toHaveCount(3, { timeout: 30_000 }); + await expect(waiting).toHaveCount(0, { timeout: 30_000 }); + + // Paused: step forward a few messages, then back. Each backward step rewinds + // the panels, which must be refilled at the new time on every camera. + await page.locator('body').click({ position: { x: 1, y: 1 } }); + const pause = page.getByRole('button', { name: 'Pause playback' }); + if (await pause.isVisible()) await pause.click(); + await page.evaluate(() => (document.activeElement as HTMLElement | null)?.blur()); + for (let i = 0; i < 9; i++) await page.keyboard.press('ArrowRight'); + for (let i = 0; i < 6; i++) { + await page.keyboard.press('ArrowLeft'); + await page.waitForTimeout(i % 2 ? 40 : 400); + } + await page.waitForTimeout(1_000); + await expect(waiting).toHaveCount(0); +});