feat(DASH): Add period caching to speed up manifest parsing by grabofus · Pull Request #9353 · shaka-project/shaka-player

grabofus · 2025-11-04T17:16:00Z

Adds period caching to speed up manifest parsing. The aim of this feature is to improve parsing of length multi-period DASH manifests on low power devices.

shaka-bot · 2025-11-04T18:14:37Z

Incremental code coverage: 52.21%

avelad · 2025-11-05T10:52:48Z

Why create a configuration and not always enable it?

lib/dash/dash_parser.js

tykus160 · 2025-11-05T11:56:14Z

@grabofus thanks for your PR, finding out serious bottleneck and trying to find a way to overcome it! Your change looks OK in general, however let me share another idea I have about it.

Personally I don't think we should maintain such a cache. Preferably, I think parser should be a bit smarter and work on some assumptions. The overall expectation in live updates should be that either some data has been removed from the beginning of a file, or added at the end of it. These assumptions are already used silently in other parts of code in shaka, i.e. SegmentIndex, PeriodCombiner or MPD Patch implementation. In my opinion live updates logic should leverage based on those assumptions.

Due to my thoughts above, I'm also not sure would we need to calculate hash in a way introduced here, as it's basically DASH agnostic. In doing such live updates we should instead check period characteristics, in particular did any timeline has changed in starting or ending periods.

Another idea would be to lazy parse periods, but that's definitely not in scope of this work.

That's my general thought that I wanted to share with community. If we decide this approach is a better way to go, I will of course review it through-and-through.

cc @shaka-project/shaka-player

Murmur · 2025-11-06T13:21:12Z

The overall expectation in live updates should be that either some data has been removed from the beginning of a file, or added at the end of it. | we should instead check period characteristics, in particular did any timeline has changed in starting or ending periods.

Yes and I think DASH specs has a text saying "middle periods" should/shall not be modified after introduced in a live streaming.

First period (oldest): expiration of old segments in a timeline ok (make a period shorter)
last period (latest): append new segments in a timeline is ok (make a period longer)

There might be live streams available that may change "middle periods" adhoc and a specific app players may cope it well.
Maube OP has test streams for a reason of "personalized advert/dash event" inserter logic?

joeyparrish · 2025-11-07T20:45:59Z

lib/dash/dash_parser.js

    const periods = [];
    let prevEnd = seekRangeStart;
    const periodNodes = TXml.findChildren(mpd, 'Period');
