feat: Pause mediaContentTimeSpent calculation on ad breaks by mmustafa-tse · Pull Request #777 · mParticle/mparticle-web-media-sdk

mmustafa-tse · 2025-10-30T17:59:37Z

Background

Per this doc, Currently, the mParticle Media SDK requires developers to manually manage content time tracking during ad breaks, creating additional complexity and potential for error. If not managed appropriately, the media content time spent value is overstated with ad break time. This PR adds an (optional) ability to allow pausing mediaContentTimeSpent when an Ad Break starts and ends

What Has Changed

Added a true/false flag (false by default) on the mediaSession init level that when true, mediaContentTimeSpent will be paused when an Ad Break starts and ends, otherwise when false, the mediaConentTimeSpent will stay the same as is

Screenshots/Video

{Include any screenshots or video demonstrating the new feature or fix, if applicable}

Checklist

I have performed a self-review of my own code.
I have made corresponding changes to the documentation.
I have added tests that prove my fix is effective or that my feature works.
I have tested this locally.

Additional Notes

same feature will be added to iOS, Android and Roku media SDKs

Reference Issue (For employees only. Ignore if you are an outside contributor)

Closes https://go/j/SDKE-446

Copilot

Pull request overview

This PR adds an optional feature to pause mediaContentTimeSpent calculation during ad breaks. When the new pauseOnAdBreak flag is set to true on MediaSession initialization, the time spent in ad breaks will be excluded from content time tracking. This addresses the issue where developers previously had to manually manage content time tracking during ad breaks, which could lead to overstated media content time values.

Adds pauseOnAdBreak boolean parameter to MediaSession constructor (defaults to false)
Implements ad break time tracking using timestamps on AdBreak start/end
Modifies mediaContentTimeSpent() calculation to subtract accumulated ad break time when the flag is enabled

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 10 comments.

File	Description
src/session.ts	Adds pauseOnAdBreak parameter, implements ad break time tracking logic, and modifies mediaContentTimeSpent calculation
src/types.ts	Extends AdBreak type with optional timestamp fields for tracking ad break duration
test/session.test.ts	Adds tests for pauseOnAdBreak feature (both enabled/disabled cases) and updates existing tests with new constructor parameter

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-11T20:53:36Z

test/session.test.ts

+            // Another 100ms delay added after logPause is triggered to account for time spent on media session (total = +200ms).
+            await new Promise(f => setTimeout(f, 100));
+            customSession.logAdBreakEnd(options);
+            // Another 100ms delay added after logPause is triggered to account for time spent on media session (total = +200ms).


The comment mentions "logPause is triggered" but logPause is not actually called in this test. This delay occurs after logAdBreakEnd, before logMediaSessionEnd.

Suggested change

// Another 100ms delay added after logPause is triggered to account for time spent on media session (total = +200ms).

// Another 100ms delay added after logAdBreakEnd to account for time spent on media session (total = +200ms).

Copilot · 2025-12-11T20:53:37Z

test/session.test.ts

+            // Another 100ms delay added after logPause is triggered to account for time spent on media session (total = +200ms).
+            await new Promise(f => setTimeout(f, 100));
+            customSession.logAdBreakEnd(options);
+            // Another 100ms delay added after logPause is triggered to account for time spent on media session (total = +200ms).


The comment mentions "logPause is triggered" but logPause is not actually called in this test. This delay occurs after logAdBreakEnd, before logMediaSessionEnd.

Suggested change

// Another 100ms delay added after logPause is triggered to account for time spent on media session (total = +200ms).

// Another 100ms delay added after logAdBreakEnd to account for time spent on media session (total = +200ms).

Copilot · 2025-12-11T20:53:37Z

test/session.test.ts

