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

Add support for delivery methods other than progressive HTTP (in the player only) #6537

Closed
wants to merge 41 commits into from

Conversation

AudricV
Copy link
Member

@AudricV AudricV commented Jun 20, 2021

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

This PR adds support of YouTube OTF streams and the full support of ended livestreams in NewPipe (these streams cannot be downloaded or played in external players right now).
It also fixes the seek of PeerTube streams which have HLS-only variants of streams.
The support for downloading these streams will be not added in this PR.

Some code is inspired from #3803.

For more detailed changes, check the commits.

TO DO:

  • Fix the crash when pressing the Download button (IndexOutOfBoundsException);
  • Add a method in ListHelper to return only streams which have a delivery method (for e.g. return only HLS streams for PeerTube videos in the player)
  • Use the PROGRESSIVE_HTTP delivery method for external players when possible and add ability to play other variants of streams in these players
  • Support HLS "raw" manifests (the manifests which are returned by the extractor and not the URLs of HLS manifests) (if ExoPlayer supports them) (thanks to @Redirion for this).

Fixes the following issue(s)

Relies on the following changes

APK testing

On the website the APK can be found by going to the "Checks" tab below the title and then on "artifacts" on the right.

Due diligence

@AudricV AudricV added bug Issue is related to a bug player Issues related to any player (main, popup and background) labels Jun 20, 2021
@AudricV AudricV changed the title Add support of other delivery methods (in the player only) Add support of other delivery methods than progressive HTTP (in the player only) Jun 20, 2021
@AudricV

This comment has been minimized.

@wb9688

This comment has been minimized.

@AudricV

This comment has been minimized.

@AudricV

This comment has been minimized.

@opusforlife2
Copy link
Collaborator

This breaks PeerTube playback completely. All videos show the "Could not play this stream" toast.

@opusforlife2
Copy link
Collaborator

How is this implemented? Does it look for progressive streams first? Then DASH if those aren't available?

For videos where only one or two progressive streams are available, like 720p and 360p MP4, does it replace those resolutions with their DASH versions as well?

@AudricV
Copy link
Member Author

AudricV commented Jul 3, 2021

This breaks PeerTube playback completely. All videos show the "Could not play this stream" toast.

The app tries to play torrent streams. I will investigate why.

How is this implemented? Does it look for progressive streams first? Then DASH if those aren't available?

A DASH manifest is only returned by YouTube when OTF streams are present.

For videos where only one or two progressive streams are available, like 720p and 360p MP4, does it replace those resolutions with their DASH versions as well?

Only for the 1080p30/60 stream (and the 720p30 stream for cipher protected URLs). Other streams will use DASH versions.

@opusforlife2
Copy link
Collaborator

A DASH manifest is only returned by YouTube when OTF streams are present.

Still, does this PR prefer DASH or progressive? Or are progressive streams absent when OTF ones are present for all Youtube videos?

Only for the 1080p30/60 stream (and the 720p30 stream for cipher protected URLs). Other streams will use DASH versions.

So do you try to replace all streams with DASH versions and only show progressive ones where DASH streams aren't available?

@AudricV
Copy link
Member Author

AudricV commented Jul 3, 2021

Still, does this PR prefer DASH or progressive? Or are progressive streams absent when OTF ones are present for all Youtube videos?

So do you try to replace all streams with DASH versions and only show progressive ones where DASH streams aren't available?

I didn't change anything about the current implementation in the app but I think I need to, because NewPipe wants to play torrent streams first instead of progressive streams.

@opusforlife2
Copy link
Collaborator

Oh, I'm asking specifically in the context of Youtube.

@AudricV
Copy link
Member Author

AudricV commented Jul 3, 2021

Yes, I know but I don't know right now how the app manages which streams are chosen among the streams returned by the extractor.

@AudricV AudricV force-pushed the delivery-methods branch 2 times, most recently from 2ab85d9 to 827276e Compare July 8, 2021 16:09
@AudricV AudricV force-pushed the delivery-methods branch 2 times, most recently from d96dfa6 to f5d513c Compare July 14, 2021 14:11
@AudricV

This comment has been minimized.

@AudricV AudricV force-pushed the delivery-methods branch 2 times, most recently from 4144f90 to 950f8c0 Compare July 17, 2021 14:52
@AudricV AudricV force-pushed the delivery-methods branch from f8a9dbe to 8595125 Compare July 27, 2021 12:33
@AudricV
Copy link
Member Author

