will fix issue #523 JACK transport support with ExSync API draft

[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 .

[LmmsBot]

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

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)

:robot:

{"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):

https://user-images.githubusercontent.com/57725851/123448390-87594880-d5e3-11eb-8037-7c3378f0323a.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: https://github.com/LMMS/lmms/pull/6066#issuecomment-868652135.

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]

is similar to VST sync, but for JACK?

I partially understand "VST sync" (may be something used in AutoTune plugins), but it is similar.

could be given a nice generic transport/timeline info interface which would let VST, JACK, and also CLAP make use of it

ExSync.h/ExSync.cpp may be a "home" for such things, but as extension (VST, JACK, CLAP are plugins - not external processes) not as targets . The key difference : (target) mostly used one in one session; (plugins) mostly used more than one and in different formats in one session.

P.S. (1) For new LMMS contributors, I add line to src/core/CMakeLists.txt for src/core/ExSync.cpp any for me

Look at src/CMakeLists.txt

is good and useful hint. Thank You, @Rossmaxx ! (2) Now I need ~ 10 days to check PR usability (work as planned, may be some improvements, ... ) (3) I will not use macros black magic:

• view/controls code home is SongEditor.cpp and should not be "teleport-ed" to ExSync.h;
• signal "wires" may be "teleport-ed" but why change #ifdef ... #end with macros?

(4) More "quickly going over the code" should be useful even if someone "don't understand what the feature is". May be last variant is better - "stress test" for code readability.

[firewall1110]

Now I am trying to use this feature and see, what should add/change. I found: (1) New buttons should not take keyboard focus. (2) Should be good another "wire" to send position in Master-mode when user change tempo Edited (2024.08.01): existed one is used (no more "wire" added). This wire is ugly (is polling wire), but useful.

[firewall1110]

So 10 days are over and always will be something to improve - so I should be ready to code review ...

Some comments for code reviewer:

• pinned to jack to reduce complexity and because now does not make sense without jack (so (dis)connection provide jack audio back-end, but in future LMMS_HAVE_EXSYNC or something similar should be compilation option and (dis)connection should be here too - but not right now);
• C++ dialect is "C with Classes", and Classes used only to call external code, used "old good" C static for encapsulation (names taken from old LMMS coding conventions, now removed - some "cool hot heads" plan to use only QT12++ C+++++... style);
• with //ExSync marked all changes in files except ExSync.h/ExSync.cpp;
• some hints for future are in comments (but comments must be cleaned);
• some code really not used but I keep it not commented (as only hint for future): if any "cool refactor-er" will change public interface - will be forced to make changes in this code too;
• there are some "logic duplication", but if be optimized - than code logic became "hybrid" : keep unchanged - may be useful to some state checks in future;
• there are no any kind memory (de)allocation so "old" pointer is always valid (not freed) - no need use mutex.

[messmerd]

Is the "ExSync API" something you've designed, or is there API documentation for it somewhere on the internet that I can read?

[messmerd]

I partially understand "VST sync" (may be something used in AutoTune plugins), but it is similar.

VST sync provides LMMS transport info to VST plugins similar to how this PR provides transport info to JACK.

It does not allow VST plugins control over the LMMS transport as this PR does, but if we refactored LMMS's messy transport implementation to provide a generic transport provider/controller, we could implement VST sync and this "ExSync" in terms of that quite easily. Then we wouldn't have to perform shotgun surgery by modifying multiple places in several files whenever some new LMMS feature needs access to transport information.

I'm not saying this PR has to be the PR that cleans up LMMS's transport mess, but it also shouldn't introduce any new messes.

As it stands, this PR is pretty messy and indecipherable. The LMMS coding conventions are not followed at all, everything is written in C style, and it just feels unorganized and confusing in my opinion.

It will need to be cleaned up before it can be merged.

[firewall1110]

@messmerd ! Thank You for comments!

Quote: "Is the "ExSync API" something you've designed... ?"

Yes, it is my designed API: (title) "... with ExSync API draft". This API parts are: From ExSync.h (for controlling driver and send position to external aplication): struct SongExtendedPos - information - be sent; struct ExSyncHandler - interface for implementation (the same as abstract class, but without extra C++ complexity ); exSyncGetHandler() - function, to get current working object (the same as interface implementation object).

In core/ExSync.cpp (for using in driver implementation: protected interface): struct ExSyncCallbacks - used mostly to control LMMS from external application (the same as abstract class, ...).

