Skip to content

Conversation

@ycollet
Copy link
Contributor

@ycollet ycollet commented May 1, 2018

I tried to backport the jack transport changes from https://github.com/gi0e5b06/lmms.git.
I compiles fine on my repo and it works.
Certainly some more changes will be required.
I performed some tests with xjadeo + lmms (jack slave) both controled by qjackctl and some times, lmms forgot to play some notes. I doesn't checked the processor load during the run of xjadeo + lmms.

@PhysSong
Copy link
Member

PhysSong commented May 2, 2018

This pull request contains many unnecessary and unwanted changes, and it crashes when I don't use JACK. Since the original branch was significantly diverged from mainstream, it will need many fixes.

@ycollet
Copy link
Contributor Author

ycollet commented May 2, 2018

I performed a diff betwwen the files which have changes related to jack transport.
On lmms I was on the master branch and on the other repo I was also on the master branch. I know that the other repo is checked out from 1.2.0rc3 but only the required changes are backported. So I don't understand why you say that the branch have diverged.
Could you have some more informations related to the crashes ?

@PhysSong
Copy link
Member

PhysSong commented May 2, 2018

So I don't understand why you say that the branch have diverged.

Many commits have been added into mainstream master since the branch is created. The downstream branch isn't well-maintained and has a number of unnecessary/unrelated changes.

but only the required changes are backported.

This pull request reverts large parts of #3711, which is probably undesired. There are some more issues(ex. formatting, redundant changes).

Could you have some more informations related to the crashes ?

You need to set audio interface to anything except JACK and press play button. The reason is: s_transport is set to NULL on initialization and has non-null value if and only if AudioJack sets a proper value.

After addressing these points, we still need code reviews and testings.

@PhysSong
Copy link
Member

I'll close this if it won't be cleaned up. I think clean (re-)implementation can be better than this

@ycollet ycollet closed this May 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants