More Usability Update #1189
More Usability Update #1189
Conversation
|
Some updates:
@theScrabi Livestream should now always start at the live edge, though it seems to me the startup for livestreams are bit slower now as well. I'll probably look into why this is in a little while. |
-Added button to wipe metadata cache. -Added more paddings on player buttons. -Added new animations to main player secondary controls and play queue expand/collapse. -Refactored play queue item touch callback for use in all players. -Improved MediaSourceManager to better handle expired stream reloading. -[TeamNewPipe#1186] Changed live sync button text to "LIVE". -Removed MediaSourceManager loader coupling on main players. -Moved service dependent expiry resolution to ServiceHelper. -[TeamNewPipe#1186] Fixed livestream timeline updates causing negative time position. -[TeamNewPipe#1186] Fixed livestream not starting from live-edge. -Fixed main player system UI not retracting on playback start.
-Changed play queue items to display service names. -Fixed Soundcloud playlist not fitting thumbnail. -Refactored image display options to follow uniform behavior. -Refactoring and style changes on audio reactor and media button receiver.
…een prepared. -Fixed livestream not seeking to live when started from play queue. -Fixed media source manager synchronization to only occur after timeline change has completed. -Fixed auto queue not working when last item is replayed after the auto-queued item is removed. -Updated ExoPlayer to 2.7.1.
-Extracted version numbers in gradle dependencies. -Updated ExoPlayer to 2.7.1. -Updated RxJava to 2.1.10, RxAndroid to 2.0.2 and RxBinding to 2.1.1. -Removed deprecated implementation of media buttons.
…agedMediaSource. -Fixed null input causing potential NPE on PlayQueueItem.
app/build.gradle
Outdated
| exoPlayerLibVersion = '2.7.1' | ||
| roomDbLibVersion = '1.0.0' | ||
| leakCanaryVersion = '1.5.4' | ||
| okHttpVersion = '1.5.0' |
There was a problem hiding this comment.
Shouldn't it be stethoVersion?
Also switching between . Forgot about the string interpolation." and '
|
Some things that maybe you want to fix/improve in this pull request, but just for the record if you don't:
The thing is, we talked about replace the
We have to eliminate all uses of the |
|
@mauriciocolli: I'd recommend using Fresco instead of UIL.
|
|
@karyogamy i thought about removing the paceholder banner compleatly, because it looks ugly. We could just make the view br gone if there is nothing to show |
…player tempo and pitch parameters. -Changed tempo and pitch button in service player activity and tempo button in main video player to open speed control dialog. -Changed LIVE button to be no longer clickable when player position is at or beyond default position. -Changed main video player to use AppCompatActivity rather than Activity. -Fixed video player tempo button not updating when player speed parameters change. -Fixed player crashing on lower sdk versions due to no MediaButtonReceiver, added intent back to manifest. -Fixed inconsistent gradle library naming. -Fixed stetho dependencies incorrect version.
Now the LIVE button is disabled whenever the progress update shows the current position is greater or equal to the default position =D
The adaptive resolution selection should be seamless, maybe it is because the bandwidth meter not reacting to connections slowing down and not changing to a lower resolution which causes the stream to buffer? Also, this might happen if you are at the very edge of the livestream, where the source may not have enough to watch yet. Would appreciate more info on this issue.
Yeah, this does look strange. Right now, I can't think of a generic UI that works well for both livestreams with small and large back buffer. Though when we do have a solution, we will probably have way too many different UI to handle under different stream types, so it is probably a good time to do a good refactor on the player. At this point, I think it is a good idea to separate out the player and play queue into from the app, so new features to the player no longer impede development of other core functionalities, and vice versa. This should eliminate most of the coupling between the app and the player, making the embedded branch easier to develop, test and merge.
I was thinking we can transition into using Glide, since it seems Fresco adds about 1.1 mb to the apk size compared to Glide's 200kb when I build the app after adding them to the dependencies. Also, from what I've seen on stackoverflow, Fresco uses a lot of RAM and need a large cache, though I'm not sure if this still holds true today. What's your reason for choosing Fresco, though? @theScrabi Anyways, here's something else to play with. Added as of this commit. This also means we now have a spare button in the service activity. If anyone have a good idea what other functionality we should have there, please don't hesitate to suggest it. |
|
@karyogamy: Fresco should only add about 500 kB (according to its FAQ). It has a lot more features than Glide and it's faster, mainly because it uses native code, but we'd probably wanna have different build variants for the different architectures, but I don't know how good F-Droid supports that and it'd probably be confusing for our users, so now I think about it again, maybe it'd be better to indeed use Glide. When you implement that, make sure to use our |
So how does the official YouTube app do it? |
This might be a problem, since I believe this was the main reason why we stayed away from including ffmpeg support in the app.
For livestream with long back buffer, they remove the current position when its live and only show the current time when seeking to some time in the past. For short back buffers, they get rid of the progress bar altogether. On the video players, this may look fine. But on the play queue activity, this will probably look strange. |
That's indeed one of the reasons (besides FFmpeg just being big). That's why I said:
Couldn't we disable the play queue for a live stream? I think a play queue wouldn't make sense at all for a live stream. |
|
@wb9688 I disagree with disabling play queue.
|
Ah, sorry. Late nights =\
@avently has some valid points. I don't think users would expect this change in consistency either. That being said, we could just use the youtube UI. And on livestream with short buffer, force the seekbar thumb to stay at the end of the bar and have the progress filled, so the UI appears consistent. Though, because we can't determine the size of the buffer in the extractor, there will be multiple places where the UI strategy can be determined. Since right now this UI change doesn't impact any player functionalities, it's probably a good idea to do this UI change after we pull the player out and have done a good deal of restructuring, so UI elements can be updated from a single source of change. |
That banner you mentioned |
|
@theScrabi Oh, yeah. Please do, or at least shrink it down. The video not available image |
-Modified playback speed control to use quadratic sliders instead of linear. -Modified number formatters in player helper to use double instead of float. -Simplified slider behavior in playback parameter dialog. -Fixed potential NPE in base local fragment.
Do. I'll put it onto my todo list.
Maybe we can use ShapeDrawable here. |
|
So from the UI I think the refactoring is nice :) However I found a small thing that can be improved:
|
On a related issue, maybe we should also replace our existing icons with vector drawables, since right now they look really bad when enlarged (i.e. the play/restart button on the main video player on a large display).
If I recall correctly, this was added and then removed in favor of the LIVE label (as duration) on the thumbnail. For Soundcloud, the audio icon on the thumbnail should also indicate that this is audio only. So I think having another label on the toolbar is somewhat redundant. But you are right, that space does feel empty. @mauriciocolli what do you think we should do with the toolbar? |
| @@ -587,7 +588,8 @@ private void initThumbnailViews(StreamInfo info) { | |||
| imageLoader.displayImage( | |||
| info.getThumbnailUrl(), | |||
| thumbnailImageView, | |||
| DISPLAY_THUMBNAIL_OPTIONS, new SimpleImageLoadingListener() { | |||
| ImageDisplayConstants.DISPLAY_THUMBNAIL_OPTIONS, | |||
| new SimpleImageLoadingListener() { | |||
There was a problem hiding this comment.
I guess this can be converted into a lambda expression.
|
I like this commit very much, it adds those tiny awesome details that one missed :D Also the code looks very clean and strait forward 👍 And when I'm already there: I'd like to thank you @karyogamy and @wb9688 you two are currently making my days. SoundCloud support and automatic playlist playback are the most vital tools when I do my daily work on the owncloud app :D Thanks guys! |
…r rather than a full error activity.
…gating away from activity during interruption, when resume on focus regain is enabled. -Separated onPause and onPlay functions from onPlayPause. -Renamed onVideoPlayPause to onPlayPause.
I'm glad to hear that =D As for the detail fragment thumbnail loading, I've changed it to produce a snackbar error instead of a full-blown error activity. Also fixed a bug where the main player is not paused when navigating away under very specific circumstances, so please be sure to go over that too. |
@theScrabi I've opted to use "LIVE" instead of "NOW", since this is probably easier to understand when the user is coming from the Youtube app.
@theScrabi @mauriciocolli I just noticed, when benchmarking the memory, the
background_header.pngand placeholder png for the services takes up about 3mb and 1mb (respectively) of memory when stored as bitmap, as opposed to the 30kb when the are png in theresfolder. Perhaps should shrink these images down to save RAM usage?