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 Sample Clip Reversing Position #7446

Merged
merged 4 commits into from
Sep 13, 2024

Conversation

regulus79
Copy link
Contributor

This pr makes it so that when you reverse a trimmed/cropped sample, the cropped section is actually reversed. This is in contrast to the previous behavior, where the original sample would be reversed, causing the clip bounds to be on a different part of the sample.

Fixes #7352

@sakertooth
Copy link
Contributor

Just tested the PR. It seems it didn't fix it for me.

@Gabrielxd195
Copy link

I tried this and it didn't work, but it's good that they took this into account.

@sakertooth sakertooth marked this pull request as draft August 12, 2024 13:02
@Gabrielxd195
Copy link

@regulus79 I did the test and now this improvement works, if there is no other problem I think it can be merged.

@regulus79 regulus79 marked this pull request as ready for review September 7, 2024 22:26
Copy link
Member

@messmerd messmerd left a comment

Choose a reason for hiding this comment

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

Tested it and it seems to work fine

@messmerd
Copy link
Member

If @sakertooth approves, I think it's good to merge

Copy link
Contributor

@sakertooth sakertooth left a comment

Choose a reason for hiding this comment

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

Just retested, it seems to work for me now.

@sakertooth sakertooth merged commit 4831495 into LMMS:master Sep 13, 2024
11 checks passed
@bratpeki
Copy link
Member

This might work, but I noticed that the reversed and forward sample slices don't look the same. On further inspection, I noticed LMMS behaves oddly when different zoom is applied.

screencap-2024-09-14-14-26-45
screencap-2024-09-14-14-26-49
screencap-2024-09-14-14-26-52

These each subsequent step is one zoom-in action. Noticed how the last one looks. Very odd.

@bratpeki
Copy link
Member

screencap-2024-09-14-14-28-00
screencap-2024-09-14-14-28-04

Notice how the reversed one has this "tail" after the transient.

@bratpeki
Copy link
Member

Sounds perfectly fine, so the feature is a success!

However, the sample zoom thing is most odd.

Here's proof that the reverse doesn't match the sound.

a.mp4

This is what ffprobe has to say about the audio file:

Input #0, mp3, from 'MidiMelo2updatedMidi3.mp3':
  Metadata:
    Software        : LMMS (libsndfile-1.0.28)
  Duration: 00:01:03.79, start: 0.025057, bitrate: 294 kb/s
  Stream #0:0: Audio: mp3 (mp3float), 44100 Hz, stereo, fltp, 294 kb/s
      Metadata:
        encoder         : LAME3.100

Should I open a new issue regarding this?

@regulus79
Copy link
Contributor Author

@bratpeki I cannot reproduce the issue; would you mind providing a sample file/project where the bug happens?

@bratpeki
Copy link
Member

bratpeki commented Oct 9, 2024

Just to note, we discussed this further on Discord.

mmeeaallyynn pushed a commit to mmeeaallyynn/lmms that referenced this pull request Oct 27, 2024
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.

Sample Track: Clips trimmed from Sample Track are not reverted.
5 participants