+        it('should pause mediaContentTimeSpent when pauseOnAdBreak is true', async () => {
+            const mediaSessionAttributes = {
+                session_name: 'amazing-current-session',
+                session_start_time: 'right-now',
+                custom_session_value: 'this-is-custom',
+            };
+
+            const customSession: MediaSession = new MediaSession(
+                mp,
+                song.contentId,
+                song.title,
+                song.duration,
+                song.contentType,
+                song.streamType,
+                false,
+                true,
+                true,
+                mediaSessionAttributes,
+            );
+
+            const bond = sinon.spy(mp, 'logBaseEvent');
+
+            customSession.logMediaSessionStart();
+
+            const options = {
+                customAttributes: {
+                    content_rating: 'epic',
+                },
+                currentPlayheadPosition: 0,
+            };
+
+            const adBreak: AdBreak = {
+                id: '08123410',
+                title: 'mid-roll',
+                duration: 10000,
+            };
+
+            // logPlay is triggered to start media content time tracking.
+            customSession.logPlay(options);
+            // 100ms delay added to account for the time spent on media content.
+            await new Promise(f => setTimeout(f, 100));
+            customSession.logAdBreakStart(adBreak);
+            // Another 100ms delay added after logPause is triggered to account for time spent on media session (total = +200ms).
+            await new Promise(f => setTimeout(f, 100));
+            customSession.logAdBreakEnd(options);
+            // Another 100ms delay added after logPause is triggered to account for time spent on media session (total = +200ms).
+            await new Promise(f => setTimeout(f, 100));
+            customSession.logMediaSessionEnd(options);
+
+            // the 6th event in bond.args is the Media Session Summary which contains the mediaContentTimeSpent and mediaTimeSpent.
+            const mpMediaContentTimeSpent =
+                bond.args[5][0].options.customAttributes
+                    .media_content_time_spent;
+
+            // the mediaContentTimeSpent varies in value each test run by a millisecond or two (i,e value is could be 100ms, 101ms, 102ms)
+            // and we can't determine the exact value, hence the greaterThanOrEqual and lessThanOrEqual tests.
+            expect(mpMediaContentTimeSpent).to.greaterThanOrEqual(200);
+            expect(mpMediaContentTimeSpent).to.lessThanOrEqual(300);
+        });


The test for pauseOnAdBreak feature doesn't include scenarios where logPause is called during or after an ad break. This is important to test because the storedPlaybackTime calculation in logPause doesn't account for ad break time when pauseOnAdBreak is enabled, which could lead to incorrect mediaContentTimeSpent values. Consider adding test cases that call logPlay, logAdBreakStart, logAdBreakEnd, logPause, and then verify mediaContentTimeSpent excludes ad break time.

Copilot · 2025-12-11T20:53:37Z

src/types.ts

+     */
+    adBreakStartTimestamp?: number;
+    /**
+     * Timestamp for AdBreak end playback
+     */
+    adBreakEndTimestamp?: number;


The adBreakStartTimestamp and adBreakEndTimestamp fields are added to the AdBreak type as optional properties, but they are being used internally for tracking purposes rather than being set by the API consumer. Consider whether these implementation details should be exposed in the public AdBreak type, or if they should be tracked separately in the MediaSession class. This could prevent API consumers from inadvertently setting these values when creating AdBreak objects.

Suggested change

*/

adBreakStartTimestamp?: number;

/**

* Timestamp for AdBreak end playback

*/

adBreakEndTimestamp?: number;

* (internal use only)

*/

// adBreakStartTimestamp?: number;

