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

Fix hang on exit with SDL2 audio backend #6070

Closed
wants to merge 13 commits into from

Conversation

firewall1110
Copy link
Contributor

@firewall1110 firewall1110 commented Jun 30, 2021

Now dangerous in capture callback locking and gui signaling functions are disabled if not recording audio.
Also capture callback is not enabled on startup, but startCapture(); stopCapture(); functions provided.
Also some information provided if capture callback is not used properly:

  • capture callback called when devise is stopped (now only in rendering process);
  • startCapture(); stopCapture(); are called when capture is not implemented or not implemented properly .

Fixes #4668.

@LmmsBot
Copy link

LmmsBot commented Jun 30, 2021

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

macOS

Linux

Windows

🤖
{"platform_name_to_artifacts": {"macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://14205-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.125%2Bg362d97095-mac10.14.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/14205?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://14203-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.125%2Bg362d970-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/14203?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://14201-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.125%2Bg362d97095-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/14201?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://14202-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.125%2Bg362d97095-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/14202?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/wifnajdgtw3ivs6o/artifacts/build/lmms-1.3.0-alpha-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/39848994"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/avkkfebf5va70n75/artifacts/build/lmms-1.3.0-alpha-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/39848994"}]}, "commit_sha": "de92f811b87950e82cb69e470de80e880c7b84be"}

@PhysSong PhysSong changed the title Capture : will fix issue #4668 Fix hang on exit with SDL2 audio backend Jul 2, 2023
@firewall1110
Copy link
Contributor Author

It seems, that always right is capture choice, except GUI lines in AudioSdl.cpp line ~350, where right is master choice (capture hold some lines removed by refactoring ). But I am not sure ...

P.S. I will try to rebase locally on my computer, resolve as I mention before, and than ... do the same here ?

@firewall1110
Copy link
Contributor Author

This #6070 and #5990
May be have hidden conflict:
now audio call back is always active, but after #6070 must be activated before recording (and turned off after)
P.S. It seems, that #5990 will be merged to master before this, so conflict should be resolved here

@michaelgregorius
Copy link
Contributor

It seems, that always right is capture choice, except GUI lines in AudioSdl.cpp line ~350, where right is master choice (capture hold some lines removed by refactoring ). But I am not sure ...

P.S. I will try to rebase locally on my computer, resolve as I mention before, and than ... do the same here ?

@firewall1110, to prevent a force push the best course of action would be:

  1. Merge master into your capture branch.
  2. Fix the merge conflicts. I have just checked and they should not be too big. The main change is the introduction of the SampleFrame class which is now used instead of the using/typedef that was used before.
  3. Commit the merge into your branch.
  4. Push the branch.

@firewall1110
Copy link
Contributor Author

firewall1110 commented Jul 23, 2024

This PR should be deleted (or continued with different goal):
can not fix bug already fixed (by LMMS initialization process correct order):
SDL2 audio backend never hung on current master.

[see my "experiment": Program not closing properly #7385 ]

@firewall1110
Copy link
Contributor Author

firewall1110 commented Jul 30, 2024

About different goal:
sdl_settings
but should be like this:
soundio
but with:

  • Input:
  • Output:

Edited (2024.08.01): Is implemented, but not merged ... I'll think about another goal ...

@michaelgregorius
Copy link
Contributor

About different goal: sdl_settings but should be like this: soundio but with:

* Input:

* Output:

@firewall1110, this is how it should ideally look for all audio devices indeed. This has already been implemented for the SDL back end via the issue #5990. Unfortunately, it is mixed with something that might not be merged for a long time. Perhaps it will make sense to cherry-pick the relevant commits and to open an individual pull request.

See also these comments for more information: #5990 (comment), #5990 (comment)

@firewall1110
Copy link
Contributor Author

@michaelgregorius
Quote: "Perhaps it will make sense to cherry-pick the relevant commits and to open an individual pull request."
How I understand - it is Yours commit: You should do this.
P.S.
(1) 3 years ago I had idea how to implement audio monitoring, input changing and simple recording, but than give priority to #5990 author. I am "shocked" - why #5990 is not in master yet ... I made video to give hint to author merge my PR to his testing copy - it is difficult to develop recording without working driver with input support ...
(2) My opinion is that such "brand new" feature is too big for one PR: #5990 should be "splinted" ...

@michaelgregorius
Copy link
Contributor

@michaelgregorius Quote: "Perhaps it will make sense to cherry-pick the relevant commits and to open an individual pull request." How I understand - it is Yours commit: You should do this.

Yes, that's how I meant it.

P.S. (1) 3 years ago I had idea how to implement audio monitoring, input changing and simple recording, but than give priority to #5990 author. I am "shocked" - why #5990 is not in master yet ... I made video to give hint to author merge my PR to his testing copy - it is difficult to develop recording without working driver with input support ... (2) My opinion is that such "brand new" feature is too big for one PR: #5990 should be "splinted" ...

Perhaps it would indeed be better to work incrementally in smaller steps towards that goal.

@michaelgregorius
Copy link
Contributor

[...] Perhaps it will make sense to cherry-pick the relevant commits and to open an individual pull request.

Done via pull request #7421.

@firewall1110
Copy link
Contributor Author

firewall1110 commented Aug 4, 2024

About another goal: Audio recording in alternative way.
Quote:"via the issue #5990. Unfortunately, it is mixed with something that might not be merged for a long time."[@michaelgregorius]
Strategy:
(1) Simple audio input level monitoring and recording with processing format somewhere, there file can be easily found and included in sound track part (clip). This is custom widget placed in main application window (near oscilloscope). Recording may be started/stopped by mouse click, or may be pinned to song play start/stop. (This PR another goal.)
(2) Implement some kind session audio pool context to easy place samples to sample track with context menu (now lmms don't have any context menu on empty track - only on part; sometimes we have Paste). New recordings automatically be placed in this context menu. (These feature is completely independent and may be developed right now, if not already done, but not merged)
(3) Improve managing recordings and project audio files: folder and name and format for recording files, easy file renaming(, format changing, normalization,....) and moving, algorithm to find missing audio. In this stage may be implemented audio embedding in project file (easy can be done if using compressing format - compress project xml file and audio files together in one archive). (Also can be started right now - but only without recording managing, but in recording in mind).

After all this recording session happens in such way (assumed, that recording folder, format and name template is already prepared):

  • prepare or not prepare sample tracks (assumed prepared);
  • chose start place and pin recording to song start/stop;
  • start song playing and play your instrument or sing;
  • stop song (file now is recorded, may be in computer memory);
  • with context menu place recording where you want;
  • continue recording
  • mange placed files: rename, normalize, change format, save recorded (if in memory), delete not placed files.

In this case:

  • no change in clip/part look needed;
  • no need for track to be armed for recording.

One big problem: I do not know any DAW, with such audio recording style (some problems with multi-track recording can be solved by extension).

@michaelgregorius
Copy link
Contributor

@firewall1110, the problem with the approach that you have described is that it assumes that users only want to records single tracks from single sources. However, most audio interfaces have more than one input and in all DAWs it is possible to record from them in parallel.

To solve this issue it must be possible to assign each track the input that it should record from. Because there are several potential inputs not it also does not make sense to have one global input monitor. Instead it must be possible to monitor for each individual track.

Regarding the audio pool. If it is not already implemented it would also be nice to be able to drag samples, e.g. from a sample track into AFP, etc.

Perhaps it would be best to improve LMMS from the bottom up instead of starting with "high level" features like audio recording. A good first step might be to implement the management of input and output audio channels in LMMS. This would for example enable a more flexible routing or at one point even potential support for surround sound and other multi-channel related features.

@firewall1110
Copy link
Contributor Author

Quote: "it assumes that users only want to records single tracks from single sources. " No ... Let's continue strategy:
(4) Enabling placement for multiple audio files from session (as we can now paste multiple clips). Some handy pool files grouping. (Can not start develop now: session audio pool must already be in master).
(5) Enable multiple input file naming policy and "round robin" input source monitor. (Can not start develop now: no multi-track recording capable audio back-end, project audio file managing must already be in master).
(6) Enable input source "mixer" with possibility record plugin processed audio, adjust gain, .... (Can not start develop now: no multi-track recording capable audio back-end).

I wrote before only steps, could be started right now - in parallel.

P.S.
(a) I do not think my idea as #5990 replacement - it is something in addition, that can be implemented easier.
(b) I have (and not one) multi-track recording capable audio device, but mostly recording single track from single source (Only in sound recording studio engineers mostly record multi-track).
(c) SDL2 is only audio back-end supporting audio input (more than 3 years) but feature can not be used, even tested - is it really working?
(d) How someone can enable audio input on Jack (good start for multiple source recording - even not needed record capable sound card for implementing), pulseaudio, portaudio, ... : it is not possible to test input device.

@firewall1110
Copy link
Contributor Author

firewall1110 commented Aug 7, 2024

10 days alert to change goal to: (edited 2024.08.13) Stopped: better is to open new issue and PR
(1) Simple audio input level monitoring and recording with processing format somewhere, there file can be easily found and included in sound track part (clip). This is custom widget placed in main application window (near oscilloscope). Recording may be started/stopped by mouse click, or may be pinned to song play start/stop.
P.S.
Alternative is to close this PR and delete branch.

@firewall1110
Copy link
Contributor Author

10 days alert to close this PR as not actual

This pull request was closed.
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.

LMMS process keeps running (master)
4 participants