Add player support for different types of stream delivery #3803
Conversation
mauriciocolli
added
bug
|
Not conclusive, but I watched a few videos this morning and things were a lot smoother than usual. The hangups are rather spurious because the stream type isn't consistent, but I feel relatively confident this at least improves most of the cases, if not all of them. |
|
Yes! Yes! I had a bunch of videos in mind to try in case a PR came along, and they ALL work! Thank you so much, Mauricio! You're a champ. This is the best thing that's happened this week. ... I'm not crying. You're crying. |
| } else if (deliveryFormat instanceof DeliveryFormat.HLS) { | ||
| url = ((DeliveryFormat.HLS) deliveryFormat).getUrl(); | ||
| } else { | ||
| Toast.makeText(context, R.string.selected_stream_external_player_not_supported, |
There was a problem hiding this comment.
So this means external players can't play DASH videos, right?
There was a problem hiding this comment.
Yes, that's right.
Although, we could send instead the url pointing to the DASH manifest containing all the resolutions, but it'd make the resolution picker a little useless and maybe confusing.
Maybe even saving a file containing a manually crafted video+audio manifest and sending that to external players? I think it could work.
There was a problem hiding this comment.
I assume such videos would throw an error if shared from the Youtube app to VLC, for example?
|
Lots of music which are HLS only (TeamNewPipe/NewPipeExtractor#273) still don't work on SoundCloud :/ |
mauriciocolli
force-pushed
the
add-support-stream-delivery
branch
from
40ef789 to
b2dad01
Compare
For example, some streams have a direct link to them, and others use something like DASH or a HLS playlist. This adds support for them. The downloader doesn't support them yet, so when downloading these types of streams, they will be hidden.
Streams delivered by SoundCloud are not valid for very long, so a lower expiration time is needed.
mauriciocolli
force-pushed
the
add-support-stream-delivery
branch
from
b2dad01 to
efbf79f
Compare
Yeah, I left that for later, I just pushed a fix for that in the extractor PR. |
|
Any idea when this will get merged? Would love to see a new release with this included. |
|
Any reason this still hasn't been merged a month plus later? Is there anyone else who can review/push it? |
|
@dbenjaminmiller: Yes, the changes in #2907, this only being for the player and not the downloader, and me not liking the Extractor-side of this implementation. |
TobiGr
force-pushed
the
dev
branch
2 times, most recently
from
679bc75 to
2aeccc0
Compare
|
Any news about the PR? |
|
Closing in favour of #6537. |
8 participants
What is it?
Description of the changes in your PR
I am working in adding Twitch support for NewPipe and ran into this issue. Some streams have a direct link to them, and others use something like DASH or a HLS playlist. This PR adds support for them.
The downloader doesn't support them yet, so when downloading these types of streams, they will be hidden by default.
Fixes the following issue(s)
Relies on the following changes
Testing apk
add-support-stream-delivery.apk.zip
@obfripper @opusforlife2 @clarfon: Can you guys test it?