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

Recording after rebase to master #4994

Closed
wants to merge 112 commits into from

Conversation

Reflexe
Copy link
Member

@Reflexe Reflexe commented May 26, 2019

This is a refactored and rebased-to-master version of #4531 .

Bugs

  • TripleOscillator preset previews show errors and don't stop due to nonexistent samples/empty.wav
  • w2_invsinehalf.flac has a furry tail in BitInvader due to resampling(maybe an issue with Graph?)
  • Visualization gets weird when resizing tracks while recording

Enhancements

  • (optional) Add more backends
    • ALSA
    • CoreAudio
    • Sndio
    • libsoundio
  • (optional) Refactor
  • Decide when to resample data
  • Optimize

Reflexe added 30 commits May 26, 2019 10:18
SampleBuffer.

That solves problem with recording that caused samples to be moved and
messed up.

Thanks @-BaraMGB for helping.
Previously, begining record in any position would result the data being
in the end of the TCO.

@see LMMS#3947 (comment)

Thanks to @-BaraMGB :)
`updateMenu`, use a virtual function overriden by every Track
implemetation.
input channel configurations. And support recording of one channel and
converting it to two.

The user would be able to set their input config per track, or globally.
For each recorded sampletrack, the recorded will get the channel config
and will duplicate the samples from one channel to the other in a case
of mono input.
options unchecked will be considered as global default.

Suggested by @-tresf, Thanks!
instead of hacking sampleBuffer ()->startFrame ().

That solves a bug with `startFrame ()` being negetive in recording some
cases.
rate.

The deadblock was caused by the mixer stopping which make the recording
thread to wait until it get started again and in the other side, the
exporting thread which wants to acquire a mutex that had been locked by
the recording thread.

Many thanks to PhysSong :)
support is not available.

Thanks to @-Sawuare.
played correctly after saving and loading a project due to sample rate
not getting saved in the project file.
right place when it not played from the begining.

That has created a difference between the ticks and the metronome and
the sample track.

The cause of the problem was that the calculation of the frame to play
was wrong: we had calculated `framesPerTick` according to the current
engine's sample rate instead of the SampleBuffer's sample rate.
automaticlly create "record sample tco" if there is no recording tco.

This commit adds per-track record setting. If a sample track is set as
record, lmms will check if there is a tco that set to record:
  * If there is, it will record into it.
  * If there is not, it will create a special tco that get created on
recording, and as long as the recording.
@PhysSong
Copy link
Member

@Reflexe Sorry for repetitively bumping this, but do you have any further plans on this PR? It's one of the features with many demands.

@Reflexe
Copy link
Member Author

Reflexe commented Sep 19, 2019

@Reflexe Sorry for repetitively bumping this, but do you have any further plans on this PR? It's one of the features with many demands.

Sorry about the delay. I'm actively working on this locally and will keep your all updated.

@PhysSong
Copy link
Member

The biggest problem of this PR is that it's very hard to review. I think this PR mainly consists of:

  • changes in SampleBuffer and follow-ups in other classes
  • core changes that enable recording
  • UI changes for capturing audio
  • backend-specific capturing implementations
  • build system changes

@Reflexe As you may know, the changes in SampleBuffer dominates this PR, making the core functionality hard to review. Since the core functionality heavily depends on those changes, I think you should open a new PR for SampleBuffer changes to allow other devs to review it.

@LostRobotMusic LostRobotMusic mentioned this pull request Dec 15, 2019
@ChildishGiant
Copy link

Any update on this? I feel like this is a key feature for any DAW

@PhysSong
Copy link
Member

PhysSong commented Feb 4, 2020

I want to separate this PR into multiple "easier-to-chew" (test and review) PRs. I just can't find the time for it.

@Reflexe Do you want/need some help? If it's the case, I can try.

@therealhammer
Copy link

Just forked and compiled this branch. Great job! Recording worked pretty good with Jack. Looking forward to see this feature in a release version! :)

@gl3nnleblanc
Copy link

Sorry if I shouldn't bump this, but any update? My workflow would be heavily improved if this is implemented.

Just forked and compiled this branch. Great job! Recording worked pretty good with Jack. Looking forward to see this feature in a release version! :)

Are there currently any UI elements in place? I built this branch but still see no way to record.

@qnebra
Copy link

qnebra commented Jun 20, 2020

There is a lot more merge conflicts than "non-technical" user can solve.

@PhysSong
Copy link
Member

As I mentioned a while ago, I'm preparing to revive this PR by splitting to smaller PRs. As far as I understand, this PR consists of:

  • Backend changes to support audio capture
  • Some changes in core classes like Mixer, SampleRecordHandle
  • SampleTrack changes for recording
  • Massive SampleBuffer refactoring
  • Threading-related changes
  • UI changes related to recording
  • Build system changes(mostly related to JACK on Windows)
  • Some CI changes, probably not needed anymore
  • Other minor changes

Now I have to analyze their dependency chain and look into individual commits.

@PhysSong
Copy link
Member

It seems like only a few changes are actually required if we give up supporting live updates while recording. Live updating requires a lot of changes regarding internal data structure, thread safety, and much more.
While trying to understand changes in this PR, I found some odd code fragments. I built and tried to test this PR, but everything was too unstable and buggy to merge this as-is.
I even had to drop a lot of recent commits to get a version which doesn't break much. This is a result of poorly reviewed and tested code.

@PhysSong
Copy link
Member

PhysSong commented Jan 1, 2021

Notes for who are looking for downloads: Due to the merge conflicts, neither I or the bot can't provide downloads as-is. As I mentioned above, this pull request is being reworked by me. It's not ready for review at this moment, and there won't be a new pull request until the project reorganization(#5592) finishes.

If there are enough demands for testing binaries, However, I can consider providing unofficial binaries for the new rebased version(Warning: still WIP!).

@PhysSong PhysSong added rework required Pull requests which must be reworked to make it able to merge or review and removed needs code review A functional code review is currently required for this PR needs style review A style review is currently required for this PR labels Jan 4, 2021
@Reflexe
Copy link
Member Author

Reflexe commented Jan 10, 2021

I remember trying a few odd things on the latest commits. Removing them should give a relatively working recording functionality.

Do you have specific instabilities / errors?

@brigazvi
Copy link

Is there any reason to not merge it to master?

@IanCaio
Copy link
Contributor

IanCaio commented Apr 14, 2021

Is there any reason to not merge it to master?

That's one of them:

This branch has conflicts that must be resolved

Conflicting files
.travis/linux..install.sh
CMakeLists.txt
include/AudioSampleRecorder.h
include/AutomationTrack.h
include/BBTrack.h
include/Engine.h
include/InstrumentTrack.h
include/SampleBuffer.h
include/SampleRecordHandle.h
include/SampleTrack.h
include/SongEditor.h
include/Track.h
include/lmms_basics.h
plugins/audio_file_processor/audio_file_processor.cpp
src/CMakeLists.txt
src/core/Effect.cpp
src/core/Mixer.cpp
src/core/SampleBuffer.cpp
src/core/SamplePlayHandle.cpp
src/core/SampleRecordHandle.cpp
src/core/Song.cpp
src/core/Track.cpp
src/core/audio/AudioDevice.cpp
src/core/audio/AudioJack.cpp
src/core/audio/AudioSdl.cpp
... 

@netpipe
Copy link

netpipe commented May 10, 2021

it will need to be updated to merge it now ?

@PhysSong
Copy link
Member

This one is being continued in #5990 and probably more future PRs.

@Reflexe
Copy link
Member Author

Reflexe commented May 17, 2021

Moved here: #5990

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rework required Pull requests which must be reworked to make it able to merge or review
Projects
None yet
Development

Successfully merging this pull request may close these issues.