Skip to content
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

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

FireMasterK
Copy link
Member

@FireMasterK FireMasterK commented Dec 19, 2020

  • I carefully read the contribution guidelines and agree to them.
  • I have tested the API against NewPipe.
  • I agree to create a pull request for NewPipe as soon as possible to make it compatible with the changed API.

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.

@FireMasterK
Copy link
Member Author

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

@opusforlife2
Copy link
Collaborator

You need to add the 'closes' line in the OP for Github to link the issues. Doesn't work in a comment.

@FireMasterK
Copy link
Member Author

You need to add the 'closes' line in the OP for Github to link the issues. Doesn't work in a comment.

Changed 👍

@opusforlife2
Copy link
Collaborator

You also need the 'closes' keyword in front of each link. :P

@FireMasterK
Copy link
Member Author

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?

@Stypox
Copy link
Member

Stypox commented Dec 20, 2020

@FireMasterK you could take a look at https://exoplayer.dev/supported-formats.html

@FarisZR
Copy link

FarisZR commented Dec 20, 2020

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?

So i tested th apk, on a 4k video with HDR.
The video player crashes when trying to play any resolution higher than 1080p with HDR(mpeg).
And when playing in 1080 mpeg-4 HDR, its playing in like 5fps with everything kind of grey.

Now when switching to Webm it works(on all resolutions) but i barely notice the differences. Tried multiple videos and its the same issue.
The most noticeable difference is the contrast/saturation
When selecting HDR the video is noticeably desaturated

Device: S10+ with linage os(modpunk)

@FireMasterK
Copy link
Member Author

In addition to Android’s platform decoders, ExoPlayer can also make use of software decoder extensions. These must be manually built and included in projects that wish to make use of them. We currently provide software decoder extensions for AV1, VP9, FLAC, Opus and FFmpeg.

It looks like this may be required for unsupported devices that don't support AV1 and VP9(.2).

Device implementations must support dynamic video resolution and frame rate switching through the standard Android APIs within the same stream for all VP8, VP9, H.264, and H.265 codecs in real time and up to the maximum resolution supported by each codec on the device. src: https://developer.android.com/guide/topics/media/media-formats#core

This suggests that software encoding is required for higher resolution.

Thoughts? @Stypox
Is there something I missed?

@Stypox
Copy link
Member

Stypox commented Dec 22, 2020

A while ago it was decided not to include code requiring NDK into NewPipe, idk if this relates

@FireMasterK
Copy link
Member Author

What's the solution then? I'm not sure what can be done on devices with no hardware decoding..

@orangpelupa
Copy link

Hiya, the "bad contrast" when playing HDR youtube video thru the debug newpipe posted above could be due to

  1. It displays HDR to SDR directly, without proper tone mapping. Thus it looks to have a bad contrast. On the other hand if you play a HDR video thru 3rd party player (VLC, mx player, etc) it'll be displayed in SDR with proper tone mapping. So you loss its "white sky is brighter than a white paper" but the contrast and colors looks fine.
  2. It did not switch the display mode from SDR to HDR, thus it was displayed in SDR. You can easily check this by swiping down the notification bar or opening the recent menu. In HDR mode (invoked by playing an HDR video in a stock Samsung video player), those overlays/screens will have wonky colors.

@TobiGr
Copy link
Member

TobiGr commented Jan 5, 2021

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).

@AudricV AudricV changed the title 8K/HDR/360-degree videos - Add dynamic itag support. [YouTube] 8K/HDR/360-degree videos - Add dynamic itag support. Mar 5, 2021
@AudricV AudricV added enhancement youtube service, https://www.youtube.com/ labels Mar 5, 2021
@Stypox
Copy link
Member

Stypox commented Mar 19, 2021

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).

I'd go with the opposite: let the NewPipe app filter out the unsupported formats

@TeamNewPipe TeamNewPipe deleted a comment from HxYxI Apr 16, 2021
@TeamNewPipe TeamNewPipe deleted a comment from HxYxI Apr 16, 2021
@AudricV
Copy link
Member

AudricV commented Apr 17, 2021

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 :(.

@FireMasterK FireMasterK force-pushed the dynamic-itag branch 2 times, most recently from 22e1d35 to f9e05b9 Compare August 11, 2021 12:46
@FireMasterK FireMasterK marked this pull request as ready for review August 12, 2021 06:45
Copy link
Member

@Stypox Stypox left a 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 ;-)

Copy link
Member

@Stypox Stypox left a 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

@FireMasterK
Copy link
Member Author

Note: There are some changes in NewPipe to be made before merging this. (I need some help with Exoplayer for this)

return false;
}

@Deprecated
Copy link
Member

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

final ItagItem itag = ItagItem.getItag(Integer.parseInt(id));
still relies on that code, so maybe some fixing has to be done there

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

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.

@litetex
Copy link
Member

litetex commented Aug 24, 2021

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.:
grafik

Personally I would also currently not recommend implementation:

  • <10% of all android devices even support AV1, which is optionally (decoder) shipped with Android 10+
    grafik
    grafik
  • there could be certain changes from YT's side - this PR is now open since about 9 months; in likely 2-4 years we will see a noteworthy market share of Android 10+ with >20-40% - that may render it useless and it has to be (completely) reworked again

For now, I'll stay out of this topic as it's too subject-heavy.

@FireMasterK
Copy link
Member Author

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.

@0Karakurt0
Copy link

Mmmm... Where can I find new testing .apk or sources AND instruction?
One in the beginning of the thread is quite outdated and can't load videos fast enough to viev HDR

@Stypox
Copy link
Member

Stypox commented Nov 21, 2021

@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

@0Karakurt0
Copy link

0Karakurt0 commented Nov 22, 2021

here is the apk built upon the latest version of the app

Mmm. Idk if I'm lacking hardware, but I belive that on earlier version I did not had that issue.
I am expiriencing frame skipping and video slowdowns on 1080p HDR. 720p HDR is okay.
Hardware is Mi 9T, I'll write later if the same issue is present on earlier build. And add more testing overall, tho I gave no idea what information would be helpful in solving this.
May the OS have a part in preformance penalty? I'm running DoT OS on my phone

EDIT:
So, I did a bit more testing, and found out some interesting results. My hardware is laking for 1080+ HDR or MPEG-4 720p60 HDR, but some videos where played correctly on that resolution (MPEG-4 1080p HDR too). Selecting any resolution higher (MPEG-4) leads to critical player error.
WebM had no issues at all

@0Karakurt0
Copy link

0Karakurt0 commented Nov 22, 2021

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.
But I'm not sure if it's New Pipe's fault, because if takes effect on the whole system and fixes only after reboot.
New Pipe debug edition is the only app causing that behavior

@0Karakurt0
Copy link

Just right now same doubled sound happened in main build, so it's either not NewPipe specific or a bug from mainstream.
What it had in common was HDR, I was watching downloaded HDR video in Just Player right before opening a video in NewPipe.

@ag2s20150909

This comment was marked as spam.

@Valmar33
Copy link

@FireMasterK Is there a more recent test APK that we can test? Cheers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement youtube service, https://www.youtube.com/
Projects
None yet