From 2f18b47d95b79f5e8109e155c8ff36c190569569 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Velad=20Galv=C3=A1n?= Date: Fri, 14 Mar 2025 14:01:28 +0100 Subject: [PATCH] fix: Update region observer with new rules for small regions (#8275) This is necessary because some EMSG and ID3 events might not fire because the endTime is equal to or very close to the startTime. Related to https://github.com/shaka-project/shaka-player/pull/8012 --- lib/media/region_observer.js | 11 +++++++- test/media/region_observer_unit.js | 45 +++++++++++++++++------------- 2 files changed, 36 insertions(+), 20 deletions(-) diff --git a/lib/media/region_observer.js b/lib/media/region_observer.js index d6ffa7673..a519378e2 100644 --- a/lib/media/region_observer.js +++ b/lib/media/region_observer.js @@ -96,7 +96,16 @@ shaka.media.RegionObserver = class extends shaka.util.FakeEventTarget { { weWere: BEFORE_THE_REGION, weAre: AFTER_THE_REGION, - invoke: (region, seeking) => this.onEvent_('skip', region, seeking), + invoke: (region, seeking) => { + if (seeking) { + this.onEvent_('skip', region, seeking); + } else { + // This is the case of regions whose duration is smaller than the + // resolution of our polling. + this.onEvent_('enter', region, seeking); + this.onEvent_('exit', region, seeking); + } + }, }, { weWere: AFTER_THE_REGION, diff --git a/test/media/region_observer_unit.js b/test/media/region_observer_unit.js index fd9c43627..d572c3514 100644 --- a/test/media/region_observer_unit.js +++ b/test/media/region_observer_unit.js @@ -113,25 +113,29 @@ describe('RegionObserver', () => { expect(onExitRegion).toHaveBeenCalledOnceMoreWith([region, false]); }); - it('fires skip event when we enter and leave region in one move', () => { - const region = createRegion('my-region', 5, 10); - timeline.addRegion(region); + it('fires start and end events when we enter and leave region in one move', + () => { + const region = createRegion('my-region', 5, 10); + timeline.addRegion(region); - // Make sure we are before the region starts. - poll(observer, - /* timeInSeconds= */ 4, - /* seeking= */ false); - expect(onSkipRegion).not.toHaveBeenCalled(); + // Make sure we are before the region starts. + poll(observer, + /* timeInSeconds= */ 4, + /* seeking= */ false); + expect(onEnterRegion).not.toHaveBeenCalled(); + expect(onExitRegion).not.toHaveBeenCalled(); + expect(onSkipRegion).not.toHaveBeenCalled(); - // Make sure we call |onSkip| when we move so far that we skip over the - // region. - poll(observer, - /* timeInSeconds= */ 15, - /* seeking= */ false); - expect(onSkipRegion).toHaveBeenCalledOnceMoreWith([region, false]); - }); + // Make sure we call |onEnter| and |onExit| + poll(observer, + /* timeInSeconds= */ 15, + /* seeking= */ false); + expect(onEnterRegion).toHaveBeenCalledOnceMoreWith([region, false]); + expect(onExitRegion).toHaveBeenCalledOnceMoreWith([region, false]); + expect(onSkipRegion).not.toHaveBeenCalled(); + }); - it('fires skip events for zero-duration regions', () => { + it('fires start and end events for zero-duration regions', () => { // Make a region with no duration. const region = createRegion('my-region', 5, 5); timeline.addRegion(region); @@ -140,14 +144,17 @@ describe('RegionObserver', () => { poll(observer, /* timeInSeconds= */ 4, /* seeking= */ false); + expect(onEnterRegion).not.toHaveBeenCalled(); + expect(onExitRegion).not.toHaveBeenCalled(); expect(onSkipRegion).not.toHaveBeenCalled(); - // Make sure we call |onSkip| when we move so far that we skip over the - // region. + // Make sure we call |onEnter| and |onExit| poll(observer, /* timeInSeconds= */ 10, /* seeking= */ false); - expect(onSkipRegion).toHaveBeenCalledOnceMoreWith([region, false]); + expect(onEnterRegion).toHaveBeenCalledOnceMoreWith([region, false]); + expect(onExitRegion).toHaveBeenCalledOnceMoreWith([region, false]); + expect(onSkipRegion).not.toHaveBeenCalled(); }); // We want to simulate a "normal" case of overlapping regions. For this we