P.S.1 Functions after exSyncGetHandler() are not part off API, but used to integrate with other parts of LMMS (I call tham "wires"). This functions should be documented ... Without documentation it is "pretty messy and indecipherable." P.S.2 I am concentrated only in one task: allow work with xjadeo , I should not write "... with ExSync API draft", in the PR header. But what is done is done and I have to implement this "ExSync API" properly ...

[firewall1110]

Quote: "VST sync provides LMMS transport info to VST plugins similar to how this PR provides transport info to JACK."

In this case Vestige code should be able somehow to implement struct ExSyncHandler and connect it to ExSync.o ... This is how "ExSync API" can be used ... It is TODO ...

Quote: "The LMMS coding conventions are not followed at all" I not agree , but it does not matter ... : "ExSync API" should be useful for LMMS team (but now it is not useful) and only after this any discussion about coding conventions make sense ...

Quote: "everything is written in C style"

Is C style not allowed by LMMS coding conventions? I think - no, but @messmerd - Your are LMMS member and Your opinion must be respected. Question is about - do I understand You ?

P.S. Adequate alternative to C - style struct with pointers to functions should be abstract class with abstract static methods ... C++ does not allow such things (am I wrong?) . Abstract class (Java interface analog) with implementation allow multiple (for example) lmms transport driver objects - so than additional "design pattern" must be used. It is additional complexity.

But before I should prove that such kind of construction is useful at all (and only after that find better implementation) - but I am not sure...

[firewall1110]

So another 10 days ... (Edited:) Stopped as it seems not more actual

My plan 1-st step is: (Edited 2024.08.14) This plan is bad : my intuition bring me better one, but work anyway stopped [ExSync.h] (1) Transform struct ExSyncHandler to abstract class. (2) Remove struct ExSyncCallbacks at all - not needed. (3) Document and may be change names for "wires" [all after exSyncGetHandler() declaration] (4) Provide function, that returns current LMMS transport state (5) Provide functions, that allows to add/remove external ExSyncHandler as an extension.
[ExSync.cpp] (5) abstract class implementation for jack transport will be embedded in core/ExSync.cpp (I think- declaration not in header file) in this 1-st step. (But anyway nobody should use it in external code, but edit only if public interface changes) (7) implement new provided by ExSync.h functions (see (4) and (5) ).

After this code will enter the state, covered by LMMS coding conventions, but still will be in "C with classes" style.

P.S. Stop me if I am wrong ... Now I am about add additional complexity, not needed for communication with Jack.

[Rossmaxx]

You know what would be nice, develop the "ExSync" API on your own and we can submodule it. It doesn't need to be this coupled into LMMS afaik. Or does it use any LMMS specific features?

[firewall1110]

Quote: "Or does it use any LMMS specific features?" (@Rossmaxx)

Much worse ( ;) ) ... It should be (became) LMMS ExSync API.

But seriously : it is adapter from LMMS "transport+" (and I think - it is LMMS specific feature) to another application : now only to jackd.

P.S. If somebody think that I am somehow hurt - no. I had feeling, that such code style will not be accepted here. And my english is not as good as should be ...

[Rossmaxx]

Oh i see. @DomClark does have an IPC refactor in mind, which might end help with jack transport too, instead of using another highly coupled API, which led us into this mess in the first place.

@firewall1110 sorry to say, but your implementation looks way too messy. Better to have a cleaner implementation with better interfaces. Not closing for now, as there's still scope for a jack transport implementation.

[firewall1110]

@Rossmaxx , so I should better stop and wait?

[Rossmaxx]

I would ask dom for that, I said it like that because introducing a new API on your own will very quickly lead to an unmaintainable mess, which is what led to the VST code being so messy. And your PR doesn't really look either.

[Rossmaxx]

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, so I'll leave a better explanation of my idea in #1467

[firewall1110]

Quote: "Not closing for now, as there's still scope for a jack transport implementation." And then: Quote: " It's out of scope, ..." (@Rossmaxx - LMMS maintainer )

So this PR is not more actual for LMMS team.

10 days alert to close this PR. (Edited 2024.08.15) Canceled

[tresf]

Quote: "Not closing for now, as there's still scope for a jack transport implementation." And then: Quote: " It's out of scope, ..." (@Rossmaxx - LMMS maintainer )

So this PR is not more actual for LMMS team.

10 days alert to close this PR.

Please do not close this PR. <3

[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?"