Simplify MetaPlaylist Manifest loading by relying on the same code than other transports#1487
Open
peaBerberian wants to merge 1 commit intodevfrom
Open
Simplify MetaPlaylist Manifest loading by relying on the same code than other transports#1487peaBerberian wants to merge 1 commit intodevfrom
peaBerberian wants to merge 1 commit intodevfrom
Conversation
83a64bb to
48dda70
Compare
4203867 to
e3c71ee
Compare
e3c71ee to
3db7b58
Compare
3db7b58 to
bbb90d4
Compare
bbb90d4 to
889105b
Compare
00fc806 to
b7216b4
Compare
889105b to
673f56c
Compare
cd1b1e5 to
4ed3fce
Compare
4ed3fce to
a5fb237
Compare
a5fb237 to
c358202
Compare
c358202 to
f83bdb1
Compare
529d46c to
6d4fed2
Compare
f83bdb1 to
a2957f6
Compare
|
✅ Automated performance checks have passed on commit DetailsPerformance tests 1st run outputNo significative change in performance for tests:
|
a2957f6 to
b192a50
Compare
|
✅ Automated performance checks have passed on commit DetailsPerformance tests 1st run outputNo significative change in performance for tests:
|
6cfd206 to
1e55170
Compare
a42734c to
29372ac
Compare
…an other transports
I was PoCing protocol-detection when initially parsing the Manifest, so
application developers wouldn't e.g. have to tell to us that they want
to play "dash".
While doing that, I noticed that all transports had close to the same
logic for Manifest fetching. Only differences I've seen are:
- DASH may add integrity checks if the `checkManifestIntegrity` option
is set
- DASH may ask the browser to obtain the response as an ArrayBuffer
(and not as a default JS string) if it is probable that we will rely
on the WASM MPD parser
- the `local` transport throws if no custom `manifestLoader` is
declared.
As such, those differences look minor and straightforward enough so the
corresponding logic can be factorized into e.g. a single parameterized
`createManifestLoader` function.
This was already done for DASH and Smooth, but I don't know why we
didn't use it for the experimental MetaPlaylist feature. This commit
fixes this.
We could also use it for the experimental `local` transport in the
future, though I'm unsure of how we could generically make sense of its
peculiar rule (no real loading).
b192a50 to
8da4db3
Compare
|
✅ Automated performance checks have passed on commit DetailsPerformance tests 1st run outputNo significative change in performance for tests:
|
d4be192 to
9ad6758
Compare
0142e34 to
1fd9df3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I was PoCing protocol-detection when initially fetching the Manifest, so application developers wouldn't e.g. have to tell to us that they want to play "dash".
While doing that, I noticed that all transports had close to the same logic for Manifest fetching. Only differences I've seen are:
DASH may add integrity checks if the
checkManifestIntegrityoption is setDASH may ask the browser to obtain the response as an ArrayBuffer (and not as a default JS string) if it is probable that we will rely on the WASM MPD parser
the
localtransport throws if no custommanifestLoaderis declared.As such, those differences look minor and straightforward enough so the corresponding logic can be factorized into e.g. a single parameterized
createManifestLoaderfunction.This was already done for DASH and Smooth, but I don't know why we didn't use it for the experimental MetaPlaylist feature. This commit fixes this.
We could also use it for the experimental
localtransport in the future, though I'm unsure of how we could generically make sense of its peculiar rule (no real loading).