Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Fix memory leak in DASH live streams with inband EventStream #3957

Merged
merged 1 commit into from Feb 16, 2022
Merged

fix: Fix memory leak in DASH live streams with inband EventStream #3957

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
50 changes: 31 additions & 19 deletions lib/dash/dash_parser.js
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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">',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test was broken before. You can't have a dynamic manifest without a start time. However, the code in DashParser hid this mistake, because it didn't set the PresentationTimeline to dynamic until after all the periods had been parsed.

I moved up the code that sets properties on PresentationTimeline, so that the correct values for start time and isLive() are available during parsing. That caused this test to fail, however, due to a failed assertion in PresentationTimeline. Making the MPD in this test more realistic fixes that previously-hidden issue.

' <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
Open in desktop
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
Open in desktop
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