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.

Sign up for GitHub

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

Jump to bottom

Metrics collector UI #655

Merged
merged 18 commits into from
Jul 30, 2024
Merged

Metrics collector UI #655

merged 18 commits into from
Jul 30, 2024

Conversation

StaehliJ
Copy link
Contributor

@StaehliJ StaehliJ commented Jul 25, 2024 •
edited
Loading

Pull request

Description

The Goal of this PR is to had a metrics overlay into the demo. The overlay can be enabled and customized in the new tab "setting"

Changes made

  • Add mobile new settings navigation tab.
  • Add compose overlay to display PlaybackMetrics.

Checklist

  • Your branch has been rebased onto the main branch.
  • APIs have been properly documented (if relevant).
  • The documentation has been updated (if relevant).
  • New unit tests have been written (if relevant).
  • The demo has been updated (if relevant).
  • All pull request status checks pass.

@MGaetan89 MGaetan89 linked an issue Jul 26, 2024 that may be closed by this pull request
Basic metrics collector UI #639
Closed
3 tasks
MGaetan89
MGaetan89 reviewed Jul 26, 2024
View reviewed changes
Copy link
Member

@MGaetan89 MGaetan89 left a comment

Choose a reason for hiding this comment

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

Just a few comments.
I took the liberty of updating the setting screen, so its design is more in line with the rest of the demo app. It's in a dedicated commit (e32129c), so it's easy to revert if needed.

...mo-shared/src/main/java/ch/srgssr/pillarbox/demo/shared/ui/settings/MetricsOverlayOptions.kt Show resolved Hide resolved
pillarbox-demo/src/main/java/ch/srgssr/pillarbox/demo/ui/player/DemoPlayerView.kt
val appSettingsRepository = remember {
AppSettingsRepository(context)
}
val appSettings by appSettingsRepository.getAppSettings().collectAsStateWithLifecycle(AppSettings())
Copy link
Member

Choose a reason for hiding this comment

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

Why not use the AppSettingsViewModel here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could yes, in won't be short to write and the viewmodel will be scoped to the Activity if I'm not wrong.

pillarbox-demo/src/main/java/ch/srgssr/pillarbox/demo/ui/player/PlayerView.kt Show resolved Hide resolved
pillarbox-demo/src/main/java/ch/srgssr/pillarbox/demo/MainNavigation.kt Outdated Show resolved Hide resolved
pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/utils/BitrateUtil.kt Show resolved Hide resolved
pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/utils/BitrateUtil.kt Outdated Show resolved Hide resolved
@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 26, 2024 •
edited
Loading

Code Coverage

Overall Project 53.29% -0.01% 🟢
Files changed 84.21% 🟢

Module Coverage
:pillarbox-player 59.26% -0.02% 🟢
Files
Module File Coverage
:pillarbox-player SessionMetrics.kt 88.58% 🟢
QoSCoordinator.kt 85.68% 🟢
BitrateUtil.kt 57.14% -42.86% 🟢

StaehliJ and others added 18 commits July 29, 2024 15:05
@StaehliJ @MGaetan89
Simple overlay view wip
394a1de
@StaehliJ @MGaetan89
Simple demo application settings
612e670
@StaehliJ @MGaetan89
Add Media3 lib version
543ca98
@StaehliJ @MGaetan89
Connect debug metrics view to app settings
965494c
@StaehliJ @MGaetan89
Add text color and size as settings
b6ec265
@StaehliJ @MGaetan89
Refactor and add string resources
d130638
@StaehliJ @MGaetan89
Settings are scrollable
3b385d8
@StaehliJ @MGaetan89
Remove duplicated value
f1c9b27
@StaehliJ @MGaetan89
Rebase on qos
2341a1e
@StaehliJ @MGaetan89
Return NO_VALUE if both Format are not defined
fd221b0
@StaehliJ @MGaetan89
Simplify MetricsOverlay to be updated every 500ms
bf99ca7
@StaehliJ @MGaetan89
Apply AndroidLint suggestions
da6f499
@MGaetan89
Simplify AppSettingsRepository and align some namings
1490971
@MGaetan89
Align AppSettingsView design with the rest of the demo app
da75bcf
@MGaetan89
Make the dropdown menu display where the click happened
6b7cb2e 
Also update `CustomPlaybackSettingsShowcase` to have the same behavior
@StaehliJ @MGaetan89
Remove unused methods
25611bd
@StaehliJ @MGaetan89
Add settings to top level routes
d77b5a3
@StaehliJ @MGaetan89
Use BitrateUtil
7577e27
@StaehliJ StaehliJ merged commit 09eb787 into qos Jul 30, 2024
10 checks passed
@StaehliJ StaehliJ deleted the metrics-collector-ui branch July 30, 2024 07:17
MGaetan89 added a commit that referenced this pull request Jul 30, 2024
@StaehliJ @MGaetan89
Metrics collector UI (#655)
ce95824 
Co-authored-by: Gaëtan Muller <[email protected]>
MGaetan89 added a commit that referenced this pull request Aug 12, 2024
@StaehliJ @MGaetan89
Metrics collector UI (#655)
b3a566d 
Co-authored-by: Gaëtan Muller <[email protected]>
MGaetan89 added a commit that referenced this pull request Aug 13, 2024
@StaehliJ @MGaetan89
Metrics collector UI (#655)
97b3f3f 
Co-authored-by: Gaëtan Muller <[email protected]>
MGaetan89 added a commit that referenced this pull request Aug 20, 2024
@StaehliJ @MGaetan89
Metrics collector UI (#655)
7648c01 
Co-authored-by: Gaëtan Muller <[email protected]>
StaehliJ added a commit that referenced this pull request Aug 28, 2024
@StaehliJ @MGaetan89
Metrics collector UI (#655)
ec3548b 
Co-authored-by: Gaëtan Muller <[email protected]>
StaehliJ added a commit that referenced this pull request Sep 11, 2024
@StaehliJ @MGaetan89
Metrics collector UI (#655)
a46173c 
Co-authored-by: Gaëtan Muller <[email protected]>
MGaetan89 added a commit that referenced this pull request Sep 12, 2024
@StaehliJ @MGaetan89
Metrics collector UI (#655)
f228d5b 
Co-authored-by: Gaëtan Muller <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MGaetan89 MGaetan89 MGaetan89 left review comments

Assignees
No one assigned
Labels
None yet
Projects
Pillarbox
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Basic metrics collector UI
2 participants
@StaehliJ @MGaetan89