JACK transport support

[firewall1110]

(1) All changes make sense only if JACK support is compiled, but some parts are programmed as be placed in ExSinc.h/ExSync.cpp
(2) I use C style static functions and variables and violate LMMS coding style for them by prefix cs_ - this guarantee name collision never be happen.
(3) Introducing completely new thing can't be done perfect:

• ExSync API is draft assumed to be enough for (for example) MTC synchronization, ... ;
• conversions ticks from/to frames in Song class assume constant tempo so projects with tempo automation not supported yet;
  (4) When audio interface is not JACK , than ExSync button can't be activated and than clicked "Master" be cleaned. (GUI part should be improved but I hope it is not problem).
  (5) When ExSync is not activated, in Slave and Duplex mode external messages are received but LMMS not react;
  (6) When ExSync is activated LMMS react to external messages in Slave and Duplex mode , and send messages in Master and Duplex mode.
  (7) ExSync activation/deactivation don't perform any synchronization or driver activation/inactivation : this seems as bug, but allow more freedom by playing with ExSync buttons .

Closes #523

[LmmsBot]

🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩

Windows

• Windows 64-bit: lmms-1.3.0-alpha.1.121%2Bg217a98b0e-mingw-win64.exe (build link)
• Windows 32-bit: lmms-1.3.0-alpha.1.121%2Bg217a98b0e-mingw-win32.exe (build link)
• Windows 32-bit: lmms-1.3.0-alpha-msvc2017-win32.exe (build link)
• Windows 64-bit: lmms-1.3.0-alpha-msvc2017-win64.exe (build link)

Linux

• Linux (AppImage): lmms-1.3.0-alpha.1.121%2Bg217a98b-linux-x86_64.AppImage (build link)

macOS

• macOS : lmms-1.3.0-alpha.1.121%2Bg217a98b0e-mac10.14.dmg (build link)

🤖

{"platform_name_to_artifacts": {"Windows": [{"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://14116-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.121%2Bg217a98b0e-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/14116?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://14120-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.121%2Bg217a98b0e-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/14120?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/8bom1aqi29cagkvy/artifacts/build/lmms-1.3.0-alpha-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/39758894"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/v43q6m6okcfdeaec/artifacts/build/lmms-1.3.0-alpha-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/39758894"}], "Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://14117-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.121%2Bg217a98b-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/14117?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://14119-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.121%2Bg217a98b0e-mac10.14.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/14119?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "878a14ed64bf7d7d2c5c4dbafbea5c943cfc9892"}

[firewall1110]

Some files for testers in zip:
jackdexsync_t.zip
it is hard to change position in x264 video file so XJadeo delays position change but catch it synchronized.
Video is a little bit late - 1 or 2 starting frames should be deleted.

[firewall1110]

This PR replace #4412 PR .
But #4412 is mentioned in Issue #5592 .

[firewall1110]

Some video illustration (on the start : ALSA ; than JACK):

jackd_sync_v02_red.mp4

And finally main PR goals:
(1) Make possible to use LMMS with Xjadeo .
(2) Make possible to improve and develop independently (after PR merging in master):
external synchronization "drivers" (Song.cpp is ready to send and receive start/pause and reposition);
external synchronization API (slave mode now is only frame based - enough and natural for jackd);
Song.cpp functionality (to support synchronization with tempo automation);
external synchronization GUI.
For the last purpose I write "future hint" comments - are not TODO right now, only after for example MTC synchronization will be implemented (but not in PR with JACK synchronization goal).
There for I push button "Ready for review" ...

[firewall1110]

Is LMMS developers command core interested in this PR?
May be simply delete ?...

[Rossmaxx]

Sorry for the late response. We ust haven't paid any attention to this PR, among a few others, and idk who should be reviewing this, but from my instinct, i would like to invite @michaelgregorius

[Rossmaxx]

Don't delete PRs just because they are stale. Someone else might work on top of it.

Also, tbh i don't understand what the feature is.

[Rossmaxx]

Forgot to mention this too, you can add fixes #523 and it'll automatically close on merge.

[michaelgregorius]

Also, tbh i don't understand what the feature is.

It synchronizes LMMS' transport with the JACK transport and vice versa. There's a demonstration video by @firewall1110 in this comment: #6066 (comment).

IMO the following needs to be done before this PR is reviewed:

1. The @LMMS/developers have to decide if the feature is wanted and if they want to maintain it.
2. The merge conflicts must be resolved.
3. The code must/can be reviewed.

My first gut feeling after quickly going over the code is that it is a bit all over the place because there is no abstraction of the synchronization feature, i.e. there is no interface that LMMS defines which describes the sync features that are supported by LMMS.

An abstraction would be useful if other audio back ends also support synchronization. In that case there would be an implementation of the interface for JACK and for all other back ends that support it. It would also keep the JACK related code mostly in one place and not scattered with ifdefs over several files.

[firewall1110]

I understand :I will wait for @LMMS/people ( @LMMS/developers make wrong link) decision.

About my implementation:
A:(1) synchronizes by time (only one of possible variants useful to xjadeo);
A:(2) audio back ends will not support synchronization - synchronization is not audio back end function;
[xjadeo supported synchronizations: MTC, LTC, JACK transport; only MTC via portmidi is on Windows/MacOS]
===>
A:(3) Jack is 3 in 1:

• audio back end (partially implemented in LMMS: no recording, no multiple playback (bus));
• MIDI back end (implemented but not so well as ALSA (provide track names, JACK - not))
• multiple audio&MIDI application "integration" and synchronization "service" [this PR is one small step in implementation]

A:(4) every IPC type can be used for synchronization - LMMS can introduce (may be - should) LMMS synchronization protocol (for example: named shared memory with struct {unsigned char status; unsigned long time /* in usec */ ; })

B:(1) I assumed new feature implementation with small compact steps: PR : <1:JackTransport> <2:PortMidi MTC> <3:ShaMem IPC> <4:ExSync.h + code refactoring> ... This is 1-st step <1:JackTransport> abstraction is <4:ExSync.h + code refactoring>
But how I see LMMS style is long evolving PR ... May be evolving several years ...
B:(2) Interface make sense if we have more than one interface implementations. Now we have no interface implementations, this PR introduce one.
Is Your logic: implement maximally complete external synchronization with all needed abstractions ?

But once more "The @LMMS/developers have to decide if the feature is wanted and if they want to maintain it."

[firewall1110]

Now I think about " keep the ExSync related code mostly in one place" but with the same goal : nothing should be added in compiled code if ExSync not used (now ExSync is pinned to JACK: so - JACK not compiled in).
Plan:

• all code move to ExSync.h and core/ExSync.cpp (this feature have no connection with audio, but right now using only JACK transport "channel")
• ExSync.h provides (if JACK not compiled in) some empty inline functions (at least - macros) so minimize ifdef usage
• keep all lines added in other files commented with //ExSync - feature is new and so extension developers easily can find related lines (with grep -i "//ExSync" -R ./)

So questions are:
How to add core/ExSync.cpp to project? [see other opened PR for hint]
Is core/ExSync.cpp right name and place for file?
P.S.
In this PR I finished previous author work: should be good to mention previous author, but I can not find his closed PR (edited: author is @djfm - link to closed PR is at this PR start).
In this closed PR I had discussion with LMMS member so " if the feature is wanted " probably "yes" (but this happened 3 years ago )
So I wait for answer: " if the feature is wanted " now - after 3 years.

[Rossmaxx]

How to add core/ExSync.cpp to project?

Look at src/CMakeLists.txt

[messmerd]

If I understand this correctly, this is similar to VST sync, but for JACK?

Maybe this could be given a nice generic transport/timeline info interface which would let VST, JACK, and also CLAP make use of it. Then we wouldn't need a bunch of different code all performing similar tasks patched into Song.cpp, TimeLineWidget.cpp, and other files.

[firewall1110]

@tresf :
(1) watching from the side LMMS project "evolution" I make conclusion that one of the serious project management problems is old stalled PR-s. I do not want my PR near the last page - I want it in 1-st , accept 2-nd.
(2) now I see this PR goal mistake (I should not mention any kind of new API)
(3) every PR (active or not active) make some thing marked as "work in progress", I have to free way for another developer - may be new one will suggest better implementation;
(4) it is not initially my PR

P.S.
I think: "Are my made changes enough to give me rights change ExSync.cpp and ExSync.h licence to Public Domain without agreement with previous author?"

[michaelgregorius]

P.S. I think: "Are my made changes enough to give me rights change ExSync.cpp and ExSync.h licence to Public Domain without agreement with previous author?"

Have you copied ExSync.cpp and ExSync.h from somewhere and adjusted them? I am not a layer but I can imagine that it could be potentially problematic if they are now added to this GPL2 repository with the header that they have. Depending on how much you have copied it might not be okay to add it here without consent from other authors. The situation might be different if it is based on some non-licensed example code for the API.

[tresf]

I do not want my PR near the last page - I want it in 1-st , accept 2-nd.

We do too! <3

(4) it is not initially my PR

Understood, this was not obvious. Please edit your original description to link to the PR which this supercedes.

I think: "Are my made changes enough to give me rights change ExSync.cpp and ExSync.h licence to Public Domain without agreement with previous author?"

Generally speaking, no, never do this. The original PR's copyright information and license must be kept. In some edge-case instances, a license can change but this is a bit off-topic here.

The reason I asked to NOT close this PR is because this comment suggests that you were prepared to improve it: #6066 (comment). If this is no longer true, the team will respect your wish to close this PR.

[michaelgregorius]

[...] (1) watching from the side LMMS project "evolution" I make conclusion that one of the serious project management problems is old stalled PR-s. I do not want my PR near the last page - I want it in 1-st , accept 2-nd.

The problem is that there are very few developers and I guess many of them also work day jobs. Most/all developers work on this project in their spare time. This spare time is scarce and therefore it can take time until a PR is reviewed. One way to deal with this is to directly address people for a code review or to remind them if they have started one and potential changes have been incorporated with no progress afterwards. It's definitively not ideal and a hassle but that's reality.

On the other hand, it's not like your code did not get any review or attention. It rather seems that you are unhappy with the results?

(2) now I see this PR goal mistake (I should not mention any kind of new API) (3)

It should be okay to add new APIs. But they have to be discussed and worked out. It must be defined how LMMS interfaces with an API and vice versa. And if there is more than one potential implementation (as is the case for this PR) then this must be abstracted with some kind of interface. Doing so ensures that the actual implementations are confined in a contained space and not spread all over the code. It's what I have alluded to in my comment here: #6066 (comment)

[firewall1110]

Quote: "It rather seems that you are unhappy with the results?"
But this is no reason to close PR.

I asked question: "if the feature is wanted"

I had no direct answer so I continue this on my own risk.

But this:
"That said, I've got a feel at times that jack, being feature rich, has lots of complex features, implementing everything might mess up. It's out of scope" @Rossmaxx
I was interpreted as direct answer for "if the feature is wanted" : Direct answer "No, feature is not wanted"

It seems I simply understand this wrong

[firewall1110]

It seem's that something happens with MinGW build system ...

[PhysSong]

It seem's that something happens with MinGW build system ...

You have two solutions at least.

• Do not include JACK headers in ExSync.h, which may requires some changes in ExSync.h and ExSync.cpp
• Try moving #include <windows.h> below other headers in RemotePlugin.cpp (untested)

[michaelgregorius]

By the way, if ExSync is intended to mean "external sync" then it should be called ExternalSync or in this case perhaps rather ExternalSyncJack. The current name sounds like something that previously was a sync but now is no more.

[firewall1110]

By the way, if ExSync is intended to mean "external sync" then it should be called ExternalSync

You are right, @michaelgregorius , I will change this in next commit.

Do not include JACK headers in ExSync.h

Jack header should not be in ExSync.h : it is only made , using jack API.
@PhysSong , I will change this in next commit.
(Removing:
void syncJackd(jack_client_t* client);
from header, and including
extern void syncJackd(jack_client_t* client);
in AudioJack.cpp, just before
void AudioJack::startProcessing()

(+ moving JACK headers from ExSync.h to ExSync.cpp)
)

@michaelgregorius ! @PhysSong ! Thank You for help!

[firewall1110]

New plan in this PR

(1) Now (in 1-st iteration) bad plan (3) Document and may be change names for "wires" [all after exSyncGetHandler() declaration] is completed.

Next steps:
(2) New SyncHandle (using C-style structure - de facto C++ class with virtual static methods) with description.
(3) SyncHandle for LMMS (to get/control state).
(4) Function to add/remove extension
(5) Abstract class for targets (targets - extension that use LMMS SyncHandle to control LMMS in Slave and Duplex mode) [Edited: use renames Master->Leader, Slave->Follower , using @Spekular recommendation ]
(6) Function to add target
(7) JackTransport target implementation, hidden in ExternalSync.cpp

All 1-st iteration done in ExternalSync.h/ExternalSync.cpp

P.S.
But in branch, not "exported" to PR I implement JackTransport without any ExSync API:

• files renamed to JackTransport.h , core/JackTransport.cpp ;
• all API part removed (only SyncCtl and SyncHook);
• JaclTransport implementation using class with static methods, but declaration in core/JackTransport.cpp (not shown in header) and only frames are used.

If I by mistake add new PR - than "This is fate".

[Spekular]

(5) Abstract class for targets (targets - extension that use LMMS SyncHandle to control LMMS in Slave and Duplex mode)

Can we use leader-follower terminology instead of master-slave here? Should be less effort to do so from the start and not worry about having to rename it later. If third parties use master-slave, we could add a small hint text in the UI, but stick to leader-follower internally.

[firewall1110]

I am working on it but only locally (and not so fast, as should be ...):

Working on "implement JackTransport without any ExSync API" I started with last master and find some strange change, that impact work in Follower mode (pulse() is not called anymore).

I want 1-st do cleaning code ignoring this change, and only than

• find another place for pulse();
• find a way, how to implement all without pulse()

(I need to read Jack API once more - may be find another way to catch Jack Transport stop event. )

[firewall1110]

I simply make this active again.
All is working - so can be improved after merging, but I not pretend to be this PR merged ...

The LMMS coding conventions are not followed at all

I hope that now (not followed at all) became (followed only partially) : so very fast Coding-Conventions review should be helpful for me .

[firewall1110]

Next steps:
(1) Extension management
(2) make ExternalSyncTimer public
(3) actualize Coding-Conventions
(4) now some events are "hooked" on GUI level, but should only on song level
(5) some starting preparations for MIDI in/out clock sync

Future plan:
(1) Reorganize code and change UI to be ready manage not only jack transport external synchronization
(but also MIDI in/out clock sync, named shared memory sync, ... )
(2) Renew pull request (make new without obsolete comments, may be also one more, that contains only jackd related part; and than close this pull request without merging)

Also:
Make xjadeo build for MS Windows with jack transport synchronization enabled.

P.S.
This is PR is low priority task (before new alpha pre release made).

[firewall1110]

If title now is " JACK transport support" , than it is simply done ...
Only now contains a lot off additional not needed code ...

may be also one more, that contains only jackd related part;

The branch:
git clone https://github.com/firewall1110/lmms.git -b jacktransport is more adequate to this title ...

But I not protest - OK ...