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.

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

locking a mutex from the audio thread when reading cues #11282

Closed
daschuer opened this issue Feb 17, 2023 · 6 comments
Closed

locking a mutex from the audio thread when reading cues #11282

daschuer opened this issue Feb 17, 2023 · 6 comments

Comments

@daschuer
Copy link
Member

Bug Description

This is a regression from #4771
The used mutexes are here:

const auto lock = lockMutex(&m_mutex);

auto locked = lockMutex(&m_qMutex);

Version

2.4-alpha

OS

all

@Swiftb0y
Copy link
Member

If the regression is really from #4771 then it seems like the problem lies in the findCueByType here:

TrackPointer pLoadedTrack = m_pLoadedTrack;
if (pLoadedTrack) {
CuePointer pN60dBSound =
pLoadedTrack->findCueByType(mixxx::CueType::N60dBSound);
if (pN60dBSound) {
const mixxx::audio::FramePos frame = pN60dBSound->getPosition();
appendCueHint(pHintList, frame, Hint::Type::FirstSound);
}
}

We could either revert it as a quick fix or put it in somewhere that is not called from the audio thread (CueControl::trackLoaded maybe?). Why is CueControl::hintReader called from the audio thread anyways?

@daschuer
Copy link
Member Author

We "just" need to introduce a control object like for the other cue points.

Why is CueControl::hintReader called from the audio thread anyways?

Sure, there is room for optimization ...

@Swiftb0y
Copy link
Member

Swiftb0y commented Feb 20, 2023

We "just" need to introduce a control object like for the other cue points.

I highly doubt thats the best solution. What does the current bug look like? Audio buffer underruns because of priority inversion?

@daschuer
Copy link
Member Author

I have not noticed that as a bug, it was a catch during debuging, but this may cause a buffer underrun whenever setting a cue point in a playing track.

I highly doubt thats the best solution.

Yes, It is sufficient to cache the position value in a
ControlValueAtomic<double> m_n60dBSoundStartPosition;

@JoergAtGithub
Copy link
Member

@daschuer Can we close this?

@JoergAtGithub JoergAtGithub added this to the 2.4.0 milestone Jun 27, 2023
@daschuer
Copy link
Member Author

Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants