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

When Jack is used, start / stop jack transport #4412

Closed
wants to merge 4 commits into from
Closed

Conversation

djfm
Copy link

@djfm djfm commented Jun 8, 2018

This allows lmms to function as a jack transport master.

Ideally there should be a switch to toggle between jack master / jack slave but this is already enough to do things like recording audio in ardour and have it sync'd with lmms easily.

@josh-audio
Copy link
Member

This appears to not be building on mingw, which I believe is Windows. Do you maybe need a preprocessor directive to prevent your logic from compiling in when Jack isn't present? This is my best guess without actually inspecting your code.

@josh-audio
Copy link
Member

Yep, Travis is only failing on Windows.

@josh-audio
Copy link
Member

Awesome, looks like the build is fixed. I'll let someone with more knowledge of JACK decide if this is good to merge.

@djfm
Copy link
Author

djfm commented Jun 10, 2018

As such the feature is pretty limited : it only starts and stops the jack transport when playback is started / stopped in LMMS. I think the next step would be to to also synchronize the play position when the song is stopped or the user skips to a random location in LMMS.

IMHO the feature as is already provides significant value because it makes recording audio with jack clients a lot easier, which is an important use case that is not handled by LMMS itself.

I'd like to go further and add support for the jack_transport_reposition API.

If anyone could share some LMMS knowledge about how to fill in a jack_position_t struct from LMMS timing info it would be very helpful.

I'm also struggling a bit to find a hook to tap into to detect when the play head position changed, I've considered Song::setPlayPos but it doesn't seem to be called when the play head skips, same for SampleTCO::playbackPositionChanged. Anybody knows what I could use?

@PhysSong
Copy link
Member

I'd like to go further and add support for the jack_transport_reposition API.

👍

If anyone could share some LMMS knowledge about how to fill in a jack_position_t struct from LMMS timing info it would be very helpful.

I guess these functions will be useful:

lmms/include/Song.h

Lines 105 to 108 in 2f19fa1

inline int getMilliseconds() const
{
return m_elapsedMilliSeconds;
}

lmms/include/Song.h

Lines 124 to 140 in 2f19fa1

inline int getBeat() const
{
return getPlayPos().getBeatWithinBar(m_timeSigModel);
}
// the remainder after bar and beat are removed
inline int getBeatTicks() const
{
return getPlayPos().getTickWithinBeat(m_timeSigModel);
}
inline int getTicks() const
{
return currentTick();
}
inline f_cnt_t getFrames() const
{
return currentFrame();
}

@djfm
Copy link
Author

djfm commented Jun 12, 2018

Thanks a lot! I'm still struggling to make the mapping, is a "Tact" the same as a "Bar"? I can't seem to find a way to get the current bar.

@rghvdberg
Copy link
Contributor

rghvdberg commented Jun 12, 2018

Tact is the same as bar.

And I really really really want this in lmms!

@djfm
Copy link
Author

djfm commented Jun 13, 2018

Thanks, yeah me too but it's proving to be harder than I thought, care to help ? :)

I've got this piece of code together to fill in the jack position struct:

jack_position_t pos;
pos.valid = jack_position_bits_t::JackPositionBBT;
pos.bar = m_song->currentTact() + 1;
pos.beat = m_song->getBeat() + 1;
pos.tick = m_song->getBeatTicks() + 1;
pos.bar_start_tick = m_song->getTicks();
pos.beats_per_bar = m_song->getTimeSigModel().numeratorModel().value();
pos.beat_type = m_song->getTimeSigModel().denominatorModel().value();
pos.ticks_per_beat = m_song->getPlayPos().ticksPerBeat(m_song->getTimeSigModel());
pos.beats_per_minute = m_song->getTempo();

and then I do

jack_transport_reposition(m_client, &pos);

but Ardour will just go to around bar 1 and loop...

Can you spot something obviously wrong in the mapping code?

@rghvdberg
Copy link
Contributor

Ok stupid noob question. Did you check if those functions actually return the correct bbt values?
By using either cout or qDebug()?

