Add support for YouTube Music search #291
Conversation
|
Already talked about it in irc, channel are unusuable from youtube music search : no streams, no subscribers count, there are only banner and profile picture |
wb9688
added
enhancement
|
This PR is 100% finished now. @TobiGr: Could this be included in v0.19.0? It's not really a big change. |
...in/java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeSearchExtractor.java
Outdated
Show resolved
Hide resolved
...va/org/schabi/newpipe/extractor/services/youtube/search/YoutubeSearchExtractorMusicTest.java
Outdated
Show resolved
Hide resolved
| Map<String, List<String>> headers = new HashMap<>(); | ||
| headers.put("X-YouTube-Client-Name", Collections.singletonList(youtubeMusicKeys[1])); | ||
| headers.put("X-YouTube-Client-Version", Collections.singletonList(youtubeMusicKeys[2])); | ||
| headers.put("Origin", Collections.singletonList("https://music.youtube.com")); |
There was a problem hiding this comment.
As discussed, the default http client from java has some restrictions about request headers.
I think using another client for tests is the right choice (like you did earlier, OkHttp is a good one).
Should we do it in its own pull request? The front end is already using it, don't think too many problems would be a result of this, but then we can test thoroughly what kind of effects it will have.
There was a problem hiding this comment.
This should indeed be a different PR. I think OkHttp should be used, so we have the exact same behavior in the tests as in NewPipe.
...in/java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeSearchExtractor.java
Outdated
Show resolved
Hide resolved
...in/java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeSearchExtractor.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
I've noticed quite a bit of these null pointer exceptions, mainly because it's hard/verbose to check every selection made with the library we use right now (which leads to try-catch hell, null checks hell too).
Maybe we should try something equivalent to jq (JSON processor) for our value selections as well?
I should open a separate issue about it...
...in/java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeSearchExtractor.java
Outdated
Show resolved
Hide resolved
...in/java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeSearchExtractor.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
In a second look, I think it makes sense to create a separate class specifically for the music version, as you already did in the tests.
In my opinion, having isMusicSearch in every method just unnecessarily complicate things, as it could be done only once when StreamingService#getSearchExtractor is called to decide which extractor will be used.
...va/org/schabi/newpipe/extractor/services/youtube/search/YoutubeMusicSearchExtractorTest.java
Show resolved
Hide resolved
@mauriciocolli: Like we're doing with the mixes? I agree and I'll fix that today. |
...va/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeMusicSearchExtractor.java
Outdated
Show resolved
Hide resolved
...va/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeMusicSearchExtractor.java
Outdated
Show resolved
Hide resolved
wb9688
dismissed
mauriciocolli’s stale review
I pushed new changes that fixes what he pointed out
...ain/java/org/schabi/newpipe/extractor/services/youtube/linkHandler/YoutubeParsingHelper.java
Outdated
Show resolved
Hide resolved
I have to do a bit of cleaning up, make
getYoutubeMusicKeys()more reliable and add tests. The problem with adding tests is thatDownloaderTestImplsomehow behaves weirdly, which causes it to not work. The code works fine with OkHttp on Android in NewPipe.