Skip to content

dash: More spec-compliant handling of Location elements - #1880

Open
peaBerberian wants to merge 4 commits into
devfrom
better-location-handling
Open

dash: More spec-compliant handling of Location elements#1880
peaBerberian wants to merge 4 commits into
devfrom
better-location-handling

Conversation

@peaBerberian

@peaBerberian peaBerberian commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

While POCing supplementary DASH features, I saw that the way we handled <Location> elements in the MPD, indicating where to refresh the MPD, was not proper.

Basically the DASH spec says (re-formulating):

For a manifest refresh the URL to do so should be:

  • The original MPD's URL if no <Location> element is present in the MPD
  • The URL indicated by the <Location> elements if present

We always used the original URL (used to make the request) in priority. We should not: if any <Location> is present, those are the new preferred URLs.

This commit fixes that.

It also does a few supplementary things:

  • At the Manifest class level: renamed uris into refreshUrls, as URI was too generic.

    It may now be confused with updateUrl though which is our concept for "an URL only useful for updates" (e.g. it can be shorter than the actual MPD, just containing the new data) where refreshUrls are all URLs to get the full version again. To see if it is too confusing - for now comments do, sadly, the work.

  • <Location> is now properly parsed as an Element, not as if it was an XML attribute like before

  • Like for BaseURL elements, I now also parse the optional serviceLocation attribute, though I do not exploit it for now outside the parsers. This will become mainly useful for content steering.

  • Previously, an empty BaseURL was ignored. Looking closely at DASH spec, it doesn't seem that any special care should be taken for it, it should be resolved like the other ones.

    An empty BaseURL makes sense for when you want to refer to the current inherited URL (itself starting from the MPD url), probably to classify it by e.g. also adding a serviceLocation attribute - an attribute used in content steering.

    However for that case, I saw that both dash.js and the shaka-player mishandle it, like we did before. I nonetheless added here as the risk is close to 0 and it's more spec-compliant.

  • An empty or relative <Location> had also a few edge cases: mainly it resolved to "" when the MPD Url was not known (should probably be skipped here).

    Now it is properly skipped with a warning

@peaBerberian
peaBerberian force-pushed the better-location-handling branch 4 times, most recently from 58f6e56 to 4218669 Compare August 14, 2026 16:51
@peaBerberian peaBerberian added Priority: 2 (Medium) This issue or PR has a medium priority. waiting-for-review labels Aug 26, 2026
While POCing supplementary DASH features, I saw that the way we handled
`<Location>` elements in the MPD, indicating where to refresh the URL,
was not proper.

Basically they said (re-formulating):
> For a manifest refresh the URL to do so should be:
> - The original MPD's URL if no `<Location>` element is present in the
    MPD
> - The URL indicated by the `<Location>` elements if present

We always prepended to those the URL used to make the requests, in terms of
importance. We should not: if any `<Location>` is present, it is the new
authority to indicate where the MPD should be refreshed.

This commit fixes that.

It also does a few supplementary things:
- At the `Manifest` class level: renamed `uris` into `refreshUrls`
- `<Location>` is now properly parsed as an Element, not as if it was an
  XML attribute like before
- Like for segment URLs, I now also parse the `serviceLocation`, though
I do not exploit it for now outside the parsers. This will become mainly
useful for content steering.
I looked at the spec, nothing prevent a BaseURL from not indicating any
URL. It could even make sense to refer to the original URL (e.g.
associated to a `serviceLocation`).

This is however a edge case that both dash.js and shaka-player
seems to mishandle.
@peaBerberian
peaBerberian force-pushed the better-location-handling branch from eef3e50 to 99f72b8 Compare September 1, 2026 09:43
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

✅ Automated performance checks have passed on commit fc79cb45a59375910d30674fefcd281d2604c509 with the base branch dev.

Details

Performance tests 1st run output

No significative change in performance for tests:

Name Mean Median
loading 38.10ms -> 36.42ms (corrected: 1.728ms, A/A bias: -0.045ms, z: 0.45226) 29.20ms -> 29.20ms (corrected: -0.175ms, A/A bias: 0.075ms, z: 0.00962)
seeking 175.85ms -> 178.35ms (corrected: -30.034ms, A/A bias: 27.531ms, z: 0.63509) 11.00ms -> 11.00ms (corrected: -0.250ms, A/A bias: 0.275ms, z: 0.62546)
audio-track-reload 30.87ms -> 31.18ms (corrected: 0.101ms, A/A bias: -0.410ms, z: 0.15396) 29.60ms -> 29.65ms (corrected: 0.250ms, A/A bias: -0.275ms, z: 0.59178)
loading over active content 21.91ms -> 22.13ms (corrected: 0.295ms, A/A bias: -0.522ms, z: 0.73612) 20.20ms -> 20.65ms (corrected: 0.650ms, A/A bias: -0.250ms, z: 0.73612)
large multi-period manifest 91.83ms -> 92.58ms (corrected: -0.530ms, A/A bias: -0.227ms, z: 0.03849) 84.20ms -> 82.70ms (corrected: -0.275ms, A/A bias: 2.475ms, z: 0.03849)
cold loading multithread 52.13ms -> 53.85ms (corrected: -1.466ms, A/A bias: -0.255ms, z: 1.33753) 50.30ms -> 49.55ms (corrected: 0.825ms, A/A bias: -0.050ms, z: 4.40230)
seeking multithread 173.34ms -> 193.24ms (corrected: -44.855ms, A/A bias: 24.947ms, z: 0.64471) 13.20ms -> 13.25ms (corrected: -0.450ms, A/A bias: 0.275ms, z: 0.92376)
audio-track-reload multithread 29.88ms -> 29.75ms (corrected: 0.160ms, A/A bias: -0.029ms, z: 0.44264) 28.80ms -> 28.65ms (corrected: 0.475ms, A/A bias: -0.375ms, z: 0.96225)
hot loading multithread 20.74ms -> 20.40ms (corrected: 2.973ms, A/A bias: -2.637ms, z: 1.03923) 19.60ms -> 19.50ms (corrected: 0.075ms, A/A bias: -0.025ms, z: 0.73612)

@canalplus canalplus deleted a comment from github-actions Bot Sep 1, 2026
@canalplus canalplus deleted a comment from github-actions Bot Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority: 2 (Medium) This issue or PR has a medium priority. waiting-for-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant