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

Bugfix seek and play commands #738

Merged
merged 15 commits into from Oct 28, 2015
Merged

Bugfix seek and play commands #738

merged 15 commits into from Oct 28, 2015

Conversation

daschuer
Copy link
Member

@daschuer
Copy link
Member Author

https://bugs.launchpad.net/mixxx/+bug/1467057 is also fixed by this PR

@daschuer
Copy link
Member Author

The reported error is a Travis VM error.

This PR also fixes Bug https://bugs.launchpad.net/mixxx/+bug/1504643

ywwg
ywwg reviewed Oct 12, 2015
View reviewed changes
src/engine/enginebuffer.cpp

// Add SEEK_PHASE bit, if any
seekType |= static_cast<SeekRequests>(
m_iSeekPhaseQueued.fetchAndStoreRelease(SEEK_NONE));
Copy link
Member

Choose a reason for hiding this comment

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

this interface is now confusing because m_iSeekQueued could still be set to SEEK_PHASE
It looks like SEEK_PHASE should be removed, and m_iSeekPhaseQueued should only be set to true or false

@daschuer
Copy link
Member Author

m_iSeekPhaseQueued is now used as a boolean

@daschuer
Copy link
Member Author

Checks faild because of the famous Travis error:

No output has been received in the last 10 minutes, this potentially indicates a stalled build or something wrong with the build itself.

@daschuer
Copy link
Member Author

Done.

@daschuer
Copy link
Member Author

@ywwg: ready for merge?

@DJChloe
Copy link
Contributor

DJChloe commented Oct 27, 2015

Thank you for this big fix :)

@DJChloe
Copy link
Contributor

DJChloe commented Oct 27, 2015

Could be good to mark the bug reports as fixed :)

@daschuer
Copy link
Member Author

I have added fixes for some corner cases.
Please test and review again.
Thank you.

https://bugs.launchpad.net/mixxx/+bug/1510291

@DJChloe
Copy link
Contributor

DJChloe commented Oct 27, 2015

For this one, it's fixed :
https://bugs.launchpad.net/mixxx/+bug/1504503
👍

@DJChloe
Copy link
Contributor

DJChloe commented Oct 28, 2015

Do not consider this post, mistake

ywwg
ywwg reviewed Oct 28, 2015
View reviewed changes
src/engine/enginebuffer.cpp
@@ -401,12 +402,19 @@ void EngineBuffer::queueNewPlaypos(double newpos, enum SeekRequest seekType) {
// All seeks need to be done in the Engine thread so queue it up.
// Write the position before the seek type, to reduce a possible race
// condition effect
m_queuedPosition.setValue(newpos);
DEBUG_ASSERT_AND_HANDLE(seekType != SEEK_PHASE) {
// SEEK_PASE with a position is not supported
Copy link
Member

Choose a reason for hiding this comment

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

typo -> SEEK_PHASE

@ywwg
Copy link
Member

ywwg commented Oct 28, 2015

small typo, then LGTM

@daschuer
Copy link
Member Author

Thank you for review!

daschuer added a commit that referenced this pull request Oct 28, 2015
@daschuer
Merge pull request #738 from daschuer/gotoandplay
a723178 
Bugfix seek and play commands
@daschuer daschuer merged commit a723178 into mixxxdj:1.12 Oct 28, 2015
@daschuer daschuer deleted the gotoandplay branch January 8, 2016 16:38
@daschuer daschuer mentioned this pull request May 30, 2018
Merged
This was referenced Aug 22, 2022
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@daschuer @DJChloe @ywwg