-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
adding Sample Folder #7538
base: master
Are you sure you want to change the base?
adding Sample Folder #7538
Conversation
I'm no longer working on this feature. |
This has been brought up many times, but why are you convinced that hundreds of lines of code are necessary to add a way and a place to store sample files for projects? |
I wouldn't say that I'm convinced or if this is the best way to do this. This PR has secondary goals that include:
I believe these goals have been agreed to in a discussion about this feature by multiple users. There was an other discussion about this feature also. I believe you didn't object to these goals when those discussions happened. I tried to follow most of these guidelines and I tried to implement most of these features. Some features have been left out (like the ability to export samples that are still being recorded, so in case of a crash the sample isn't lost) I hope this comment clarifies the larger than expected code amount. If not, feel free to ask more questions! |
If you are not convinced that your work is necessary or a satisfactory solution to a problem, then this shows uncertainty. I applaud your effort, but if you are uncertain about it then I am too.
A good practice for PRs is to keep them small and focused. It helps with reviewing them, minimizes regressions, and allows the team to have a better grasp of the PR and be more confident with knowing how and why the changes were made. However, it seems as if you have coupled many somewhat related things, and as a consequence the PR lacks direction. At the end of the day, the PR is still trying to do many things all at once. Stick to doing one thing please :)
I have read the goals you have listed, and I get the feeling the goals we mentioned in our discussions are different than what you had in mind. This isn't your fault really, we definitely need to get better at specifying what exactly we want to do for the project and what our overall direction is. We did this very nicely with working towards the 1.3 release, but a lot of the secondary goals, especially relating to sample functionality and sample workflow, could be better explained clearly somewhere, and not just in discussions floating on Discord. This "specifying of goals" is a team effort: no one developer can make all the decisions here. As small as the team size is, working on a project like this and making goals for it requires everyone to be considered. The community and developers alike.
I understand, but from my perspective, you are biting more than you can chew. I was like this once: I wanted to fix everything, rewrite the whole project, start from scratch. I still feel that way at times. However, I realized that it probably isn't as simple as that. A better, safer alternative is to take it one step at a time. |
Please ignore my feelings about this PR, it will lead nowhere
My issue is that I can't build on unmerged features. My FFT filter effect is still on hold because it is waiting for #7158. Other developers have advised me to merge features into 1 pull request rather than to make multiple smaller ones because it will be approved faster that way. I believe only 1 of my features were merged after 1 year of work.
I would rather not change this with this PR, but I will keep this in mind in the future. I think this PR accomplishes the goals I believe were set.
What direction should this PR aim for? I aim to completely implement and finish the folder component of "Sample folder" and exporting outside of
I agree with this. I usually feel unguided regarding the development direction. I usually try to communicate my ideas and sometimes nobody responds (this happened to the 1. discussion about this feature, where completely different goals were set). Edit: maybe I should take part in other discussions related to this project, so I can be a better contributor and take part in this team effort
What do you mean by this in the scope of this PR? Could you explain it with some examples? What are you afraid of (or what feeling leads you to this conclusion)? |
Sorry, but I refuse your request. You feel uncertain, and as such, I am uncertain. As a result of this, the conversation will amount to nothing, and not because the feelings about the PR are being ignored. |
How should one feel about a pull request? I don't feel certain. I don't feel uncertain. This is why I would rather avoid talking about this. I would like to focus on the facts, that sample track recording stage 1 needs audio exporting, an other fact is the code that accomplishes this (exists). |
That's normal, you're not doing anything wrong. Partaking in the discussions is fine, but even for people that aren't in the Discord but still contribute here (michaelgregorius comes to mind), we still need to improve how we specify our goals for the project. |
But you mentioned you were not convinced that your work was appropriate for the problems you were trying to solve? |
Yes, because this PR will change as reviews are done. I said this: "I wouldn't say that I'm convinced or if this is the best way to do this." I think if more experienced developers review this pull request, they might find issues that need to be fixed. |
That's fair. However, my opinion is that the PR just doesn't seem really focused. It accomplishes too many things all at once and it is really hard to understand if the PR has validity (validity, meaning if the PR is logically sound in its reasoning and consists of changes we need). Put simply, I do not understand what the PR does exactly. There's not a single, clear defined goal that I can gather unfortunately, but many slightly vague goals which lack direction and clarity in my opinion. I also mentioned before that I personally think the problem you are trying to solve, "to add a way and a place to store sample files for projects." has a far simpler solution. The way we create a place for users to store sample files is by giving them an option to specify a path to put sample files in. That's it. I don't really want to go into anything about this, because I really do think it is, in its simplest form, that rudimentary. Choosing just one of those things to start off would be a good start, even though there are maybe other developers possibly working on that same problem. However, having just one singular idea makes it so much easier to understand the PR and your vision as the author. |
You are right, it is simpler for that specific problem. I'm starting to understand your point about making more specific and clear Pull Requests. I guess I will make a new PR that includes the exporter and what you've described above. Thanks for taking the time to get to this point with one of the contributors. I still don't know what will happen to this PR once an other one exists with the same export code, I guess then I will rebase this branch. |
As development is taking place here on GitHub to me it would be a logical step to also make the goals of the project transparent on GitHub so that people can contribute in a meaningful way even if they are not on the Discord. My personal impression is that the project is lacking leadership, e.g. someone like Ton Roosendaal for Blender or Guido van Rossum for Python. However, it might only be my impression because I am not on Discord and therefore things are not transparent to me. My personal goal is to help push LMMS more towards a "professional" direction and to make it more on par with other DAWs (see for example my PR for faders with a dB scale). If you compare LMMS to other DAWs you will find that it is lacking quite a lot. However, my time is sparse so unfortunately I cannot help with the real big tasks that would be necessary to do so (real-time safety, audio graph with latency compensation, more sophisticated features for audio drivers, e.g. multiple inputs and outputs, etc.). That's also the reason why I do not want to get too involved, e.g. via Discord. I guess it might make sense to discuss this in a discussion thread which I might create. |
I have created the following discussion to not derail this PR: #7642. |
closes #7452
This pull request aims to add a way and a place to store sample files for projects.
A new class is added called
SampleFolder
. It will store audio files in 2 ways: relative (Project Folder) and absolute (inside a folder that can store all audio files for multiple projects).All sample loading and saving should be routed through
SampleFolder
.SampleFolder
class will require 3 new folders for each project. A project folder and a "Used" and a "Unused" folder placed inside it. There will be an option to save the project inside a project folder in the main window menus. If the project file is inside a folder with the same name,SampleFolder
will interpret it as a project folder. If this condition isn't true, a new global "everything" folder will be used as the current project folder.SampleFolder
will scan for ".flac" samples insideProjectFolder
,ProjectFolder/Used
andProjectFolder/Unused
and will load them if something is requesting ashared_ptr<SampleBuffer>
. The "Used" and the "Unused" folder makes managing files for the users easy, If they wants to delete the unused files, they easily can.A new class is added for exporting samples outside of the
AudioEngine
. This class is namedLmmsExporterSample
, this name isn't the best, so any better suggestion is appreciated! This class exports anySampleBuffer
in a new thread as a ".flac" file.How to test:
lmms/
(where theplugins
folder is), this folder is called "lmms_recorded_samples" (should be renamed in my opinion). Save the project without the sample folder option and reload the project. The sample should be loaded in correctly.this allows #5990 to be finished