/**

* Timestamp for AdBreak end playback

* (internal use only)

// adBreakEndTimestamp?: number;

Copilot · 2025-12-11T20:53:38Z

test/session.test.ts

+                bond.args[5][0].options.customAttributes
+                    .media_content_time_spent;
+
+            // the mediaContentTimeSpent varies in value each test run by a millisecond or two (i,e value is could be 100ms, 101ms, 102ms)


Typo: "i,e" should be "i.e." (the abbreviation for "id est" meaning "that is").

Suggested change

// the mediaContentTimeSpent varies in value each test run by a millisecond or two (i,e value is could be 100ms, 101ms, 102ms)

// the mediaContentTimeSpent varies in value each test run by a millisecond or two (i.e. value could be 100ms, 101ms, 102ms)

Copilot · 2025-12-11T20:53:38Z

test/session.test.ts

+                bond.args[5][0].options.customAttributes
+                    .media_content_time_spent;
+
+            // the mediaContentTimeSpent varies in value each test run by a millisecond or two (i,e value is could be 100ms, 101ms, 102ms)


Typo: "i,e" should be "i.e." (the abbreviation for "id est" meaning "that is").

Suggested change

// the mediaContentTimeSpent varies in value each test run by a millisecond or two (i,e value is could be 100ms, 101ms, 102ms)

// the mediaContentTimeSpent varies in value each test run by a millisecond or two (i.e. value could be 100ms, 101ms, 102ms)

Copilot · 2025-12-11T20:53:38Z

src/session.ts

+                    mediaAdBreakTimeSpent)
            );
        } else {
            return this.storedPlaybackTime;


When pauseOnAdBreak is enabled and playback is paused (currentPlaybackStartTimestamp is undefined), mediaContentTimeSpent should still exclude ad break time by subtracting mediaAdBreakTimeSpent from storedPlaybackTime. Currently, only the active playback branch (line 109-114) subtracts ad break time, but the paused branch returns storedPlaybackTime without this adjustment. This causes mediaContentTimeSpent to incorrectly include ad break duration when calculated after a pause.

Suggested change

return this.storedPlaybackTime;

return this.storedPlaybackTime - mediaAdBreakTimeSpent;

Copilot · 2025-12-11T20:53:39Z

test/session.test.ts

+            // Another 100ms delay added after logPause is triggered to account for time spent on media session (total = +200ms).
+            await new Promise(f => setTimeout(f, 100));
+            customSession.logAdBreakEnd(options);
+            // Another 100ms delay added after logPause is triggered to account for time spent on media session (total = +200ms).


The comment mentions "logPause is triggered" but logPause is not actually called in this test. The comment should accurately describe that this delay occurs after logAdBreakStart, before logAdBreakEnd. The same issue exists in the comment at line 232 (after logAdBreakEnd, before logMediaSessionEnd).

Suggested change

// Another 100ms delay added after logPause is triggered to account for time spent on media session (total = +200ms).

await new Promise(f => setTimeout(f, 100));

customSession.logAdBreakEnd(options);

// Another 100ms delay added after logPause is triggered to account for time spent on media session (total = +200ms).

// Another 100ms delay added after logAdBreakStart and before logAdBreakEnd to account for time spent on media session (total = +200ms).

await new Promise(f => setTimeout(f, 100));

customSession.logAdBreakEnd(options);

// Another 100ms delay added after logAdBreakEnd and before logMediaSessionEnd to account for time spent on media session (total = +300ms).

Copilot · 2025-12-11T20:53:39Z

test/session.test.ts

+            // 100ms delay added to account for the time spent on media content.
+            await new Promise(f => setTimeout(f, 100));
+            customSession.logAdBreakStart(adBreak);
+            // Another 100ms delay added after logPause is triggered to account for time spent on media session (total = +200ms).


The comment mentions "logPause is triggered" but logPause is not actually called in this test. This delay occurs after logAdBreakStart, before logAdBreakEnd.

Suggested change

// Another 100ms delay added after logPause is triggered to account for time spent on media session (total = +200ms).

// Another 100ms delay added after logAdBreakStart to account for time spent on media session (total = +200ms).

Copilot · 2025-12-11T20:53:39Z

src/session.ts

     * @param streamType A descriptor for the type of stream, i.e. live or on demand
     * @param logPageEvent A flag that toggles sending mParticle Events to Core SDK
     * @param logMediaEvent A flag that toggles sending Media Events to Core SDK
+     * @param pauseOnAdBreak A flag that toggles wether to stop calculating mediaContentTimespent when ad break events are logged


Typo: "wether" should be "whether"

Suggested change

* @param pauseOnAdBreak A flag that toggles wether to stop calculating mediaContentTimespent when ad break events are logged

* @param pauseOnAdBreak A flag that toggles whether to stop calculating mediaContentTimespent when ad break events are logged

feat: Pause mediaContentTimeSpent calculation on ad breaks

d00d96b

nickolas-dimitrakas mentioned this pull request Nov 6, 2025

feat: update media content time spent calculations with ad breaks mParticle/mparticle-apple-media-sdk#22

Merged

4 tasks

alexs-mparticle requested a review from Copilot December 11, 2025 20:43

Copilot started reviewing on behalf of alexs-mparticle December 11, 2025 20:44 View session

Copilot AI reviewed Dec 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Pause mediaContentTimeSpent calculation on ad breaks#777

feat: Pause mediaContentTimeSpent calculation on ad breaks#777
mmustafa-tse wants to merge 1 commit intomParticle:developmentfrom
mmustafa-tse:feat/adBreakPause

mmustafa-tse commented Oct 30, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 11, 2025

Uh oh!

Copilot AI Dec 11, 2025

Uh oh!

Copilot AI Dec 11, 2025

Uh oh!

Copilot AI Dec 11, 2025

Uh oh!

Copilot AI Dec 11, 2025

Uh oh!

Copilot AI Dec 11, 2025

Uh oh!

Copilot AI Dec 11, 2025

Uh oh!

Copilot AI Dec 11, 2025

Uh oh!

Copilot AI Dec 11, 2025

Uh oh!

Copilot AI Dec 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	// Another 100ms delay added after logPause is triggered to account for time spent on media session (total = +200ms).
	// Another 100ms delay added after logAdBreakEnd to account for time spent on media session (total = +200ms).

	// the mediaContentTimeSpent varies in value each test run by a millisecond or two (i,e value is could be 100ms, 101ms, 102ms)
	// the mediaContentTimeSpent varies in value each test run by a millisecond or two (i.e. value could be 100ms, 101ms, 102ms)

	return this.storedPlaybackTime;
	return this.storedPlaybackTime - mediaAdBreakTimeSpent;

	* @param pauseOnAdBreak A flag that toggles wether to stop calculating mediaContentTimespent when ad break events are logged
	* @param pauseOnAdBreak A flag that toggles whether to stop calculating mediaContentTimespent when ad break events are logged

Conversation

mmustafa-tse commented Oct 30, 2025

Background

What Has Changed

Screenshots/Video

Checklist

Additional Notes

Reference Issue (For employees only. Ignore if you are an outside contributor)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants