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

Sample Track Recording Stage One #5990

Draft
wants to merge 48 commits into
base: master
Choose a base branch
from

Conversation

Reflexe
Copy link
Member

@Reflexe Reflexe commented Apr 17, 2021

recording.mov

This is the first stage of the Sample Track Recording feature and was intended to only work on one backend (SDL) with no fancy features like looping and visualization.
It has been tested very quickly on my setup with a monitor interface.

Note: the changes are from #4994.

@LmmsBot
Copy link

LmmsBot commented Apr 17, 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! 🎩

Linux

Windows

macOS

🤖
{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://output.circle-artifacts.com/output/job/a63b2a2d-f02b-4b83-8235-cdb92f29af95/artifacts/0/lmms-1.2.0-rc6.1231+gb2f3a1f86-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/Reflexe/lmms/1626?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "Windows": [{"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://output.circle-artifacts.com/output/job/0f86efe9-e3ad-4d7a-9f38-5831d542b9d2/artifacts/0/lmms-1.2.0-rc6.1231+gb2f3a1f86-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/Reflexe/lmms/1624?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://output.circle-artifacts.com/output/job/c724e479-cedb-48eb-affc-ba13fa6ce6fc/artifacts/0/lmms-1.2.0-rc6.1231+gb2f3a1f86-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/Reflexe/lmms/1628?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/m58xebqefwmpqok0/artifacts/build/lmms-1.3.0-alpha-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/44344781"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/2b35vlj7jhr3ov4a/artifacts/build/lmms-1.3.0-alpha-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/44344781"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://output.circle-artifacts.com/output/job/b8b29d25-eaf5-4b78-a156-eaec12fed67a/artifacts/0/lmms-1.2.0-rc6.1231+gb2f3a1f86-mac10.14.dmg"}}, "build_link": "https://circleci.com/gh/Reflexe/lmms/1627?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "b2f3a1f86e95abb4f1565b7be86e69187028d028"}

Copy link
Contributor

@IanCaio IanCaio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still going to review this with more care when I get home, but decided to take a peak at it from my phone and did a first pass review just for fixing code style (stuff like explicit blocks for one line ifs, spacing, camelCase variable names). Added all as suggestions if you'd like to just batch commit them.

Is there any reason you decided to use a braced-init-list instead of the expression-init-list for those new members?

@PhysSong
Copy link
Member

My initial comments:

  • If I remember correctly, we concluded to remove shouldApplyMasterGain from pushInputFrames before.
  • I think it's okay to add more backends in this PR.

@Reflexe
Copy link
Member Author

Reflexe commented Apr 18, 2021

@PhysSong You are right. I forgot to remove it before pushing.
Hopefully will do it after work (in about 12h).
About other backends: I think we can first merge this, and then add pulseaudio and jack one by one. That will make testing more focused and wilp make review much easier.

@enp2s0
Copy link
Contributor

enp2s0 commented Apr 19, 2021

Fully agree with doing backends 1-by-1. SDL is a good start since it's available everywhere and is the default, so recording will work ootb for most users.

@PhysSong PhysSong mentioned this pull request May 10, 2021
11 tasks
@sebageek
Copy link

I compiled and ran this PR today on Linux and it seemed to work pretty well for me. Used the SDL backend (with pipewire), recorded a bit of Guitar music.
I had to search a bit for "how to actually record something", but after I found out that I could arm painted tracks for being recorded everything became clear. Maybe the o rec symbol could be accompanied by painting the track background orange/red so I know I'll record over it on the next run.

Things that worked for me:

  • saving and loading (without saving resources) - sample tracks were there on load
  • recording, cutting, moving, duplicating sampletracks

Things that did not work for me:

  1. Using the "Record samples from audio-device" button - It does nothing for me.
    No recording or anything will happen. I can only record audio using the "record while playing song / BB-Track" button. Am I holding it wrong?

  2. saving "with resources" (click in save dialog) breaks the saving process
    Directories will be created, but save will be aborted. The error seen on the console is as follows:

QFile::copy: Empty or null file name
ERROR: Failed to copy resource

I think lmms tries to save the sample track, but it has no associated file name. See DataFile::copyResources() in ./src/core/DataFile.cpp.

Another feature that would be really nice is if I had a way to save the sample track I recorded with lmms (maybe I decide I need it as an extra wav/mp3/something or want to post-process it somewhere else).

All in all: A big plus from my side! Looking forward to seeing this feature integrated in lmms once it's done!

@Reflexe
Copy link
Member Author

Reflexe commented May 17, 2021

@sebageek Thanks for the feedback. I will look into both. In any case. feel free to open an issue about being able to save individual Samples (if there isn't one open already).

  • The second issue was very simple, I will open another PR for the fix.

  • The first issue, however, IDK really know how to solve because I don't really know what is the meaning of the button (Maybe we better just remove it?) I have no idea what could it mean. The only thing I could think about is to play the song like every other track has been muted (e.g. only record w/o playing anything). But that will require some heavy modifications of the recording code I am not sure are in scope for this PR. CC: @PhysSong . @Umcaruje ?

@sebageek
Copy link

Hey @Reflexe, nice to hear that it was helpful && that the second issue is easy to fix! I also opened #6022 for the feature request.

Regarding the first issue: The record button apparently dates back to a stub implementation in 2008 and is only now appearing as it checks if the current audio backend is implementing capturing. So, how about using this button for its original intention? It could just allow the user to record samples without any other track playing (essentially the same as the two record buttons in the piano roll). On the other hand while I could imagine this functionality useful.. if this is not needed I'd opt for removing the button.

@PhysSong
Copy link
Member

PhysSong commented Jun 5, 2021

I think you should remove applyMasterGainToInputBuffer function as well, it's not used anymore after the removal of shouldApplyMasterGain.

@firewall1110
Copy link
Contributor

I have compiled this branch ...
Is real recording implemented or stage 1 means some preparations for this?

I am about to PR that fix Issue #4668 , but also changes some things how capture works in goal to enable input capture callback only if needed. Also see my questions on Discord ...
Let's collaborate!

firewall1110 added a commit to firewall1110/lmms that referenced this pull request Jul 2, 2021
@firewall1110
Copy link
Contributor

Now recording is working if merge this branch over my capture branch:

RecordingStageOne.mp4

Is this correct?

@Reflexe Reflexe force-pushed the feature/recording-stage-one branch from 4a62327 to c14a357 Compare July 31, 2021 18:35
@Reflexe
Copy link
Member Author

Reflexe commented Jul 31, 2021

Rebased on master, remove apply master gain related changes.
Left to apply styling issues. Will do it shortly.

@Reflexe Reflexe force-pushed the feature/recording-stage-one branch from c14a357 to 6158e76 Compare July 31, 2021 18:55
@JGHFunRun
Copy link
Contributor

I think at some point we should switch from set/clear record to a proper record button but that's for the future IG

@Reflexe Reflexe force-pushed the feature/recording-stage-one branch 2 times, most recently from d01abd2 to e5c795c Compare August 21, 2021 18:13
@JGHFunRun
Copy link
Contributor

@LmmsBot

@Mayravixx
Copy link

Mayravixx commented Sep 21, 2021

Tried doing this today, sadly it isn't working for me on Arch. When I place down a sample section then right click it, I get no "set/clear record" option at all. Would be really simple if I could grab the exe's in some way but none of the .exe's can be downloaded, same story with the appimage file.

2021-09-21.17-07-36.mp4

@DomClark
Copy link
Member

@Mayravixx That version doesn't contain the code from this PR - it's instead the latest master version (see the commit hash in the title bar - 8a9a2fa).

@firewall1110
Copy link
Contributor

It is a relatively large new feature

For final variant - yes. For stage one - no: only one thing be added - samples to sample track not only imported, but may be recorded too.

My opinion: this PR problem is that stage one is treated as last one. Why it is problem? LMMS device drivers are not ready to final variant - but device drivers can not be improved without testing possible: are waiting for this stage one be in master.

Both implementation will have their challenges if implemented correctly. The first thing to notice is that the audio data comes in via the real-time audio thread.

So in this case there are no difference in complexity (only variant):

  • we have to use some kind of ring buffer for real time thread;
  • we should copy (from this ring buffer) to clip-memory or save to file in another thread.
    Is this implemented here? How I understand - yes. What variant is implemented?

(All after I treat as off-topic)

The users rename the track to "Track B" and record again.

I mean folder is in track configuration, so changes the file name pattern.
(Track name usage as audio recording pattern is Cubase style. But Cubase use Audio folder in project folder.)

I am not sure if is a good idea to have sub folders which are named like the tracks.

OK, but
How I understand, LMMS have no project folder in this (Cubase, Ardour, ...) meaning,
so simpler variant may be
folder with the same name as project but in samples (or even in projects).
But for file name use pattern based on track name.

Where should it be discussed if not here?

In PR "Sample Track Recording Stage Five - Final" .
Or in Issue "Audio mixing project in external folder: adding feature - save to external folder as independent project"

P.S.
Can You somehow restart workflow - or I should compile this PR myself?
I want to test this PR, but job details - artifacts are not here anymore ...

@qnebra
Copy link
Collaborator

qnebra commented Jan 2, 2025

I mean, there is a way to have audio recording working in most common audio backend used in LMMS, SDL. There is also proposed solution for saving recordings, without writing base64 blob into project file. Rudimentary, but it is.

Other stuff, like recording in other audio API (JACK is already in another PR), autosaving, project folders, can be done later.

@firewall1110
Copy link
Contributor

OK, I compiled this PR (but only for my Linux Debian Stable 64-bit).

It seems, that audio is recorded to memory and saved in project file as huge data="...." line .

In this case #7627 is a workaround , but it should be tested.

Now I will check, how Audio Recording is implemented ...

P.S.
To restart workflow it seems some conflicts must be resolved (why I don't see here, that PR has conflicts?)

@michaelgregorius
Copy link
Contributor

@JohannesLorenz, I have just checked with Bitwig. In the project folder there is a sub folder called "recordings" where the files go that have been recorded with Bitwig. There's also a sub folder called "samples" where the samples go that come from somewhere else, e.g. samples that have been loaded from disk and which are used in a drum machine.

Perhaps this is some nice compromise so that we still do not have any vinyls in our folders. 😉

@michaelgregorius
Copy link
Contributor

In this case #7627 is a workaround , but it should be tested.

It seems that #7627 only exports the content of sample clips into files, i.e. it will not replace the recorded data with references to the exported files. This means that the data would still be stored in unwieldy data blocks in the file.

Saving the data as data blocks in files has the disadvantage that it wastes lots of space on disk if there are several versions of a project and that it likely also has a bad performance when these project files are loaded.

Example for several versions:

  • "My big hit 01.mmpz"
  • "My big hit 02.mmpz"
  • ... [several more iterations] ...
  • "My big hit - Final.mmpz"
  • "My big hit - Final for real.mmpz" 😉

@JohannesLorenz
Copy link
Contributor

Well, there's a difference between being able to at least quickly import the recorded audio files and recreate the project or to lose everything.

Sorry, I don't understand what you're trying to say. Can you please elaborate?

@michaelgregorius
Copy link
Contributor

Well, there's a difference between being able to at least quickly import the recorded audio files and recreate the project or to lose everything.

Sorry, I don't understand what you're trying to say. Can you please elaborate?

I meant to say that relying on the auto-save feature might lead to losing lot of work if the auto-save is not "quick" enough, e.g. if you have done a lot of recordings between the last auto-save point and the next one that is not reached anymore due to a crash. If the recordings are streamed to disk then you will have at least all your recordings and might only lose part of some recordings if LMMS crashes during the recording.

The auto-save files are also another example where storing everything in memory and then in data blocks in the file would waste a lot of disk space because every auto-save would contain all the data instead of just references.

@michaelgregorius
Copy link
Contributor

I have just also checked how Bitwig handles projects which have not been saved yet. The are assigned a GUID as the "project name" and their data is stored in a temporary folder, e.g. the recordings are stored under $HOME/.BitwigStudio/temp-projects/21a3ed0e-00aa-465c-8053-e6b36e56bf06/recordings/.

I guess if you then save a project for the first time then all that stuff is moved into the actual project folder together with a newly created save file.

@JohannesLorenz
Copy link
Contributor

I meant to say that relying on the auto-save feature might lead to losing lot of work if the auto-save is not "quick" enough, e.g. if you have done a lot of recordings between the last auto-save point and the next one that is not reached anymore due to a crash. If the recordings are streamed to disk then you will have at least all your recordings and might only lose part of some recordings if LMMS crashes during the recording.

The same accounts for the rest of the project file, too. I assume you say that it's more easy to create the notes and the instrument/effect tunings of the last 1 hour than the records of the last 1 hour? In this case, we could allow 2 autosave options (How often should records be saved? How often should the rest be saved).

However, I am not against the proposal of saving it while recording - it's just more overhead to code that 😬 Possibly the audio thread would smash everything into a ringbuffer, and another thread fetches it and writes it on disk. I imagine it's more ugly to split the data into chunks and find a good naming for those chunks. At this point though, I am also not sure why we should split it...

@szeli1
Copy link
Contributor

szeli1 commented Jan 4, 2025

I would like to mention that I have made 3 PRs about audio exporting:

  1. Exporting with AudioEngine: Remove audio engine from exporting code #7452
  2. Exporting to sample folder: adding Sample Folder #7538
  3. Exporting without sample folder: adding sample exporter #7627

I would suggest moving the discussions about project folders to #7538 (since that pull request is a project folder implementation)

@firewall1110
Copy link
Contributor

IMO recording to files should be implemented as fast as possible instead of some workarounds.

It is 1-st thing, that should be implemented and it is surprise for me that it is not done yet ...

P.S.

But about question: "Where and how place recorded files?"
Any reasonable variant is good if user can find just recorded files, can find recorded files associated with project.

@firewall1110
Copy link
Contributor

Now I will check, how Audio Recording is implemented ...

Audio Recording implementation is not here, but already in master: SampleRecordHandle.h , SampleRecordHandle.cpp

And this implementation is already in lmms-1.3.0-alpha.1.102

@firewall1110 firewall1110 mentioned this pull request Feb 9, 2025
1 task
@firewall1110
Copy link
Contributor

firewall1110 commented Feb 16, 2025

As @StakeoutPunch observed and reported on Discord, this feature has a blocker:

While I doubt I have any right to block a PR, I think that storing the recorded data in the project file as base64 is not an appropriate implementation.

So implementation with storing recorded data in project file is not acceptable by LMMS team.

P.S.
This PR has 4 (!!!!) approvals - and not merged ... Now I understand why, so I duplicate this information.

@szeli1
Copy link
Contributor

szeli1 commented Feb 16, 2025

So implementation with storing recorded data in project file is not acceptable by LMMS team.

Where was this decided? I didn't hear about this

@firewall1110
Copy link
Contributor

firewall1110 commented Feb 16, 2025

There are ~149 hidden items, and one is:

"tresf commented on Jul 17, 2024 •"

P.S.
This PR should be manually renewed... - became hard to understand what happens here.
And also title "... Stage One", it is neither stage one (SDL2 input enabled - it is stage one), nor stage two - recording implementation (not completed - only in memory, and this is this PR problem key) is stage two.

@szeli1
Copy link
Contributor

szeli1 commented Feb 16, 2025

"tresf commented #5990 (comment) •"

I implemented 2 out of 3 ideas mentioned here. I have made 3 PRs that allow audio exporting for this PR.

So implementation with storing recorded data in project file is not acceptable by LMMS team.

Yes, this seems to be true. I don't see why this was mentioned after 5 months, but it is a good reminder.

@firewall1110
Copy link
Contributor

So it is deadlock like situation:

  • this PR can not be merged without yours , @szeli1 , ideas connected to it;
  • this ideas can not connected to this PR, because it is not merged in master.

May be (manual) this PR integration into one of Yours , @szeli1 , PR be a solution?

P.S.
But let's LMMS team make decision ...

@firewall1110
Copy link
Contributor

Also it seems some of this PR changes are also in #7567 ...

@michaelgregorius
Copy link
Contributor

To me it looks like this PR is stalled because it tries to do too many things at once. There are at least two big tasks in making recording available:

  • Make audio input buffers available: This must happen in the "lower" levels of LMMS. An interface should be defined on the LMMS side that can be used by the different driver implementations to push all available buffers of the input devices/ports towards LMMS. The implementation of that interface, let's call it IOManager for now, knows how to manage that data and how to for example copy it into LMMS' internal buffers. The interface (and implementation) should also define methods that can be used by drivers to announce the availability of new audio inputs/ports (e.g. if an audio interface is connected while LMMS is running) or the removal of inputs. In a way that interface will mirror some of the functionality that is available in different audio drivers and it will reflect the given subset of that functionality that LMMS wants to support.
    As the name implies the IOManager will also make the available output ports known to LMMS.
  • Usage of input buffers: This might be a second interface that is also implemented by IOManager and it describes all the methods that clients, e.g. tracks that want to record data, etc. need to know to accomplish their tasks. So it provides read-only access to the input buffers that are available in the current period. A second interface is useful because clients should not even get the idea that they can also push input buffers into the manager. This functionality should only be available to the drivers. Clients of the second interface should only be able to consume the data that is available in each period. It's on this level that some policies are implemented which are also discussed in this PR, e.g. how to organize and store audio files, etc.

The general flow/loop in an audio driver would then look as follows:

  • A new period is started.
  • Push all available inputs to IOManager so that the data is available when the outputs are rendered.
  • Compute the outputs by traversing the audio graph. The nodes of that graph read the inputs of all kinds (e.g. the buffers of the IOManager), process them (e.g. by writing audio to a file, applying plugins, etc.) and then write the final output buffers.
  • Copy the computed output buffers to the driver output buffers.

This PR is concerned with both tasks described above whereas #7567 is attempting to make (one) input buffer available via the Jack driver.

The first task would be responsible for making all available inputs available to LMMS (so that users do not have a make a single rather limiting choice when setting up the audio driver) and the second task would be responsible for extending LMMS so that it can make use of them.

It would likely make sense to work on these tasks separately and to work on them in a bottom-up way in a collaborative effort.

@tresf
Copy link
Member

tresf commented Feb 17, 2025

To me it looks like this PR is stalled because it tries to do too many things at once.

Does it?

Make audio input buffers available

Can you explain (in a few words, please) why this isn't already functioning in supportsCapture | captureDeviceAvailable?

The first task would be responsible for making all available inputs available to LMMS (so that users do not have a make a single rather limiting choice when setting up the audio driver

Well, @sakertooth just removed this capture support in PortAudio #7444, quoting "... input device is useless as of right now ..." so I agree that we're -- as a collective -- limiting choice, but I can't see anything in this PR specifically that is "limiting choice when setting up the audio driver". The code reads pretty backend agnostic and touches a minimal set of files.

When this was discussed on Discord, the hold-up was the storage of the recorded data, as pointed out in several comments above. That's my high-level understand of it, happy to be corrected (preferably in a few words, please).

@michaelgregorius
Copy link
Contributor

To me it looks like this PR is stalled because it tries to do too many things at once.

Does it?

IMO it does because it seems to build on several things that are not fleshed out yet, like for example:

  • code that makes the input buffers available to higher level entities like tracks
  • concepts on how to deal with recorded data

Make audio input buffers available

Can you explain (in a few words, please) why this isn't already functioning in supportsCapture | captureDeviceAvailable?

Only the SDL driver seems to return true for supportsCapture and that driver currently only supports one single input. captureDeviceAvailable is introduced with this PR and flying over the diff I could not find any code which makes all potential input buffers available to tracks, etc.

If I understand correctly there is already rudimentary support for multiple inputs and outputs with the AudioPort class? If that's the case then it does not seem to be used anywhere. For example AudioJack::registerPort does nothing if the application is not compiled with AUDIO_PORT_SUPPORT being set.

I also was not able to find any usage of AudioPort where the data from an input is read and processed.

The first task would be responsible for making all available inputs available to LMMS (so that users do not have a make a single rather limiting choice when setting up the audio driver

Well, @sakertooth just removed this capture support in PortAudio #7444, quoting "... input device is useless as of right now ..." so I agree that we're -- as a collective -- limiting choice, but I can't see anything in this PR specifically that is "limiting choice when setting up the audio driver". The code reads pretty backend agnostic and touches a minimal set of files.

My comment with regards to "make a single rather limiting choice when setting up the audio driver" was about the current state of the SDL driver. If you activate and configure it then you can select a single input in the dialog. It is limiting in that I can only use one input and have to decide which one to use upfront. The option is also in the wrong place. The driver should make all inputs available and users should be able to select them in the application and not in some preferences/configuration screen.

When this was discussed on Discord, the hold-up was the storage of the recorded data, as pointed out in several comments above. That's my high-level understand of it, happy to be corrected (preferably in a few words, please).

My understanding is that it is indeed one of the open questions and if it is solved then the state will be that only the SDL driver will allow the recording of audio from a single source that has to be configured in the driver configuration screen and that therefore cannot even be switched without restarting the application. So it would be very rudimentary support that's not very comfortable for the users.

However, it would at least be a step in the direction and might highlight the places that need work.

P.S.: I hope I did not use too many words. It reminded me of how I sometimes tell ChatGPT to be concise. 😅 😉

@tresf
Copy link
Member

tresf commented Feb 22, 2025

I've only ever been able to use a single output device with SDL so I'm not sure how that's relevant, but assuming the rest is, can you please make specific recommendation? Also, can you chime in on #7444 and tell @saker to stop removing input devices ? (Edit @sakertooth confirmed in #7444 prior to writing this that the input devices will be staying).

@michaelgregorius
Copy link
Contributor

I've only ever been able to use a single output device with SDL so I'm not sure how that's relevant, but assuming the rest is, can you please make specific recommendation?

If you have several inputs you will find them listed in the combo box on the SDL driver's configuration screen. They are enumerated when the driver is initialized. I also assume that it is possible to process all of them during the processing done in the audio driver loop. So my recommendation is to devise a concept for how to forward the different input buffers to the application, e.g. as described here: #5990 (comment).

Also, can you chime in on #7444 and tell @saker to stop removing input devices ?

Unfortunately I am not familiar with that PR and only did a very quick scan as I now want to wind down my involvement in the project to an absolute minimum.

The way I understand it is that the handling of the inputs caused some problem in the implementation of the PortAudio driver. If it can only be solved by removing the input handling then this might point to a problem with the greater design revolving around audio devices, inputs, etc., i.e. the design might not work for all different ways audio driver APIs might work. So removing the input handling might be feasible until the bigger problem is solved.

IMO it should be well explained and documented though why they absolutely must be removed and what's exactly causing the problem. Otherwise it is just taking stabs in the dark and a potential back and forth that leads nowhere.

So I cannot reliably tell if it is wrong to remove the input handling.

@tresf
Copy link
Member

tresf commented Feb 22, 2025

The way I understand it is that the handling of the inputs caused some problem in the implementation of the PortAudio driver.

Oh, I didn't get that impression at all. Anyway I'll leave the conversation to be discussed over there.

If you have several inputs you will find them listed in the combo box on the SDL driver's configuration screen.

Hmm, I was under the impression that only the default output device was selectable. That's how it was when testing #7444.

Only the SDL driver seems to return true for supportsCapture and that driver currently only supports one single input.

So is the argument here that we need to be able to record from multiple inputs without restarting before this can be merged? If so, I disagree.

I now want to wind down my involvement in the project to an absolute minimum.

I'll try to avoid roping you into new PRs, but I thought it was extremely relevant to your point.

@michaelgregorius
Copy link
Contributor

If you have several inputs you will find them listed in the combo box on the SDL driver's configuration screen.

Hmm, I was under the impression that only the default output device was selectable. That's how it was when testing #7444.

For the SDL driver it should look something like shown here: #5990 (comment)

#7444 is about the PortAudio driver though and its configuration screen might look different from the SDL driver's.

Only the SDL driver seems to return true for supportsCapture and that driver currently only supports one single input.

So is the argument here that we need to be able to record from multiple inputs without restarting before this can be merged? If so, I disagree.

No, that's not my argument. As stated here an "incomplete" merge might point to the places where work is needed even if it would offer rather rudimentary functionality: #5990 (comment). However, I believe that "the perfect is the enemy of the good" so it would be counter-productive to wait with a merge until everything is implemented perfectly.

It's a trade-off with regards to what we want to present to the users though. It should be somewhat polished, doesn't need to be perfect and should not make the users run away. 😅

I now want to wind down my involvement in the project to an absolute minimum.

I'll try to avoid roping you into new PRs, but I thought it was extremely relevant to your point.

Thanks, I appreciate your consideration. I just happened to notice that being involved in lots of discussions would run counter to my farewell. 😅

@michaelgregorius
Copy link
Contributor

michaelgregorius commented Feb 23, 2025

I have now unsubscribed from this issue due to the aforementioned reasons. Just so that nobody wonders why I won't react anymore although I have been quite active in this issue before. I wish you good luck with getting this feature into a good state for the users! 🙂

Edit: @tresf, why did you downvote this? I consider it good etiquette to not just vanish without any explanations from threads where I have been quite active. Obviously I will not do this everywhere.

@JohannesLorenz
Copy link
Contributor

JohannesLorenz commented Mar 1, 2025

This branch needs to be hijacked to a master branch (no disrespect to the author). The reason is that we have already 3 PRs (#7567, #7538 and #7627) that must be merged into this branch, but we don't want to make PRs in @Reflexe 's fork (because we will not see them and we may not even have permission there).

@Reflexe are you OK with this? Can you please push your branch to LMMS/lmms and open a new PR there? Otherwise I would just do it myself (the commit authors will remain unchanged).

Note: A rebase could be done at the same time to fix the merge conflict.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

Audio Sample Recording