AudricV commented Jul 27, 2021

@opusforlife2 Can you confirm that PeerTube streams are playable for you and that for the streams that have only HLS variants, you can download these streams and seek properly within them?

@opusforlife2
Copy link
Collaborator

@TiA4f8R Finally! Yes, PeerTube streams are playable, and seeking works perfectly well within them.

Downloading them works fine (apart from the "some streams hidden" toast), but seeking within the downloaded streams doesn't. It varies from app to app, actually:

  • VLC can't seek at all.
  • FX File Explorer's media player shows a seekbar with a really tiny duration, like 4-5 seconds, but you can drag on the thumb to seek blindly.
  • mpv-android is the only one where the correct duration is shown, and seeking works properly. I assume it is making the calculations itself, then, and not relying on Android's Media Store.

@opusforlife2
Copy link
Collaborator

seeking within the downloaded streams doesn't

@kapodamy! We need your help!

@kapodamy
Copy link
Contributor

kapodamy commented Aug 1, 2021

someone pls link the apk or the downloaded stream

@AudricV
Copy link
Member Author

AudricV commented Aug 1, 2021

@opusforlife2 Please answer with a link of an HLS-only stream on which you have this issue.

…some toasts and the AlertDialog to open external video players.

Fix the crash when pressing the Background option on ended livestreams for external players: add a relevant toast when pressing the background option on VideoDetailFragement.
Add different toasts when no audio/video streams for external players are available (when using this option from RouterActivity).
Add a title of the AlertDialog used to play on external players in VideoDetailFragment and a message when there are no video streams available for external players.
…ethod

The two methods were always used at the same time so merging these methods increases code readability and reduces code size.

Also prevent a new NullPointerException in NavigationHelper.
Rename checkIfWasSomeStreamedRemoved method to checkIfSomeStreamWasRemoved.
Make wrappedAudioStreams, wrappedVideoStreams and wrappedSubtitleStreams public to be able to use DownloadDialog.newInstance(Context, StreamInfo)
Use DownloadDialog.newInstance(Context, StreamInfo) instead of DownloadDialog.newInstance(StreamInfo) in RouterActivity
Prevent some NullPointerExceptions in DownloadDialog
Remove removeTorrentAndNonUrlStreams method in PlaybackResolver interface and use the corresponding method in ListHelper in VideoPlaybackResolver
Split buildMediaSource method into five methods
Invert the if "!sortedVideoStreams.isEmpty()" in VideoDetailFragment
Readd the id of the threads LinearLayout (threads_layout)
Use instanceof instead of comparing classes with class fields and getClass method
Apply Android Studio suggestions in MissionRecoveryInfo
Fix a potential issue with the cache key given by PlayerHelper when there are different streams with an unknown resolution or with an unknown average bitrate
Add two new strings: unknown_format and unknown_quality, translated in French
Remove an unused constructor and an useless boolean check
Add some JavaDocs
Reformat some lines of code
…r.checkIfSomeStreamWasRemoved

There isn't an other way to compare if the number of downloadable streams is the same that the number of streams without introducing more complexity to the method.
So the CheckStyle warning can be supressed.
This may help us to debug some issues which may happen when trying to play a stream.
…ate and remove some old code in PlayerDataSource

The LoadErrorHandlingPolicy was set with an very big retry amount (Integer.MAX_VALUE), which makes no sense so it was removed, like the minimum manifest retry when an error occurs in live media sources (the value used is now the default of ExoPlayer).
Provide the duration of the video played to YoutubeDashManifestCreator, which is now required in order to generate manifests of OTF, progressive and post live DVR streams.
… dialog when rotating device

Previously, the Some streams were hidden message disappeared when rotating device and when the dialog was open. With this commit, that's not the case anymore.
Also migrate the TextView of this message to our custom TextView.
Reformat some code in DownloadDialog and ListHelper, fix a string capitalization and always show the streams removed message.
Improve some performance with the download dialog by changing the way of how non-supported streams are removed.
…ator

The limit is set to 500, which should be suitable for most usages and devices on which NewPipe is used.
@AudricV AudricV marked this pull request as ready for review February 22, 2022 21:41
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 11 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is related to a bug feature request Issue is related to a feature in the app multiservice Issues related to multiple services peertube Service, https://joinpeertube.org/ player Issues related to any player (main, popup and background)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash when playing PeerTube videos No playback of certain videos, infinite loading for HLS/DASH streams
7 participants