Skip to content

Commit

Permalink
fix: Fix memory leak in DASH live streams with inband EventStream (sh…
Browse files Browse the repository at this point in the history 
…aka-project#3957)

EventStreams in DASH generate TimelineRegionInfo objects, which are
then stored in the RegionTimeline and RegionObserver classes.  But
DashParser would add all regions to RegionTimeline, even if they would
be quickly removed again, and RegionObserver would cache some regions
from the timeline without ever removing them.

This fixes the issue from both of those directions.  DashParser will
now ignore regions that are outside the DVR window (and therefore
would soon be removed from RegionTimeline), and RegionObserver listens
to an event on RegionTimeline to clean up its own storage when regions
fall outside the DVR window during playback.

Closes shaka-project#3949 (memory leak in DASH live streams with inband EventStream)
  • Loading branch information
@joeyparrish
joeyparrish authored Feb 16, 2022
1 parent a4e9267 commit b7f04cb
Show file tree
Hide file tree
Showing 7 changed files with 158 additions and 42 deletions.
50 changes: 31 additions & 19 deletions lib/dash/dash_parser.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,24 @@ shaka.dash.DashParser = class {
this.config_.dash.autoCorrectDrift);
}

presentationTimeline.setStatic(mpdType == 'static');

const isLive = presentationTimeline.isLive();

// If it's live, we check for an override.
if (isLive && !isNaN(this.config_.availabilityWindowOverride)) {
segmentAvailabilityDuration = this.config_.availabilityWindowOverride;
}

// If it's null, that means segments are always available. This is always
// the case for VOD, and sometimes the case for live.
if (segmentAvailabilityDuration == null) {
segmentAvailabilityDuration = Infinity;
}

presentationTimeline.setSegmentAvailabilityDuration(
segmentAvailabilityDuration);

const profiles = mpd.getAttribute('profiles') || '';

