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

issue #2119: Data loss on joining clips #2123

Merged
merged 1 commit into from Nov 16, 2021

Conversation

DavidBailes
Copy link
Collaborator

@DavidBailes DavidBailes commented Nov 12, 2021

Resolves: #2119

Joining clips using the Join command (Ctrl+J) can cause data in preceeding clips to be silenced.

Fix: In WaveTrack::Join(), the initial value of t should be PlayStartTime of the first clip, not SequenceStartTime.

Resolves: #2119

  • [x ] I signed CLA
  • [ x] The title of the pull request describes an issue it addresses
  • [ x] If changes are extensive, then there is a sequence of easily reviewable commits
  • [x ] Each commit's message describes its purpose and effects
  • [ x] There are no behavior changes unnecessary for the stated purpose of the PR

Recommended:

  • [ x] Each commit compiles and runs on my machine without known undesirable changes of behavior

Joining clips using the Join command (Ctrl+J) can cause data in preceeding clips to be silenced.

Fix: In WaveTrack::Join(), the initial value of t should be PlayStartTime of the first clip, not SequenceStartTime.
@AnitaBats AnitaBats added this to In progress in Sprint 8 - 3.1 Stabilisation via automation Nov 12, 2021
@AnitaBats AnitaBats moved this from In progress to Review in progress in Sprint 8 - 3.1 Stabilisation Nov 12, 2021
//preserve left trim data if any
newClip = CreateClip(t, clipsToDelete[0]->GetName());
newClip = CreateClip(clipsToDelete[0]->GetSequenceStartTime(),
Copy link
Collaborator

@vsverchinsky vsverchinsky Nov 12, 2021

Choose a reason for hiding this comment

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

A new clip has zero length, WaveClip::Paste then makes some checks to decide which data would be discarded, and in that case, the outcome depends on the order in which these cases are handled. So I guess I need to handle another special case in WaveClip::Paste so that pasted clip is fully preserved when the target clip is empty. Now if I make a selection strictly inside one clip, and then call join, then the hidden data on the right is lost.

Copy link
Collaborator Author

@DavidBailes DavidBailes Nov 12, 2021

Choose a reason for hiding this comment

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

So I guess I need to handle another special case in WaveClip::Paste so that pasted clip is fully preserved when the target clip is empty. Now if I make a selection strictly inside one clip, and then call join, then the hidden data on the right is lost.

An alternative fix for the reported bug, would have been to do something similar to void WaveTrack::MergeClips().
That is, not to delete clipsToDelete[0], but to use that as the "newClip", and paste the rest (+silence where necessary). I think this would avoid pasting into an empty clip.

Thanks for approving the commit!

Sprint 8 - 3.1 Stabilisation automation moved this from Review in progress to Reviewer approved Nov 12, 2021
@Paul-Licameli Paul-Licameli merged commit d45cadf into audacity:master Nov 16, 2021
4 checks passed
Sprint 8 - 3.1 Stabilisation automation moved this from Reviewer approved to Ready for QA Nov 16, 2021
@Penikov Penikov moved this from Ready for QA to Done in Sprint 8 - 3.1 Stabilisation Nov 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Data loss on joining clips
3 participants