search

drobilla / jalv

A simple fully-featured host for LV2 plugins
http://drobilla.net/software/jalv
ISC License
54 stars 19 forks source link

JALV as internal JACK client #2

Closed twischer-adit closed 5 years ago

twischer-adit commented 7 years ago

With this patch JALV can be executed in the context of the JACK process to increase the performance. We will also upstream a patch for JACK2 to execute all internal clients in the same thread to reduce the synchronization overhead.

twischer-adit commented 7 years ago

@drobilla I have fixed your findings within additional commits. If you are fine with the changes I will squash same into the corresponding commits.

twischer-adit commented 7 years ago

Should I change anything else? Could I do anything to speed up the merge?

drobilla commented 7 years ago

Sorry, haven't been paying attention. I'll take a look

drobilla commented 6 years ago

Apologies again, busy lives make crap maintainers :sweat_smile:

I'll try to get this merged this week.

twischer-adit commented 6 years ago

@drobilla

Apologies again, busy lives make crap maintainers sweat_smile I'll try to get this merged this week.

I have already started to rebase it. So you do not need to spend the effort again. I will try to push it till beginning of next week.

drobilla commented 6 years ago

@twischer-adit Awesome, thanks.

twischer-adit commented 6 years ago

@drobilla

Feel free to give any comments. I have tested it on Linux ARM64 with JACK.

drobilla commented 6 years ago

Okay. I started trying to go through it and chop up the history a bit, but it was quite a lot so I went back to popping the other PRs off the stack first. I'll get back to this one by tomorrow.

twischer-adit commented 6 years ago

Unfortunately I missed one new commit when rebasing. This commit fixes an existing issue.

If you like I can also squash this commit into https://github.com/drobilla/jalv/pull/2/commits/77a319dc807b995bb5d48405eb6da54ad2e8d706

drobilla commented 6 years ago

n/p, I picked out some stuff manually and rebased it on master.

Unfortunately that's all the time I have for today, though. My only significant problem (well, history nitpick) is the static Jalv thing, followed by a really noisy commit that removes it anyway. I plan to separate out the part that changes jalv from a value to a pointer (so all the . to -> translation) to isolate this from the significant changes tomorrow, but if you feel like a mindless mechanical task, feel free to beat me to it.

twischer-adit commented 6 years ago

@drobilla

I plan to separate out the part that changes jalv from a value to a pointer (so all the . to -> translation) to isolate this from the significant changes

The diff of jalv.c in https://github.com/drobilla/jalv/pull/2/commits/2e15a735c351908fe929edcd5e707110027e2dbf does only contain these changes. I do not get what you want to split further?

drobilla commented 6 years ago

aaaaaaaand I botched master anyway. Sigh. Nevermind, I'll just get through it.

drobilla commented 6 years ago

I ended up more or less completely rewriting history. The end result should be more or less the same, and works for me (just testing on desktop). How's that look to you?

twischer-adit commented 6 years ago

Call jalv_close() when jalv_backend_init() fails in jalv_open()

twischer-adit commented 6 years ago

Beside my comments it is compiling and working fine as Internal JACK client on ARM64.

drobilla commented 6 years ago

Ideally it should be safe to call and clean up be correct in all the error cases, sure. That seems like it might be pretty tedious, but if you want to give it a shot, go for it. If doing this totally correctly isn't feasible, personally I'd be fine with merging it as long as it doesn't leak or especially crash in pretty common scenarios, it doesn't need to be perfect right away.

twischer-adit commented 6 years ago

@drobilla

personally I'd be fine with merging it as long as it doesn't leak or especially crash in pretty common scenarios, it doesn't need to be perfect right away.

Okay than go for a merge and I will provide additional patches in a separate PR

twischer-adit commented 6 years ago

@drobilla

Ideally it should be safe to call and clean up be correct in all the error cases, sure. That seems like it might be pretty tedious, but if you want to give it a shot, go for it.

I have added the required changes (https://github.com/drobilla/jalv/pull/2/commits/a68de19de544b9a83e953466ff78c52739d0d34a) to avoid segmentation failures in case of an error case.

It would be great if you could merge it soon.