Repository navigation
Conversation
fire-light42
left a comment
fire-light42
left a comment
There was a problem hiding this comment.
Interesting idea which would be very useful for livestream viewers.
However, this pull request contains two big issues:
1. The code is over-engineered
It only uses its own logic instead of integrating with the app. ZappingContext, ZappingSession, ZappingSessionStore, etc is way too much and way to complicated. The UI is also fully customized.
I would suggest re-using the in-video episodes popup to select other livestreams. This reduces the complexity and aligns with what users already expect. I also suggest offloading the zapping logic to a custom VideoGenerator. This removes complex logic within an already complex player.
2. Views are not in XML
If you need a custom view then please create XML files instead of creating the view programmatically.
| scaleType = ImageView.ScaleType.CENTER_CROP | ||
| setBackgroundColor(context.colorFromAttribute(R.attr.boxItemBackground)) | ||
| } | ||
| val scrim = View(context).apply { |
There was a problem hiding this comment.
All throughout the pull request you create layouts programmatically. This gives a working result, but is very difficult to inspect, maintain and review. Please use XML (or Compose) to inflate the view, and only use Kotlin to fill the view (i.e. change visibility, set texts, set images...).
| * Gives specialised players a chance to consume TV channel-style DPAD input while the | ||
| * controls are hidden. Normal player navigation remains unchanged when this returns false. | ||
| */ | ||
| protected open fun handleLiveChannelKey(keyCode: Int): Boolean = false |
There was a problem hiding this comment.
The current implementation fully hooks the up and down buttons on TV which makes normal navigation in livestreams impossible.
| val currentIndex: Int, | ||
| ) { | ||
| init { | ||
| require(channels.isNotEmpty()) { "ZappingContext requires at least one channel" } |
There was a problem hiding this comment.
This will lead to app crashes. Use debugAssert instead.
| if (!enabled) toggleZappingList(false) | ||
| } | ||
|
|
||
| private fun isZappingEnabled(): Boolean { |
There was a problem hiding this comment.
Use AppSettings(context).player.zappingEnabled instead
|
Thanks for the review. I've reworked the implementation around I've also adjusted the TV navigation so the normal player focus behavior is preserved, while channel switching is only handled when the player controls are hidden. The latest version has been manually tested with touch controls and Android TV DPAD/focus navigation, and both the build and unit tests pass. Could you please take another look when you have time? One small question: we're currently using test builds with our community for this feature. If the PR is accepted, roughly when would it become available to regular CloudStream users? |
When it is merged all pre-release users will get access to it within 15 minutes. CloudStream users on stable will gain access to it within a month or two. |
|
Sabırla bekliyoruz |
|
It will be very usefull idea wish dev will add this feature |
|
This is exactly the kind of behavior I’d love to see for live TV. While watching a live stream, it should be possible to switch directly to the previous/next channel with a single Up/Down press on the TV remote — similar to CH+/CH− on a traditional TV — without leaving the player or opening the channel list first. That kind of quick channel zapping would make live TV in CloudStream feel much more natural. |
|
We're waiting, Master. |
|
özelliği bekliyoruz hocam teşekkürler |
@fire-light42 maybe this one got forgotten? 😅 |
|
Bekliyoruz teşekkürler hocam |
|
Harika bir özellik olacak bekliyoruz |
|
Sağda oynatma listesi özelliği istiyoruz 😉 |
|
I have not forgotten about this pull request, I have simply been busy recently. It will be merged 👍 |
I wonder when this feature will arrive; we are eagerly awaiting it. |
This branch has not been deployed
11 participants
Summary
LiveZappingGeneratorto keep zapping logic separate from the core player.Validation
:app:assembleStableDebug— passed:app:testStableDebugUnitTest— passedAI assistance was used during implementation and iteration. The resulting code was manually reviewed and tested before this pull request.