Bug 455339

Summary: Position indicator in the seek bar is stuck at the beginning, no matter how long the song has been played
Product: [Frameworks and Libraries] frameworks-qqc2-desktop-style Reporter: thebluequasar
Component: generalAssignee: kdelibs bugs <kdelibs-bugs>
Status: RESOLVED FIXED    
Severity: normal CC: ahiemstra, bart, hankster112, jackhill3103, kde, leo5t, marcus.yu.56, matej.starc1, me, nate, noahadvs, putr4.s
Priority: VHI Keywords: regression
Version: 5.95.0   
Target Milestone: ---   
Platform: Neon   
OS: Linux   
Latest Commit: Version Fixed In: 5.97
Attachments: Position indicator fixed at the start of progress bar

Description thebluequasar 2022-06-15 14:59:51 UTC
Created attachment 149748 [details]
Position indicator fixed at the start of progress bar

SUMMARY
Position indicator in the seek bar always stays at the start, irrespective of time passed for the song: See the screenshot


STEPS TO REPRODUCE
1. Open Elisa, start playing a track
2. Let it play the song for a few minutes, or move forward by clicking the seek bar
3. The position indicator still stays at the beginning of seek bar and doesn't move with changes in played/remaining time of the song.

OBSERVED RESULT
The position indicator in the seek/progress bar doesn't match the played and remaining time of the song, being fixed at the beginning of the slider bar.

EXPECTED RESULT
Position indicator should move according to played and remaining time of the song that's currently playing.

SOFTWARE/OS VERSIONS
Linux/KDE Plasma: KDE Neon User
KDE Plasma Version: Plasma 5.25
KDE Frameworks Version: 5.95
Qt Version: 5.15.4
Kernel: 5.13.0-48 generic (64bit)
Graphics platform: happens on both Xorg and Wayland

ADDITIONAL INFORMATION
There's a screenshot attached
Comment 1 Patrick Silva 2022-06-16 14:11:59 UTC
*** Bug 455402 has been marked as a duplicate of this bug. ***
Comment 2 Jack Hill 2022-06-21 11:06:18 UTC
*** Bug 455664 has been marked as a duplicate of this bug. ***
Comment 3 Jack Hill 2022-06-21 11:07:36 UTC
Can reproduce on flatpak version
Comment 4 bart 2022-06-21 11:21:28 UTC
Found the issue.  It's caused by this change in qqc2-desktop-style:

https://invent.kde.org/frameworks/qqc2-desktop-style/-/commit/c00006217ec18bc7be49eba0f43757d2c44e4e6a

The intention of this commit is to increase the precision of Slider by making the subdivisions 100 times more granular.  But it ends up making the value overflow at 100 times lower values.  As mentioned in one of the duplicates of this ticket, the threshold where this bug occurs is around 3 minutes 30, which equates to 2147483647 / 10000 (where 10000 is the new number of subdivisions).
Comment 5 Nate Graham 2022-06-21 13:18:55 UTC
Indeed. CCing the author of that commit. Good ol' integer overflows...
Comment 6 ratijas 2022-06-21 13:56:04 UTC
Wow
Comment 7 ratijas 2022-06-21 14:04:03 UTC
In this case we could go two routes:

- Use heuristics based on current values of `from` and `to` to adjust precision dynamically,
- Lower the values on Elisa side.

If my understanding is correct, current (new) threshold is 3 minutes 30 seconds which is only 100 times lower, so an old one was only about 5 hours 50 minutes — less than your average audiobook?

I don't think Elisa needs that much precision, as the slider isn't updated more often than once or twice per second anyway?
Comment 8 Nate Graham 2022-06-21 14:06:32 UTC
Remember that as a general rule, frameworks can't break apps, so we can do both, but not just one.
Comment 9 bart 2022-06-21 14:13:57 UTC
> I don't think Elisa needs that much precision, as the slider isn't updated
> more often than once or twice per second anyway?

Just as background info, Elisa is using the raw player position and duration values from qmediaplayer as "value" and "to" for the slider.  These qmediaplayer values are in milliseconds by default.  I agree it would make sense to also adapt this in elisa to something more sensible.

I did hit that "old" limit myself in Kasts when I was listening to a 6 hour long podcast.  I had updated Kasts to rescale all values for the slider to seconds rather than milliseconds.  With seconds being used, even the "new" limit would be at 60+ hours (I think), which should be ok.

Let me also open an MR against Elisa to implement rescaled internal Slider "value" and "to".  I already have it half-implemented on my end because I used that rescaling to debug the problem in the first place.
Comment 10 Bug Janitor Service 2022-06-21 17:01:10 UTC
A possibly relevant merge request was started @ https://invent.kde.org/multimedia/elisa/-/merge_requests/364
Comment 11 Nate Graham 2022-06-22 23:28:17 UTC
Git commit 98227f05186d80124d9375f125057af0585267f8 by Nate Graham, on behalf of Bart De Vries.
Committed on 22/06/2022 at 23:22.
Pushed by ngraham into branch 'master'.

Rescale internal Slider values to avoid integer overflow

The original implementation used the raw duration and position as
reported by qmultimedia, which are in milliseconds. This, combined with
the actual Slider implementation in qqc2-desktop-style caused
value/duration to overflow at about 3 minutes and 30 seconds.
The adapted implementation uses seconds for the internal Slider values.

M  +8    -8    src/qml/DurationSlider.qml

https://invent.kde.org/multimedia/elisa/commit/98227f05186d80124d9375f125057af0585267f8
Comment 12 Nate Graham 2022-06-22 23:32:08 UTC
Git commit b3d081d14a6da1e18ac220867eb7961b8a017837 by Nate Graham, on behalf of Bart De Vries.
Committed on 22/06/2022 at 23:32.
Pushed by ngraham into branch 'release/22.04'.

Rescale internal Slider values to avoid integer overflow

The original implementation used the raw duration and position as
reported by qmultimedia, which are in milliseconds. This, combined with
the actual Slider implementation in qqc2-desktop-style caused
value/duration to overflow at about 3 minutes and 30 seconds.
The adapted implementation uses seconds for the internal Slider values.


(cherry picked from commit 98227f05186d80124d9375f125057af0585267f8)

M  +8    -8    src/qml/DurationSlider.qml

https://invent.kde.org/multimedia/elisa/commit/b3d081d14a6da1e18ac220867eb7961b8a017837
Comment 13 Nate Graham 2022-06-27 17:28:12 UTC
*** Bug 456023 has been marked as a duplicate of this bug. ***
Comment 14 Yerrey Dev 2022-06-29 09:34:24 UTC
*** Bug 456109 has been marked as a duplicate of this bug. ***
Comment 15 Bug Janitor Service 2022-07-22 17:35:29 UTC
A possibly relevant merge request was started @ https://invent.kde.org/frameworks/qqc2-desktop-style/-/merge_requests/177
Comment 16 ratijas 2022-07-22 18:31:56 UTC
Git commit efb795efbe0579cb30cc042d61214c139c5fb110 by ivan tkachenko.
Committed on 22/07/2022 at 17:31.
Pushed by ratijas into branch 'master'.

ProgressBar,Slider: Adapt great precision to the harsh reality

Test case: `to` values >= 214749 should work.

M  +7    -3    org.kde.desktop/ProgressBar.qml
M  +8    -4    org.kde.desktop/Slider.qml
M  +4    -5    tests/testBars.qml

https://invent.kde.org/frameworks/qqc2-desktop-style/commit/efb795efbe0579cb30cc042d61214c139c5fb110