@djfm
Copy link
Author

djfm commented Jun 13, 2018

Yes I did, the values seem to make sense:

bar 3 beat 2 tick 13 ticks_per_beat 48 tempo 140
bar 4 beat 4 tick 13 ticks_per_beat 48 tempo 140
bar 5 beat 3 tick 37 ticks_per_beat 48 tempo 140

@djfm
Copy link
Author

djfm commented Jun 14, 2018

I'm getting closer, I had forgotten to set the "frame" field in the jack position object. The sync is now almost working, I can skip in lmms and it skips to what seems to be the correct position in ardour.

The problem is that I don't really know where to put this sync code. I think jack_transport_reposition should be called:

  • when moving the playhead in lmms by clicking on the timeline or otherwise, but not when playing normally
  • when the song is stopped or paused

At the moment I have the sync code in SongEditor::updatePosition, which has the side-effect of constantly stopping and restarting the jack transport, making the sync'ed clients useless.

I was hoping the Song::playbackPositionChanged signal would do the trick but it doesn't seem to be ever emitted... any ideas?

@djfm
Copy link
Author

djfm commented Jun 14, 2018

As a workaround for not finding a suitable place to hook into to adjust the transport position, I'm only running the sync code when the song is stopped. It's not ideal but usable.

@PhysSong
Copy link
Member

I guess you may use JACK timebase callback for this purpose. Setting the callback notify JACK that LMMS is working as transport master.
I also think it'd be better if users can turn on/off transport setting, too.

@PhysSong
Copy link
Member

@djfm Are you willing to continue this pull request? If not, some of us can continue working on this.

@PhysSong
Copy link
Member

PhysSong commented Jan 7, 2019

The new build failure is due to missing jack_transport_reposition in weak_libjack.def and weak_libjack.h. It will be nice if someone could create a PR against upstream, or I can do that no one is going to.

@lukas-w
Copy link
Member

lukas-w commented Mar 10, 2019

The new build failure is due to missing jack_transport_reposition

Can we use jack_transport_locate instead?

@firewall1110
Copy link
Contributor

Was not able to compile:

/home/wb/build/lmms_build/check_jackd_ss/lmms/src/3rdparty/qt5-x11embed/src/X11EmbedContainer.cpp:40:10: fatal error: QtCore/private/qobject_p.h: No such file or directory
 #include <QtCore/private/qobject_p.h>

Is this PR actual?

Is it make sense to try "insert this" against (for example) LMMS stable code base?

@PhysSong
Copy link
Member

PhysSong commented Nov 1, 2020

@firewall1110 It seems like you're missing Qt private headers for qtbase. Try searching packages for your distribution and install it.

@firewall1110
Copy link
Contributor

quote:"It will be nice if someone could create a PR against upstream, or I can do that no one is going to."
But it was mentioned ~21 month ago ...
May be this is done or in somebody plans or is not actual now.

P.S.
Start position: I had compiled stable and master + ~5 PR ...
(1) To satisfy CMake in this PR I have added +3 packages
(2) CMake download some code in configuration process and it seems that not find "one thing". So I "was forced" to -DWANT_CALF=OFF
(3) It possible, that I should install some more packages, but CMake not recognised this (packages absense).
Anyway this PR have problems with compilation (not only my building enviroppment)

This PR change only 6 files and time to manually apply this change to stable (this not download anything in configuration and "is stable" - not changing every week) may be the same as time to solve compilation problem

@PhysSong
Copy link
Member

PhysSong commented Nov 2, 2020

@firewall1110 Have you read the instruction for getting submodules?
https://github.com/LMMS/lmms/wiki/Accessing-git-repository
If you have any issues with compilation, please feel free to join and ask question on Discord.

@firewall1110
Copy link
Contributor

I "solved" compilation problem by manual adapting code against master:
pmaster.patch.gz

I am an one step to "create a PR against upstream" ...

But I do not see issue, that solves this code ...

@ycollet
Copy link
Contributor

ycollet commented Dec 6, 2020

