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.

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

Cues not discarded across attachSource calls leading to increased JS heap usage #4427

Open
vodlogic opened this issue Mar 19, 2024 · 3 comments

Comments

@vodlogic
Copy link
Contributor

vodlogic commented Mar 19, 2024

We have observed JS Heap increasing at a rate of approximately 1MB every 3 minutes when a new manifest is loaded when player.attachSource(<new manifest URL>) is called every 15 seconds when subtitles are enabled. We observe this in dash.js 4.7.4 and are yet to try earlier versions of dash.js

When subtitles are disabled, JS heap increases only a couple of MB per hour when attachSource() is called every 15 seconds.

This leads us to hypothesise that cues are not being properly cleaned up when a new manifest is loaded by the player.

We can see that the number of cues continues to grow across calls to attachSource()

document.querySelector('video').textTracks[0].cues.length

The fix merged in #4305 resolved a memory leak when soak testing a stream for 24 hours without calling attachSource()

Not sure if this is related, but we also wanted to check the logic that has been implemented for the previous PR to discard outdated cues.

https://github.com/Dash-Industry-Forum/dash.js/pull/4305/files#diff-033321ac6c729cfa349f6ca06067679dc6919fe8fe1023ec6fd990c83e98ac9aR829

Should the expression be return cue.startTime <= currentTime && cue.endTime >= currentTime to determine if cue is active?

@vodlogic vodlogic added the Bug label Mar 19, 2024
@nigelmegitt
Copy link
Contributor

Should the expression be return cue.startTime <= currentTime && cue.endTime >= currentTime to determine if cue is active?

I think the idea is not to discard future cues that are likely to be needed soon.

@dsilhavy
Copy link
Collaborator

@vodlogic Can you share a stream to reproduce this? Typically, deleteAllTextTracks is called once a new source is attached. I checked in v5.0.0, and I don't see the number of cues increasing when calling attachSource multiple times with https://dash.akamaized.net/akamai/test/caption_test/ElephantsDream/elephants_dream_480p_heaac5_1_https.mpd

isCueActive also seems to wrong to me. Instead, it should be like this:

    function _isCueActive(cue) {
        const currentTime = videoModel.getTime();

        return currentTime >= cue.startTime && currentTime <= cue.endTime
    }

@nielar0
Copy link

nielar0 commented Apr 26, 2024

@dsilhavy issue is visible also with https://dash.akamaized.net/akamai/test/caption_test/ElephantsDream/elephants_dream_480p_heaac5_1_https.mpd
Prerequisites:

  • stream with enabled subtitles
  • subtitles should be shown before call attachSource() again

So for this stream you need to wait 30s (subtitles are from around 20s after start).

I tested also the development branch and it looks the same so v5.0.0 is also affected.

There is a minimal test scenario where heap is slowly growing:

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <script src="http://cdn.dashjs.org/v4.7.4/dash.all.min.js"></script>
</head>
<body>
    <div>
        <video id="videoPlayer" controls></video>
        <div id='sub'/>
    </div>

    <script>

        const sleep = s => new Promise(r => setTimeout(r, s * 1000));

        const videoElement = document.getElementById('videoPlayer');
        const subElement = document.getElementById("sub");

        // Initialize the Dash.js player
        const player = dashjs.MediaPlayer().create();
 
        // Setup the player
        player.initialize(videoElement);
        player.attachTTMLRenderingDiv(subElement);

        // Set defaultEnabled: true to enable subtitles
        player.updateSettings({streaming: {text: {defaultEnabled: true}}});


        player.setInitialMediaSettingsFor('text', {
                    lang: 'eng',
        });

        async function test() {
            while (true) {
                player.attachSource('https://dash.akamaized.net/akamai/test/caption_test/ElephantsDream/elephants_dream_480p_heaac5_1_https.mpd');
                await sleep(30);
            }
        }

        test();

    </script>
</body>
</html>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

4 participants