Fix current track display/broadcasting/recording when starting auto-DJ #3500
Conversation
src/mixer/playerinfo.cpp
Outdated
| } | ||
| } | ||
|
|
||
| int PlayerInfo::getCurrentPlayingDeck() { | ||
| QMutexLocker locker(&m_mutex); | ||
| updateCurrentPlayingDeck(); |
There was a problem hiding this comment.
Hiding this heavy-weight mutable operation inside a"getter" function that should actually be idempotent is a code smell. Do we have other options?
There was a problem hiding this comment.
This change was required to allow fast play/pause toggle via mpris. The 2 sec timer based update is too long.
Would it be OK to rename the function to calcCurrentPlayingDeck()?
There was a problem hiding this comment.
updateCurrentPlayingDeck() could also return the deck number/index. The getter should be read-only.
There was a problem hiding this comment.
I have reverted the this.
src/mixer/playerinfo.cpp
Outdated
| emit currentPlayingDeckChanged(maxDeck); | ||
| emit currentPlayingTrackChanged(getCurrentPlayingTrack()); | ||
| TrackPointer track; |
There was a problem hiding this comment.
This could become emit currentPlayingTrackChanged(getCurrentPlayingTrack()) if only getCurrentPlayingDeck() and getCurrentPlayingTrack() would be idempotent and work as expected.
daschuer
force-pushed
the
current_track_fix
branch
from
8370a5d
to
1add09b
Compare
|
I have forced pushed and removed the critical changes. |
|
Good idea to defer the changes that are not strictly necessary for this PR. LGTM Thank you! |
This is a bug that happens due to play changes before track is loaded.