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

Support delivery methods other than progressive HTTP #8153

Merged
merged 20 commits into from
Jun 22, 2022

Conversation

AudricV
Copy link
Member

@AudricV AudricV commented Apr 5, 2022

Previous too-long title: Add support of other delivery methods than progressive HTTP (in the player only), use DASH to fetch YouTube progressive contents, fix extraction of PeerTube streams with separate audio stream and more

What is it?

  • Bugfix (user facing)
  • Feature (user facing)

Description of the changes in your PR

This PR superseeds #6537 and adds so support of other delivery methods of contents than progressive HTTP, in the player only. It also:

  • fixes seeking of PeerTube HLS streams, by using streams' HLS manifests to play HLS streams;
  • fixes the lack of some streams on some YouTube videos (OTF streams);
  • improves a lot loading times of YouTube streams, after a quality change or a playback start (progressive streams are now using DASH to fetch contents, and the current way is used as a fallback if something goes wrong when generating the DASH manifest associated to this stream);
  • fixes the extraction of YouTube ended livestreams as livestreams (this type of streams is not interpreted anymore as them but as a separate type instead, look at the extractor PR for more details), which also:
    • fixes watch count interpreted as watching count;
    • fixes the watchable time of these contents (full content instead of the last 2 or 4 hours, depending of the length returned in the HLS manifest).
  • fixes the crash when trying to play PeerTube videos with a separate audio stream;
  • improves the quality selection for external players dialog, by adding a title to it and a message when no streams are available for external players (see Before/After Screenshots/Screen Record section);
  • fixes playback of SoundCloud HLS-only tracks (which cannot be downloaded anymore, as the workaround I introduced is being removed by SoundCloud and has been already removed in the extractor with Fix extraction of some properties in ItagItems for YouTube livestreams and post-live streams and remove completely SoundCloud HLS workaround NewPipeExtractor#859).
Detailed changes

External players:

  • Add a message instruction about stream selection;
  • Add a message when there is no stream available for external players;
  • Return now HLS, DASH and SmoothStreaming URL contents, in addition to progressive HTTP ones.

Player:

  • Support DASH, HLS and SmoothStreaming streams for videos, whether they are content URLs or the manifests themselves, in addition to progressive HTTP ones;
  • Use a custom HttpDataSource to play YouTube contents, named YoutubeHttpDataSource, based of ExoPlayer's default one, which allows better spoofing of official clients (custom user-agent and headers (depending of the client used), use of range and rn (set dynamically by the DataSource) parameters);
  • Fetch YouTube progressive contents as DASH streams, like official clients, and supports full playback of livestreams which have ended recently;
  • Use ExoPlayer's default retries count for contents on non-fatal errors (instead of Integer.MAX_VALUE for non-live contents and 5 for live contents).

Download dialog:

  • Add message about support of progressive HTTP streams only for downloading;
  • Remove several duplicated code and update relevant usages;
  • Support downloading of contents with an unknown media format.

ListHelper:

  • Catch NumberFormatException when trying to compare two video streams between them.

Tests:

  • Update ListHelperTest and StreamItemAdapterTest to fix breaking changes in the extractor.

Other places:

  • Fixes deprecation of changes made in the extractor;
  • Improve some code related to the files changed.

Before/After Screenshots/Screen Record

  • Before:

    Stream selection for external players Download dialog
    select_quality_stream_dialog_before download_dialog_before
  • After:

    Stream selection for external players Download dialog
    SVID_20220413_160341_1.mp4
    download_dialog_after
  • New:

    Stream selection for external players when no stream is available
    select_quality_stream_dialog_no_stream_available

Fixes the following issue(s)

Relies on the following changes

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.

Due diligence

@AudricV AudricV added bug Issue is related to a bug feature request Issue is related to a feature in the app youtube Service, https://www.youtube.com/ multiservice Issues related to multiple services player Issues related to any player (main, popup and background) peertube Service, https://joinpeertube.org/ labels Apr 5, 2022
@robzombie91

This comment was marked as off-topic.

@AudricV
Copy link
Member Author

AudricV commented Apr 11, 2022

That's not related to my changes at all.

Anyway, thanks for testing (I will push new code with a few fixes and extractor updates soon, so it would be great if you can test again when I will push it and when the CI will build a new debug APK)!

@robzombie91
Copy link

robzombie91 commented Apr 11, 2022

Works good and fixes the issue I talked about

@@ -3,22 +3,38 @@
import org.schabi.newpipe.extractor.stream.StreamType;

/**
* Utility class for {@link org.schabi.newpipe.extractor.stream.StreamType}.
Copy link
Member

@TobiGr TobiGr Apr 13, 2022

Choose a reason for hiding this comment

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

IMO, these methods should be static methods in the StreamType enum.
But that is a task for a separate PR.

@AudricV AudricV force-pushed the delivery-methods-v2 branch 2 times, most recently from 948d1c3 to 6e0caf4 Compare April 15, 2022 17:26
@nicoursi

This comment was marked as off-topic.

@AudricV

This comment was marked as off-topic.

@nicoursi

This comment was marked as off-topic.

@AudricV

This comment was marked as off-topic.

@nicoursi
Copy link

nicoursi commented Apr 23, 2022

This bug happens only with your PR while playing this peertube stream.

Exception

Crash log

` {"user_action":"play stream","request":"Player error[type=ERROR_CODE_IO_UNSPECIFIED] occurred while playing https://peertube.it/videos/watch/a34aedc6-5cda-40c5-9fe2-4aca5587985a","content_language":"it","content_country":"IT","app_language":"it_IT","service":"PeerTube","package":"org.schabi.newpipe.debug.deliverymethodsv2","version":"0.22.1","os":"Linux Android 11 - 30","time":"2022-04-23 11:14","exceptions":["com.google.android.exoplayer2.ExoPlaybackException: Source error\n\tat com.google.android.exoplayer2.ExoPlayerImplInternal.handleIoException(ExoPlayerImplInternal.java:641)\n\tat com.google.android.exoplayer2.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:617)\n\tat android.os.Handler.dispatchMessage(Handler.java:102)\n\tat android.os.Looper.loop(Looper.java:223)\n\tat android.os.HandlerThread.run(HandlerThread.java:67)\nCaused by: com.google.android.exoplayer2.upstream.Loader$UnexpectedLoaderException: Unexpected IllegalArgumentException: null\n\tat com.google.android.exoplayer2.upstream.Loader$LoadTask.run(Loader.java:433)\n\tat java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)\n\tat java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)\n\tat java.lang.Thread.run(Thread.java:923)\nCaused by: java.lang.IllegalArgumentException\n\tat com.google.android.exoplayer2.util.Assertions.checkArgument(Assertions.java:39)\n\tat com.google.android.exoplayer2.upstream.DataSpec.(DataSpec.java:650)\n\tat com.google.android.exoplayer2.upstream.DataSpec.subrange(DataSpec.java:708)\n\tat com.google.android.exoplayer2.upstream.DataSpec.subrange(DataSpec.java:694)\n\tat com.google.android.exoplayer2.source.hls.HlsMediaChunk.feedDataToExtractor(HlsMediaChunk.java:463)\n\tat com.google.android.exoplayer2.source.hls.HlsMediaChunk.loadMedia(HlsMediaChunk.java:437)\n\tat com.google.android.exoplayer2.source.hls.HlsMediaChunk.load(HlsMediaChunk.java:394)\n\tat com.google.android.exoplayer2.upstream.Loader$LoadTask.run(Loader.java:412)\n\t... 3 more\n"],"user_comment":""}`

@AudricV
Copy link
Member Author

AudricV commented Apr 23, 2022

@nicoursi I could not reproduce this bug on my side. It seems be an issue in ExoPlayer itself (I remember I got it when working on implementing YouTube progrerssive DASH streams, but it was probably not related to the issues I had), as to play PeerTube streams, we are not using any custom components, only the ones of ExoPlayer.

According to the stacktrace you provided, the line where the crash occured is in the DataSpec ExoPlayer class:

Assertions.checkArgument(length > 0 || length == C.LENGTH_UNSET);

(so because the argument does not meet the assertion)

I'm afraid I cannot do something for this type of things. You may try to clear the application cache and try to seek in the stream, using the playback notification, after trying to play again the stream.

@nicoursi
Copy link

Clearing the cache fixed the problem. Thanks

@AudricV AudricV mentioned this pull request Apr 23, 2022
5 tasks
@Stypox Stypox changed the title Add support of other delivery methods than progressive HTTP (in the player only), use DASH to fetch YouTube progressive contents, fix extraction of PeerTube streams with separate audio stream and more Support delivery methods other than progressive HTTP Apr 30, 2022
…with list iterators and add a better toast when there is no audio stream for external players

This ensures to not remove streams from the StreamInfo lists themselves, and so to not have to create list copies.

The toast shown in RouterActivity, when there is no audio stream available for external players, is now shown, in the same case, when pressing the background button in VideoDetailFragment.
A new exception, ResolverException, a subclass of PlaybackResolver, is now thrown when errors occur in PlaybackResolver, instead of an IOException
A custom HlsPlaylistParserFactory cannot be used anymore to play HLS streams.

This needs to be replaced by a custom HlsDataSourceFactory, which returns a ByteArrayDataSource (where the bytes of this DataSource correspond to the bytes of the playlist string) and a specified DataSource for other request types.

This model has two limitations:

- if media requests are relative, the URI from which the manifest comes from (either the manifest URI (preferred) or the master URI (if applicable)) must be returned, otherwise the content will be not playable, as it will be an invalid URL, or it may be treat as something unexpected, for instance as a file for DefaultDataSources;
- if the playlist is a master playlist, endless loops should be encountered because the DataSources created for media playlists will use the master playlist response instead of fetching the corresponding playlist. With the current model of HlsDataSourceFactory, there is no possibility to distinguish the playlist type or the URI that is requested.

If ExoPlayer provides a way to create HlsMediaSources with an HlsPlaylist in the future, it should be used instead of this solution.
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.

Great! I think the code is now understandable and should work well. I only have some non-critical comments. I pushed a few commits solving the comments marked as (already done), though there are still a couple of comments that we need to discuss. Thank you!

app/src/main/java/org/schabi/newpipe/RouterActivity.java Outdated Show resolved Hide resolved
app/src/main/java/org/schabi/newpipe/util/ListHelper.java Outdated Show resolved Hide resolved
desired = stream.resolution
isDesired2 = stream.isVideoOnly
desired = stream.getResolution()
isDesired2 = stream.isVideoOnly()
Copy link
Member

Choose a reason for hiding this comment

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

Small note for the future: when resulution, isVideoOnly and other fields will be made completely private in Streams, these will be simplifiable with "property access syntax" as Android Studio suggests.

app/src/main/res/layout/download_dialog.xml Outdated Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented Jun 19, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 1 Security Hotspot
Code Smell A 14 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@Stypox
Copy link
Member

Stypox commented Jun 22, 2022

I just noticed a small player problem: YouTube livestreams where you are only allowed to seek back at most one hour behave strangely (they keep reaching the end, loading, then playing for a bit, then reaching the end again, ...). See for example https://www.youtube.com/watch?v=5qap5aO4i9A . Other than that everything I tested worked fine, and I don't even know if the mentioned problem would also occur in the current dev. So I'm merging this :-)

@Stypox Stypox merged commit 3901ffc into TeamNewPipe:dev Jun 22, 2022
@AudricV
Copy link
Member Author

AudricV commented Jun 22, 2022

I can reproduce this sometimes (also YouTube HLS manifests have sometimes issues).

To avoid buffering issues on livestreams which allows rewinding, we already set the default live position 10 seconds before the live edge.

We can also a custom ExoPlayer DASH MediaSource for YT live sources, which would require to be able to create DASH live manifests, which may solve these issues + clear the delay between the segment sent to official clients via DASH and the one sent in HLS manifests.

@Stypox Stypox mentioned this pull request Jun 23, 2022
5 tasks
@AudricV AudricV added the soundcloud Service, https://soundcloud.com/ label Jun 24, 2022
@Stypox Stypox mentioned this pull request Jun 24, 2022
3 tasks
@AudricV AudricV deleted the delivery-methods-v2 branch July 8, 2022 20:46
@Fs00 Fs00 mentioned this pull request Jul 26, 2022
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) soundcloud Service, https://soundcloud.com/ youtube Service, https://www.youtube.com/
Projects
None yet
7 participants