Skip to content

feat(toDash): Add option to include thumbnails in the manifest#446

Merged
LuanRT merged 2 commits intoLuanRT:mainfrom
absidue:dash-storyboards
Jul 18, 2023
Merged

feat(toDash): Add option to include thumbnails in the manifest#446
LuanRT merged 2 commits intoLuanRT:mainfrom
absidue:dash-storyboards

Conversation

@absidue
Copy link
Collaborator

@absidue absidue commented Jul 16, 2023

The DASH spec allows manifests to provide thumbnails, the most common use case for them is showing a preview when seeking with the seekbar. Players like shaka and DASH.js don't have a built-in UI for them, but they do parse them and provide APIs to get the thumbnail for a given timestamp, Player#getThumbnails and MediaPlayer#provideThumbnail respectively.

I decided to make this feature opt-in with a setting for the toDash function, as it requires making a few HEAD requests to get the content length, to approximate the bandwidth for the storyboards and it's not supported by all players/they don't provide a built-in UI for them. Sticking it behind an option allows people that can and want to use it, to use it without slowing the manifest generation down for everyone else.

I will create a separate pull request after this is merged (to avoid merge conflicts), to add captions/subtitles to the manifests, those require making requests too and it doesn't make sense to include both WebVTT and TTML captions, if you know your player only supports one of them.

@absidue absidue changed the title feat(toDash): Add option to include storyboards in the manifest feat(toDash): Add option to include thumbnails in the manifest Jul 16, 2023
@absidue absidue force-pushed the dash-storyboards branch from 7b9b015 to ff72f7d Compare July 16, 2023 21:15
@LuanRT
Copy link
Owner

LuanRT commented Jul 16, 2023

Haven't reviewed this completely yet but the toDash fn implementation should be go in MediaInfo so we can avoid all that duplicate code (plus the burden of having to modify all of them if say, we add another parameter).

async toDash(url_transformer?: URLTransformer, format_filter?: FormatFilter): Promise<string> {
return FormatUtils.toDash(this.streaming_data, url_transformer, format_filter, this.#cpn, this.#actions.session.player, this.#actions);
}

@absidue
Copy link
Collaborator Author

absidue commented Jul 16, 2023

Unfortunately YouTube Kids doesn't provide any storyboads, so the storyboards properties are on the YouTube Music TrackInfo and YouTube VideoInfo classes, so we can't really move that up into MediaInfo as the properties it needs to read don't exist there. (well we could probably do it in pure javascript, as the properties would exist at runtime, but typescript doesn't like that kind of stuff).

@absidue
Copy link
Collaborator Author

absidue commented Jul 16, 2023

We'll have the same issue with captions, as both YouTube and YouTube Kids provide them but YouTube music doesn't, at least not on the TrackInfo object.

@LuanRT
Copy link
Owner

LuanRT commented Jul 16, 2023

Yes I am aware, but can't we just throw an error (or even better, just ignore it and generate the manifest anyway) if someone tries using MediaInfo#toDash with that option set to true but the required data doesn't exist? At least we'd get to keep the function signature the same across all VideoInfo-like classes.

I really doubt anybody is going to use that option by accident, unless they're mixing youtube+youtube music+youtube kids.

@LuanRT
Copy link
Owner

LuanRT commented Jul 16, 2023

Also, you do have access to MediaInfo#page, which contains the parsed response. Both /player and /next responses actually. So I think it should be easy to check whether the required data exists.

@github-actions github-actions bot added the core label Jul 17, 2023
@absidue
Copy link
Collaborator Author

absidue commented Jul 17, 2023

Move the toDash implementation to just be in the MediaInfo class again, thanks for the suggestion!

@LuanRT
Copy link
Owner

LuanRT commented Jul 18, 2023

Perfect. Awesome work, as always! Thank you.

@LuanRT LuanRT merged commit 1a03473 into LuanRT:main Jul 18, 2023
@absidue absidue deleted the dash-storyboards branch July 18, 2023 05:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants