-
Notifications
You must be signed in to change notification settings - Fork 420
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[YouTube] 8K/HDR/360-degree videos - Add dynamic itag support. #488
base: dev
Are you sure you want to change the base?
Conversation
This obviously requires extensive testing, a lot of hardcoded itags aren't covered but I have never seen them in the wild, do send me such videos here so I can handle those as well. Test APK: app-debug.zip |
You need to add the 'closes' line in the OP for Github to link the issues. Doesn't work in a comment. |
Changed 👍 |
You also need the 'closes' keyword in front of each link. :P |
So after some testing, I found out that HDR webm streams and 8K videos won't play on exoplayer (on my device atleast). Any suggestions on what should be done on the NewPipe side? |
@FireMasterK you could take a look at https://exoplayer.dev/supported-formats.html |
So i tested th apk, on a 4k video with HDR. Now when switching to Webm it works(on all resolutions) but i barely notice the differences. Tried multiple videos and its the same issue. Device: S10+ with linage os(modpunk) |
It looks like this may be required for unsupported devices that don't support AV1 and VP9(.2).
This suggests that software encoding is required for higher resolution. Thoughts? @Stypox |
A while ago it was decided not to include code requiring NDK into NewPipe, idk if this relates |
What's the solution then? I'm not sure what can be done on devices with no hardware decoding.. |
Hiya, the "bad contrast" when playing HDR youtube video thru the debug newpipe posted above could be due to
|
We need to implement something like "supported formats". Pass a List of supported formats to NewPipe when initializing the extractor (empty list => all formats are supported). |
6383c51
to
f32b091
Compare
I'd go with the opposite: let the NewPipe app filter out the unsupported formats |
Sorry @Co0olboi, your suggestion was indentical to what FireMasterK made, and GitHub dosen't allow to hide the comment, so I deleted it before I saw that I can dismiss your review, sorry :(. |
22e1d35
to
f9e05b9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add final
when possible ;-)
extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/ItagItem.java
Outdated
Show resolved
Hide resolved
695b72d
to
64faa3d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, though my opinion on this is not much valuable
Note: There are some changes in NewPipe to be made before merging this. (I need some help with Exoplayer for this) |
extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/ItagItem.java
Outdated
Show resolved
Hide resolved
extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/ItagItem.java
Outdated
Show resolved
Hide resolved
return false; | ||
} | ||
|
||
@Deprecated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A reason for deprecating this/what to use instead in JavaDoc would be great here 😄
I also noticed that
NewPipeExtractor/extractor/src/main/java/org/schabi/newpipe/extractor/utils/DashMpdParser.java
Line 150 in 6db4bea
final ItagItem itag = ItagItem.getItag(Integer.parseInt(id)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #488 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @Stypox
However when I read the code I couldn't link these comments (and the next one who reads it without background information will also fail to do so).
I think it would still be a good idea to somehow mention this in the code/javadoc.
I also tested this together with #706 on an experimental branch with the NewPipe app and got the same problems as stated above (black screen on resolutions >1080 etc) on my emulator - Pixel 3a Android 11.0/API 30, e.g.: Personally I would also currently not recommend implementation:
For now, I'll stay out of this topic as it's too subject-heavy. |
The solution obviously would be to hide resolutions that aren't supported by NewPipe/Exoplayer. I haven't made any changes in the NewPipe app yet, which is essential to even consider merging this. From prior conversations, Stypox said it was decided to not allow NDK/software decoding for unsupported devices, so selectively filtering formats on the app is the only suitable solution. |
Mmmm... Where can I find new testing .apk or sources AND instruction? |
@0Karakurt0 @FireMasterK I rebased here but couldn't push here since I don't have permission. Anyway, here is the apk built upon the latest version of the app and the extractor: app-debug.zip |
Mmm. Idk if I'm lacking hardware, but I belive that on earlier version I did not had that issue. EDIT: |
Another strange issue noticed is that sometimes audio is doubled. I am using Bluetooth headphones and sometimes during the queue change phone's loudspeakers get activated in parallel to headphones. |
Just right now same doubled sound happened in main build, so it's either not NewPipe specific or a bug from mainstream. |
64faa3d
to
f638e56
Compare
2089fe5
to
9091f8a
Compare
9091f8a
to
89491ee
Compare
This comment was marked as spam.
This comment was marked as spam.
89491ee
to
b204560
Compare
@FireMasterK Is there a more recent test APK that we can test? Cheers. |
This PR adds support for dynamic itag loading and therefore 8K/HDR/360-degree videos.
Closes #485
Closes TeamNewPipe/NewPipe#5118
Closes TeamNewPipe/NewPipe#1892
Closes #39
Closes TeamNewPipe/NewPipe#2842
Closes TeamNewPipe/NewPipe#2835
Closes #672.