I "rebased" the changes on the current master here:
https://github.com/ycollet/lmms/tree/jcak_master
I don't know how to push this change on this pull request yet ...

@firewall1110
Copy link
Contributor

It is good that @ycollet is about to work with this jackd integration with LMMS .
From my point of view more advanced jackd integration should be optional and not needed.
Plan may be :
(1) New PR should be created.
(2) This PR closed.
(3) New PR reviewed and merged.

P.S.
I will not work in this direction.

@ycollet
Copy link
Contributor

ycollet commented Dec 28, 2020

Thanks for your answer :)
For me a better jack integration (being able to use jack master / slave) should allow to use lmms with xjadeo.
The goal is to use lmms to write music in sync with a video.

@firewall1110
Copy link
Contributor

LMMS with xjadeo now can be used (pmaster.patch.gz , and (I hope) Your "rebased" this PR version).
(4) May be good "Ex.Sync." toggle in LMMS main window.
Are You able to develop this Yourself?

P.S.
I have only 1 (merged bugfix) PR and in seems as You "in wonder" with LMMS project management ...
("in wonder" not means "bad", but - "I not understand")

@firewall1110
Copy link
Contributor

This PR is not replaced yet ?
I am about to return ...

jackd_sync.mp4

I will wait about 10 days . After I will realize my plan myself ...

@ycollet
Copy link
Contributor

ycollet commented Jun 17, 2021

Excellent, I will just say GO !
Once your work is commited into LMMS, I have a lot of this to do with LMMS + Xjadeo !! :)

@firewall1110
Copy link
Contributor

firewall1110 commented Jun 18, 2021

So I should not wait 10 days but do work it 10 days …
My previous plan is realized in draft:

jackd_sync_v01.mp4

But:
legacy ugly solution: … dynamic_cast<AudioJack*> …
(1) ExSync is possible not only using AudioJack (in theory) and it is possible to have more “slave target”.
(2) AudioJack may be compiled in but not using right now , especially than audio driver change without LMMS restart will be implemented.
(3) jackd now may be used not only with AudioJack but by jackd back-end in some other drivers – may be ExSync can be used too ;
But right now ExSync is possible only with AudioJack and should be placed near and compiled with it, but not as AudioJack part … .
ExSync is placed and implemented in the possible for me now way but not the good way:
I want to place button here (Is there something like this in QT):
exsync_place

now repositionTransport(pos) is called on every GUI synchronization , but should only as reaction on needed GUI events; for xJadeo “slave” current solution is enough , but for “audio slave” sound will be corrupted;
I have to find connected slots for:
marked_points

@firewall1110
Copy link
Contributor

firewall1110 commented Jun 21, 2021

Previous plan is realized in draft except place for button – I plan Slave/Duplex support:
lmms_jackd_image06_21
Next plan:
(1) Enable Slave/Duplex mode (message receiving from jackd).
(2) Work around LMMS reaction to user interface in Slave mode.
When critical user event happens, one of two thing should be happened:
disconnect from jack-transport;
OR
first aromatically change to Duplex mode.
But may be allow strange variant , when LMMS will have “own life” to next jack-transport signal … But this will look like bug …
(3) Work around potential infinite message loop in Duplex mode (make sure not send back received messages).
(4) Work around currentFrame(); wrong result if user work in PianoRoll (when SongEditor position is not updated)
(5) Place common part to Song.cpp , but Jackd part to AudiioJack.cpp. But type definitions leave in AudioJack.h (when will be more external synchronization possibilities than AudioJack, type definitions should be moved to another place).
(6) Remove debugging lines , check build without JACK and prepare new PR that enables basic Jack Transport support and external synchronization experimental feature in draft.

@ycollet
Copy link
Contributor

ycollet commented Jun 21, 2021

Thanks a lot for the update !

@firewall1110
Copy link
Contributor

I just have opened PR #6066 so this should be closed ...

@PhysSong
Copy link
Member

Closing in favor of #6066.

@PhysSong PhysSong closed this Jun 25, 2021
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.

9 participants