Properly support other delivery methods#367
Properly support other delivery methods#367wb9688 wants to merge 2 commits intoTeamNewPipe:devfrom wb9688:delivery-methods
Conversation
|
Is anyone able to review this? |
Stypox
left a comment
There was a problem hiding this comment.
Looks good to me, thank you for looking into this ;-)
The code mostly comes from #358 Co-Authored-By: Mauricio Colli <mauriciocolli@outlook.com>
|
@Stypox: Could you review this again? This PR shouldn't be merged yet though, as I haven't properly tested e.g. whether the YouTube DASH extraction is actually correct (the output looks correct though). Btw I just started working on the NewPipe part of this. |
Stypox
left a comment
There was a problem hiding this comment.
LGTM, thank you :-D
I only pointed out two small things, after they are solved and you have finished with your tests this can be merged imo
| if (fmt != null && languageCode != null) | ||
| subtitles.add(new SubtitlesStream(fmt, languageCode, url, false)); | ||
| if (fmt != null && languageCode != null) { | ||
| final String id = languageCode + "." + fmt.suffix; |
There was a problem hiding this comment.
Make a constructor of SubtitlesStream without id, so that it can be automatically set to this. I see this is being used also in YoutubeStreamExtractor
| } else if (t.getString("preset").contains("opus")) { | ||
| mediaFormat = MediaFormat.OPUS; | ||
| } else { | ||
| break; |
There was a problem hiding this comment.
Why break and not continue? Also below.
compatible with the changed API.
Changes
DeliveryFormatenumurlinStreamtocontentand addisUrlto indicate whethercontentcontains the URL or the file itself (which is useful when we have to split DASH/HLS manifests)idinStreamto identify a specific stream (there could be multipleStreams with the same ID if the same stream is offered through multiple delivery methods; in YouTube's case the ID would be the itag)When all changes are finished, I'm going to work on implementing them in NewPipe's downloader. When I'm done with that and the unified player has been merged, I'll look at implementing it in the player as well.
I'll leave not hardcoding itags to another PR.