/** @type {shaka.dash.DashParser.Context} */
Expand All @@ -432,7 +450,6 @@ shaka.dash.DashParser = class {
const duration = periodsAndDuration.duration;
const periods = periodsAndDuration.periods;

presentationTimeline.setStatic(mpdType == 'static');
if (mpdType == 'static' ||
!periodsAndDuration.durationDerivedFromPeriods) {
// Ignore duration calculated from Period lengths if this is dynamic.
Expand All @@ -451,6 +468,7 @@ shaka.dash.DashParser = class {
this.lowLatencyMode_ = this.playerInterface_.isLowLatencyMode();
}
}

if (this.lowLatencyMode_) {
presentationTimeline.setAvailabilityTimeOffset(
this.minTotalAvailabilityTimeOffset_);
Expand All @@ -464,22 +482,6 @@ shaka.dash.DashParser = class {
'https://bit.ly/3clctcj for details.');
}

const isLive = presentationTimeline.isLive();

// If it's live, we check for an override.
if (isLive && !isNaN(this.config_.availabilityWindowOverride)) {
segmentAvailabilityDuration = this.config_.availabilityWindowOverride;
}

// If it's null, that means segments are always available. This is always
// the case for VOD, and sometimes the case for live.
if (segmentAvailabilityDuration == null) {
segmentAvailabilityDuration = Infinity;
}

presentationTimeline.setSegmentAvailabilityDuration(
segmentAvailabilityDuration);

// Use @maxSegmentDuration to override smaller, derived values.
presentationTimeline.notifyMaxSegmentDuration(maxSegmentDuration || 1);
if (goog.DEBUG) {
Expand Down Expand Up @@ -715,8 +717,12 @@ shaka.dash.DashParser = class {

const eventStreamNodes =
XmlUtils.findChildren(periodInfo.node, 'EventStream');
const availabilityStart =
context.presentationTimeline.getSegmentAvailabilityStart();

for (const node of eventStreamNodes) {
this.parseEventStream_(periodInfo.start, periodInfo.duration, node);
this.parseEventStream_(
periodInfo.start, periodInfo.duration, node, availabilityStart);
}

const adaptationSetNodes =
Expand Down Expand Up @@ -1770,9 +1776,10 @@ shaka.dash.DashParser = class {
* @param {number} periodStart
* @param {?number} periodDuration
* @param {!Element} elem
* @param {number} availabilityStart
* @private
*/
parseEventStream_(periodStart, periodDuration, elem) {
parseEventStream_(periodStart, periodDuration, elem, availabilityStart) {
const XmlUtils = shaka.util.XmlUtils;
const parseNumber = XmlUtils.parseNonNegativeInt;

Expand All @@ -1795,6 +1802,11 @@ shaka.dash.DashParser = class {
endTime = Math.min(endTime, periodStart + periodDuration);
}

// Don't add unavailable regions to the timeline.
if (endTime < availabilityStart) {
continue;
}

/** @type {shaka.extern.TimelineRegionInfo} */
const region = {
schemeIdUri: schemeIdUri,
Expand Down
13 changes: 13 additions & 0 deletions lib/media/region_observer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ goog.provide('shaka.media.RegionObserver');

goog.require('shaka.media.IPlayheadObserver');
goog.require('shaka.media.RegionTimeline');
goog.require('shaka.util.EventManager');
goog.require('shaka.util.FakeEvent');
goog.require('shaka.util.FakeEventTarget');

Expand Down Expand Up @@ -92,6 +93,15 @@ shaka.media.RegionObserver = class extends shaka.util.FakeEventTarget {
invoke: (region, seeking) => this.onEvent_('skip', region, seeking),
},
];

/** @private {shaka.util.EventManager} */
this.eventManager_ = new shaka.util.EventManager();

this.eventManager_.listen(this.timeline_, 'regionremove', (event) => {
/** @type {shaka.extern.TimelineRegionInfo} */
const region = event['region'];
this.oldPosition_.delete(region);
});
}

/** @override */
Expand All @@ -102,6 +112,9 @@ shaka.media.RegionObserver = class extends shaka.util.FakeEventTarget {
// needed.
this.oldPosition_.clear();

this.eventManager_.release();
this.eventManager_ = null;

super.release();
}

Expand Down
1 change: 1 addition & 0 deletions lib/media/region_timeline.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ shaka.media.RegionTimeline = class extends shaka.util.FakeEventTarget {

/** @private {!Set.<shaka.extern.TimelineRegionInfo>} */
this.regions_ = new Set();

/** @private {!function():{start: number, end: number}} */
this.getSeekRange_ = getSeekRange;

Expand Down
43 changes: 43 additions & 0 deletions test/dash/dash_parser_live_unit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1389,6 +1389,49 @@ describe('DashParser Live', () => {
.toHaveBeenCalledWith(
jasmine.objectContaining({id: '2', startTime: 10, endTime: 25}));
});

it('will not add timeline regions outside the DVR window', async () => {
const manifest = [
'<MPD type="dynamic" minimumUpdatePeriod="PT' + updateTime + 'S"',
' availabilityStartTime="1970-01-01T00:00:00Z"',
' suggestedPresentationDelay="PT0S"',
' timeShiftBufferDepth="PT30S">',
' <Period id="1">',
' <EventStream schemeIdUri="http://example.com" timescale="1">',
' <Event id="0" presentationTime="0" duration="10"/>',
' <Event id="1" presentationTime="10" duration="10"/>',
' <Event id="2" presentationTime="20" duration="10"/>',
' <Event id="3" presentationTime="30" duration="10"/>',
' <Event id="4" presentationTime="40" duration="10"/>',
' </EventStream>',
' <AdaptationSet mimeType="video/mp4">',
' <Representation bandwidth="1">',
' <SegmentTemplate startNumber="1" media="s$Number$.mp4"',
' duration="2" />',
' </Representation>',
' </AdaptationSet>',
' </Period>',
'</MPD>',
].join('\n');

// 45 seconds after the epoch, which is also 45 seconds after the
// presentation started. Only the last 30 seconds are available, from
// time 15 to 45.
Date.now = () => 45 * 1000;

fakeNetEngine.setResponseText('dummy://foo', manifest);
await parser.start('dummy://foo', playerInterface);

expect(onTimelineRegionAddedSpy).toHaveBeenCalledTimes(4);
expect(onTimelineRegionAddedSpy).toHaveBeenCalledWith(
jasmine.objectContaining({id: '1'}));
expect(onTimelineRegionAddedSpy).toHaveBeenCalledWith(
jasmine.objectContaining({id: '2'}));
expect(onTimelineRegionAddedSpy).toHaveBeenCalledWith(
jasmine.objectContaining({id: '3'}));
expect(onTimelineRegionAddedSpy).toHaveBeenCalledWith(
jasmine.objectContaining({id: '4'}));
});
});

it('honors clockSyncUri for in-progress recordings', async () => {
Expand Down
3 changes: 2 additions & 1 deletion test/dash/dash_parser_manifest_unit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -985,7 +985,8 @@ describe('DashParser Manifest', () => {

it('duplicate Representation ids with live', async () => {
const source = [
'<MPD minBufferTime="PT75S" type="dynamic">',
'<MPD minBufferTime="PT75S" type="dynamic"',
' availabilityStartTime="1970-01-01T00:00:00Z">',
' <Period id="1" duration="PT30S">',
' <AdaptationSet mimeType="video/mp4">',
' <Representation id="1" bandwidth="1">',
Expand Down
32 changes: 31 additions & 1 deletion test/media/region_observer_unit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ describe('RegionObserver', () => {
/** @type {!shaka.media.RegionObserver} */
let observer;

/** @type {!jasmine.Spy} */
let onSeekRange;

/** @type {!jasmine.Spy} */
let onEnterRegion;
/** @type {!jasmine.Spy} */
Expand All @@ -23,12 +26,15 @@ describe('RegionObserver', () => {
let onSkipRegion;

beforeEach(() => {
onSeekRange = jasmine.createSpy('onSeekRange');
onSeekRange.and.returnValue({start: 0, end: 100});

onEnterRegion = jasmine.createSpy('onEnterRegion');
onExitRegion = jasmine.createSpy('onExitRegion');
onSkipRegion = jasmine.createSpy('onSkipRegion');

timeline = new shaka.media.RegionTimeline(
() => { return {start: 0, end: 100}; });
shaka.test.Util.spyFunc(onSeekRange));

observer = new shaka.media.RegionObserver(timeline);
observer.addEventListener('enter', (event) => {
Expand Down Expand Up @@ -318,6 +324,30 @@ describe('RegionObserver', () => {
expect(onSkipRegion).not.toHaveBeenCalled();
});

// See https://github.com/google/shaka-player/issues/3949
it('cleans up references to regions', async () => {
const region = createRegion('my-region', 0, 10);
timeline.addRegion(region);

// Poll so that RegionObserver will store regions.
poll(observer,
/* timeInSeconds= */ 5,
/* seeking= */ false);

// Hack to get a private member without the compiler complaining:
/** @type {Map} */
const regionMap = (/** @type {?} */(observer))['oldPosition_'];
expect(regionMap.size).not.toBe(0);

// Move the seek range so that the region will be removed.
onSeekRange.and.returnValue({start: 20, end: 100});
// Give the timeline time to filter regions
await shaka.test.Util.delay(
shaka.media.RegionTimeline.REGION_FILTER_INTERVAL * 2);

expect(regionMap.size).toBe(0);
});

/**
* @param {string} id
* @param {number} startTimeSeconds
Expand Down
58 changes: 37 additions & 21 deletions test/media/region_timeline_unit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,27 @@ describe('RegionTimeline', () => {
/** @type {!jasmine.Spy} */
let onNewRegion;

/** @type {!jasmine.Spy} */
let onRemoveRegion;

/** @type {!jasmine.Spy} */
let onSeekRange;

beforeEach(() => {
onNewRegion = jasmine.createSpy('onNewRegion');

onSeekRange = jasmine.createSpy('onSeekRange');
onSeekRange.and.returnValue({start: 0, end: 100});

timeline = new shaka.media.RegionTimeline(
shaka.test.Util.spyFunc(onSeekRange));

onNewRegion = jasmine.createSpy('onNewRegion');
timeline.addEventListener('regionadd', (event) => {
shaka.test.Util.spyFunc(onNewRegion)(event['region']);
});

onRemoveRegion = jasmine.createSpy('onRemoveRegion');
timeline.addEventListener('regionremove', (event) => {
shaka.test.Util.spyFunc(onRemoveRegion)(event['region']);
});
});

afterEach(() => {
Expand Down Expand Up @@ -87,24 +94,33 @@ describe('RegionTimeline', () => {
]);
});

it('filters out regions that end before the start of the seek range',
async () => {
onSeekRange.and.returnValue({start: 5, end: 100});
const region1 = createRegion('urn:foo', 'my-region', 0, 3);
const region2 = createRegion('urn:foo', 'my-region', 3, 10);
const region3 = createRegion('urn:foo', 'my-region', 5, 10);
timeline.addRegion(region1);
timeline.addRegion(region2);
timeline.addRegion(region3);
expect(onNewRegion).toHaveBeenCalledTimes(3);
let regions = Array.from(timeline.regions());
expect(regions.length).toBe(3);
// Give the timeline time to filter regions
await shaka.test.Util.delay(
shaka.media.RegionTimeline.REGION_FILTER_INTERVAL * 2);
regions = Array.from(timeline.regions());
expect(regions).toEqual([region2, region3]);
});
it('filters regions that end before the seek range', async () => {
onSeekRange.and.returnValue({start: 5, end: 100});

const region1 = createRegion('urn:foo', 'my-region', 0, 3);
const region2 = createRegion('urn:foo', 'my-region', 3, 10);
const region3 = createRegion('urn:foo', 'my-region', 5, 10);

timeline.addRegion(region1);
timeline.addRegion(region2);
timeline.addRegion(region3);
expect(onNewRegion).toHaveBeenCalledTimes(3);
expect(onNewRegion).toHaveBeenCalledWith(region1);
expect(onNewRegion).toHaveBeenCalledWith(region2);
expect(onNewRegion).toHaveBeenCalledWith(region3);

let regions = Array.from(timeline.regions());
expect(regions.length).toBe(3);

// Give the timeline time to filter regions
await shaka.test.Util.delay(
shaka.media.RegionTimeline.REGION_FILTER_INTERVAL * 2);

regions = Array.from(timeline.regions());
expect(regions).toEqual([region2, region3]);

expect(onRemoveRegion).toHaveBeenCalledWith(region1);
});

/**
* @param {string} schemeIdUri
Expand Down

0 comments on commit b7f04cb

Please sign in to comment.