+    const enablePeriodCaching = context.dynamic && !!(this.config_ && this.config_.dash &&


Small nit: I don't see why !! is needed here.

joeyparrish · 2025-11-07T20:47:27Z

lib/dash/dash_parser.js

+            this.periodCache_.get(cacheKey) : null;
+        let reusePeriod = false;
+        if (cachedEntry) {
+          signature = this.computePeriodSignature_(elem);


It's unclear to me why we need both a period cache key and a separate signature.

Let me see if I understand...

We need the key to be cheap, but the signature is more expensive? And we might find a key-match with a different signature, and then we have to treat it as a cache miss?

Are there any cases where we don't have to compute a signature for each and every period? If not, couldn't we move lift the signature computation instead of repeating it in two places?

The cache key won't change for the period, but it's signature can if its content is modified. So the idea was to be able to quickly find a cached entry then validate if it had any changes by comparing signatures.

joeyparrish · 2025-11-07T20:49:49Z

lib/dash/dash_parser.js

-      if (context.period.id && periodDuration) {
-        this.periodDurations_.set(context.period.id, periodDuration);
+      if (enablePeriodCaching && period.id && signature) {
+        this.periodCache_.set(period.id, {signature, period});


Here you set into periodCache_ by period.id. Above, you get into periodCache_ by the return of getPeriodCacheKey_(). Why wouldn't you just use getPeriodCacheKey_() in all cases?

Missed this one 👍

joeyparrish · 2025-11-07T20:50:49Z

lib/dash/dash_parser.js

+   * @return {number}
+   * @private
+   */
+  fnv1a_(str, seed) {


I would like to see these (fnv1a_ and hashTXml_) moved to a utility class. It's distracting here in the DASH parser, and also not testable as a private method.

Moved them into a HashUtils class and added some unit tests 👍

joeyparrish · 2025-11-07T20:53:23Z

lib/dash/dash_parser.js

+   * @private
+   */
+  hashTXml_(node, seed = 0x811c9dc5) {
+    let hash = this.fnv1a_(node.tagName, seed >>> 0);


What is the purpose of >>> 0?

That part was copied from somewhere, I don't know an awful lot about hashing functions so I left that in, but I don't think it makes much difference if we remove it.

avelad · 2025-12-15T19:46:03Z

@grabofus can you review the previous comments? Thanks!

grabofus · 2025-12-18T17:01:11Z

@avelad addressed the comments - sorry for the delay haven't had much time lately to concentrate on this

Why create a configuration and not always enable it?

Unsure if it benefits everyone, or if it could have side effects in some streaming configurations (e.g. if longer SegmentTimelines are used) so I didn't enable it by default. Happy to change it if you think it should be enabled.

Personally I don't think we should maintain such a cache. Preferably, I think parser should be a bit smarter and work on some assumptions. The overall expectation in live updates should be that either some data has been removed from the beginning of a file, or added at the end of it. These assumptions are already used silently in other parts of code in shaka, i.e. SegmentIndex, PeriodCombiner or MPD Patch implementation. In my opinion live updates logic should leverage based on those assumptions.

Due to my thoughts above, I'm also not sure would we need to calculate hash in a way introduced here, as it's basically DASH agnostic. In doing such live updates we should instead check period characteristics, in particular did any timeline has changed in starting or ending periods.

I was not sure initially if we can rely on middle periods not changing, but this makes perfect sense and is a more performant way to solve this issue.

joeyparrish · 2026-01-22T17:17:37Z

Personally I don't think we should maintain such a cache. Preferably, I think parser should be a bit smarter and work on some assumptions. The overall expectation in live updates should be that either some data has been removed from the beginning of a file, or added at the end of it. These assumptions are already used silently in other parts of code

I also don't love the hashing & caching, and would prefer something that uses these assumptions.

I was not sure initially if we can rely on middle periods not changing, but this makes perfect sense and is a more performant way to solve this issue.

Are we going in this direction instead? If so, who will do it? @avelad, @grabofus

Thanks!

grabofus added 6 commits November 4, 2025 16:16

feat: period caching

49f2e17

refactor: compute period hash for signature

50278bd

chore: cleanup

a332cad

chore: lint

a9df86f

fix: signature type

9e2b534

fix: read id from period

5f0583a

avelad requested a review from tykus160 November 5, 2025 10:52

avelad added type: enhancement New feature or request priority: P3 Useful but not urgent component: DASH The issue involves the MPEG DASH manifest format labels Nov 5, 2025

avelad added this to the v5.0 milestone Nov 5, 2025

avelad reviewed Nov 5, 2025

View reviewed changes

lib/dash/dash_parser.js Outdated Show resolved Hide resolved

fix: disable period caching for static manifests

6a7b4c7

joeyparrish requested changes Nov 7, 2025

View reviewed changes

grabofus and others added 2 commits December 18, 2025 16:54

create hash utils

b6f136c

Merge branch 'main' into feat/period-cache

64352f8

grabofus marked this pull request as ready for review December 18, 2025 16:56

grabofus requested a review from joeyparrish December 18, 2025 17:01

avelad modified the milestones: v5.0, v5.1 Feb 6, 2026

Conversation

grabofus commented Nov 4, 2025

Uh oh!

shaka-bot commented Nov 4, 2025

Uh oh!

avelad commented Nov 5, 2025

Uh oh!

Uh oh!

tykus160 commented Nov 5, 2025

Uh oh!

Murmur commented Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avelad commented Dec 15, 2025

Uh oh!

grabofus commented Dec 18, 2025

Uh oh!

joeyparrish commented Jan 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Murmur commented Nov 6, 2025 •

edited

Loading