Conversation
There was a problem hiding this comment.
I'd suggest moving MediaPlayer out of the library and into the executable: that will also get rid of the ImGui / gvdi dependency. The lib can then only link to capo and libxmp, and games/engines linking to it will be free to use their own windowing/rendering systems.
library/src/juke.cpp
Outdated
| m_source->play(); | ||
| m_status = m_source->is_playing() ? MediaStatus::playing : MediaStatus::stopped; | ||
| m_source->set_looping(looping); |
There was a problem hiding this comment.
- Call
set_looping()before callingplay(), just in case it finishes playing before you manage to set it to loop 😛 - IMO
m_statusis unnecessary, and risks going out of sync: it may be playing now but could have stopped in the next second
library/src/juke.cpp
Outdated
|
|
||
| void Jukebox::stop() { | ||
| m_source->stop(); | ||
| m_source->set_cursor({}); // seek to start |
There was a problem hiding this comment.
Keep in mind that this does nothing for XM streams.
library/src/juke.cpp
Outdated
| void Jukebox::update() { | ||
| if (m_status == MediaStatus::playing && !m_source->is_playing()) { m_status = MediaStatus::stopped; } | ||
| } |
There was a problem hiding this comment.
Getting rid of m_status (and instead just interpreting m_source->is_playing()) will also enable getting rid of this update() function entirely.
There was a problem hiding this comment.
yeah, agreed that status is kind of useless now. I was holding on to it just in the case of standard compressed audio format + tracking whether the stream is paused or stopped, but I'm just going to get rid of it.
| add_library(jukebox-app) | ||
| add_library(jukebox-app::lib ALIAS jukebox-app) |
There was a problem hiding this comment.
If this is also a library, is there no example executable anymore? What are we supposed to run after building / during development?
There was a problem hiding this comment.
the executable is in app/src
extract functionality into an API-friendly header file, and re-work
MediaPlayerto eliminate redundancy