diff --git a/src/models/AppController.cpp b/src/models/AppController.cpp index 7c59791a..fbecce81 100644 --- a/src/models/AppController.cpp +++ b/src/models/AppController.cpp @@ -2000,71 +2000,258 @@ drift::Clip makeAudioCompanionFromVideo(const drift::Clip &videoClip, const QStr return audio; } -// Split embedded audio onto the audio track (video keeps picture only). -// CapCut-style: the new audio clip stays linked to the video so they move together -// until the user explicitly unlinks. -bool detachEmbeddedAudioFromVideo(drift::Project &project, AssetLibrary *library, drift::Clip &videoClip) +// Relative position of a video track inside the video-track group. +// +// Example: +// +// project tracks: Video, Video, Video, Audio, Audio +// 0 1 2 +// +// The result is independent of absolute project-track indexes so audio tracks can +// mirror the video hierarchy without interleaving video and audio tracks. +int videoTrackOrdinal(const drift::Project &project, int trackIndex) +{ + if (trackIndex < 0 || trackIndex >= project.tracks().size()) + return -1; + if (project.tracks().at(trackIndex).type != drift::TrackType::Video) + return -1; + + int ordinal = 0; + for (int t = 0; t < trackIndex; ++t) { + if (project.tracks().at(t).type == drift::TrackType::Video) + ++ordinal; + } + return ordinal; +} + +// Returns the video-track ordinal associated with a linked audio track. +// +// Detached A/V companions already carry the same linkId, so there is no need to +// add persistent track metadata merely to determine their visual hierarchy. +int ownerVideoOrdinalForAudioTrack(const drift::Project &project, int audioTrackIndex) { + if (audioTrackIndex < 0 || audioTrackIndex >= project.tracks().size()) + return -1; + + const drift::Track &audioTrack = project.tracks().at(audioTrackIndex); + if (audioTrack.type != drift::TrackType::Audio) + return -1; + + for (const drift::Clip &audioClip : audioTrack.clips) { + if (audioClip.linkId.isEmpty()) + continue; + + for (int t = 0; t < project.tracks().size(); ++t) { + const drift::Track &videoTrack = project.tracks().at(t); + if (videoTrack.type != drift::TrackType::Video) + continue; + + for (const drift::Clip &videoClip : videoTrack.clips) { + if (!videoClip.linkId.isEmpty() + && videoClip.linkId == audioClip.linkId) { + return videoTrackOrdinal(project, t); + } + } + } + } + + // Ordinary/imported audio tracks do not have a linked video owner. + return -1; +} + +// Find where a newly detached audio track belongs. +// +// Rules: +// 1. Video tracks remain together. +// 2. Detached audio tracks remain together below the video group. +// 3. Their relative order mirrors their source video tracks. +// 4. Unrelated audio tracks are kept below the linked companion group. +// 5. Existing companion tracks are shifted down instead of receiving an +// overlapping clip. +int audioTrackInsertIndexForVideoTrack(const drift::Project &project, + int sourceVideoTrackIndex) +{ + const int sourceOrdinal = + videoTrackOrdinal(project, sourceVideoTrackIndex); + + if (sourceOrdinal < 0) + return project.tracks().size(); + + int lastVideoTrack = -1; + int firstAudioTrack = -1; + int lastAudioTrack = -1; + + for (int t = 0; t < project.tracks().size(); ++t) { + const drift::TrackType type = project.tracks().at(t).type; + + if (type == drift::TrackType::Video) + lastVideoTrack = t; + + if (type != drift::TrackType::Audio) + continue; + + if (firstAudioTrack < 0) + firstAudioTrack = t; + + lastAudioTrack = t; + + const int ownerOrdinal = + ownerVideoOrdinalForAudioTrack(project, t); + + // An unrelated audio track marks the end of the linked-companion block. + if (ownerOrdinal < 0) + return t; + + // Insert before the first companion belonging to a lower video track. + if (ownerOrdinal > sourceOrdinal) + return t; + } + + // No audio tracks yet: begin the audio group immediately below all videos. + if (firstAudioTrack < 0) + return qBound(0, lastVideoTrack + 1, project.tracks().size()); + + // All existing companion tracks belong to videos above this one. + return lastAudioTrack + 1; +} + +// Split embedded audio onto its own audio track (video keeps picture only). +// +// The new audio clip remains linked to the video, but receives a dedicated +// track whose relative position mirrors the source video track. +bool detachEmbeddedAudioFromVideo(drift::Project &project, + AssetLibrary *library, + int videoTrackIndex, + int videoClipIndex) +{ + if (videoTrackIndex < 0 + || videoTrackIndex >= project.tracks().size()) + return false; + + if (videoClipIndex < 0 + || videoClipIndex >= project.tracks().at(videoTrackIndex).clips.size()) + return false; + + drift::Clip &videoClip = + project.tracks()[videoTrackIndex].clips[videoClipIndex]; + if (!clipHasEmbeddedAudio(project, library, videoClip)) return false; - const QString linkId = videoClip.linkId.isEmpty() - ? QUuid::createUuid().toString(QUuid::WithoutBraces) - : videoClip.linkId; + const QString linkId = + videoClip.linkId.isEmpty() + ? QUuid::createUuid().toString(QUuid::WithoutBraces) + : videoClip.linkId; + videoClip.linkId = linkId; videoClip.suppressEmbeddedAudio = true; - const int audioTrack = drift::ensureTrackForClipType(project, drift::ClipType::Audio, false); - project.tracks()[audioTrack].clips.append(makeAudioCompanionFromVideo(videoClip, linkId, videoClip.audioStreamIndex)); + // Build the companion before inserting into project.tracks(); QList insertion + // may relocate Track objects and invalidate references into that container. + const drift::Clip audioClip = + makeAudioCompanionFromVideo( + videoClip, + linkId, + videoClip.audioStreamIndex); + + const int insertAt = + audioTrackInsertIndexForVideoTrack( + project, + videoTrackIndex); + + drift::Track audioTrack; + audioTrack.type = drift::TrackType::Audio; + audioTrack.clips.append(audioClip); + + project.tracks().insert(insertAt, audioTrack); + return true; } -// Split all embedded audio streams onto separate audio tracks (video keeps picture only). -bool detachAllAudioTracksFromVideo(drift::Project &project, AssetLibrary *library, drift::Clip &videoClip) -{ +// Split every embedded audio stream onto dedicated, consecutive audio tracks. +// +// All streams from the same video stay together, and that group follows the +// source video's position relative to the other video tracks. +bool detachAllAudioTracksFromVideo(drift::Project &project, + AssetLibrary *library, + int videoTrackIndex, + int videoClipIndex) +{ + if (videoTrackIndex < 0 + || videoTrackIndex >= project.tracks().size()) + return false; + + if (videoClipIndex < 0 + || videoClipIndex >= project.tracks().at(videoTrackIndex).clips.size()) + return false; + + drift::Clip &videoClip = + project.tracks()[videoTrackIndex].clips[videoClipIndex]; + if (!clipHasEmbeddedAudio(project, library, videoClip)) return false; - const QList streams = MediaProbe::audioStreams(videoClip.path); - if (streams.isEmpty()) - return detachEmbeddedAudioFromVideo(project, library, videoClip); + const QList streams = + MediaProbe::audioStreams(videoClip.path); + + if (streams.isEmpty()) { + return detachEmbeddedAudioFromVideo( + project, + library, + videoTrackIndex, + videoClipIndex); + } + + const QString linkId = + videoClip.linkId.isEmpty() + ? QUuid::createUuid().toString(QUuid::WithoutBraces) + : videoClip.linkId; - const QString linkId = videoClip.linkId.isEmpty() - ? QUuid::createUuid().toString(QUuid::WithoutBraces) - : videoClip.linkId; videoClip.linkId = linkId; videoClip.suppressEmbeddedAudio = true; + QList companions; + companions.reserve(streams.size()); + for (int i = 0; i < streams.size(); ++i) { - const StreamInfo &s = streams.at(i); + const StreamInfo &stream = streams.at(i); + QString trackName = videoClip.name; - if (!s.title.isEmpty()) { - trackName = QStringLiteral("%1 (%2)").arg(videoClip.name, s.title); + + if (!stream.title.isEmpty()) { + trackName = + QStringLiteral("%1 (%2)") + .arg(videoClip.name, stream.title); } else if (streams.size() > 1) { - trackName = QStringLiteral("%1 (Audio %2)").arg(videoClip.name).arg(i + 1); + trackName = + QStringLiteral("%1 (Audio %2)") + .arg(videoClip.name) + .arg(i + 1); } - int targetTrack = -1; - int audioTrackCount = 0; - for (int t = 0; t < project.tracks().size(); ++t) { - if (project.tracks()[t].type == drift::TrackType::Audio) { - if (audioTrackCount == i) { - targetTrack = t; - break; - } - ++audioTrackCount; - } - } - if (targetTrack < 0) { - drift::Track newTrack; - newTrack.type = drift::TrackType::Audio; - project.tracks().append(newTrack); - targetTrack = project.tracks().size() - 1; - } + companions.append( + makeAudioCompanionFromVideo( + videoClip, + linkId, + i, + trackName)); + } + + const int insertAt = + audioTrackInsertIndexForVideoTrack( + project, + videoTrackIndex); + + for (int i = 0; i < companions.size(); ++i) { + drift::Track audioTrack; + audioTrack.type = drift::TrackType::Audio; + audioTrack.clips.append(companions.at(i)); - project.tracks()[targetTrack].clips.append( - makeAudioCompanionFromVideo(videoClip, linkId, i, trackName)); + project.tracks().insert( + insertAt + i, + audioTrack); } + return true; } @@ -9831,8 +10018,15 @@ void AppController::separateAudioFromSelection() drift::Clip &clip = m_project.tracks()[pair.first].clips[pair.second]; if (clip.type != drift::ClipType::Video || detachedVideoIds.contains(clip.id)) continue; - if (detachEmbeddedAudioFromVideo(m_project, m_assetLibrary, clip)) { - detachedVideoIds.insert(clip.id); + + const QString videoId = clip.id; + + if (detachEmbeddedAudioFromVideo( + m_project, + m_assetLibrary, + pair.first, + pair.second)) { + detachedVideoIds.insert(videoId); changed = true; } } @@ -9857,7 +10051,11 @@ void AppController::separateAllAudioTracks(int trackIndex, int clipIndex) if (clip.type != drift::ClipType::Video) return; - if (!detachAllAudioTracksFromVideo(m_project, m_assetLibrary, clip)) + if (!detachAllAudioTracksFromVideo( + m_project, + m_assetLibrary, + trackIndex, + clipIndex)) return; m_selection = selectionWithLinkedPartners(m_project, trackIndex, clipIndex); @@ -9883,8 +10081,15 @@ void AppController::separateAllAudioTracksFromSelection() drift::Clip &clip = m_project.tracks()[pair.first].clips[pair.second]; if (clip.type != drift::ClipType::Video || detachedVideoIds.contains(clip.id)) continue; - if (detachAllAudioTracksFromVideo(m_project, m_assetLibrary, clip)) { - detachedVideoIds.insert(clip.id); + + const QString videoId = clip.id; + + if (detachAllAudioTracksFromVideo( + m_project, + m_assetLibrary, + pair.first, + pair.second)) { + detachedVideoIds.insert(videoId); changed = true; } } diff --git a/src/qml/components/timeline/TimelineClipItem.qml b/src/qml/components/timeline/TimelineClipItem.qml index 03a5b6ed..165874ca 100644 --- a/src/qml/components/timeline/TimelineClipItem.qml +++ b/src/qml/components/timeline/TimelineClipItem.qml @@ -24,6 +24,26 @@ Item { // Wider trim/move hit areas for phones; desktop leaves this false. property bool touchMode: false + // Desktop timeline deletion is intentionally scoped to the clip that owns + // keyboard focus. Clicking a clip already calls forceActiveFocus(), so this + // does not steal Delete from the Media Bin, dialogs or other editor surfaces. + // + // macOS sends the key labelled Delete on MacBook keyboards as Backspace; + // extended keyboards can also send the forward-delete Key_Delete. + // + // deleteSelectedClip() owns the A/V semantics: + // linked pair -> delete the whole linked set + // unlinked pair -> delete only the current selection + Keys.onPressed: function(event) { + if ((event.key === Qt.Key_Delete + || event.key === Qt.Key_Backspace) + && clipItem.selected) { + + EditorState.deleteSelectedClip() + event.accepted = true + } + } + property var clipData: panel.tracks[trackIndex].clips[clipIndex] property bool selected: (EditorState.selection, EditorState.selectionContains(trackIndex, clipIndex)) diff --git a/tests/tst_editorstate.cpp b/tests/tst_editorstate.cpp index 694e7873..e34adc8c 100644 --- a/tests/tst_editorstate.cpp +++ b/tests/tst_editorstate.cpp @@ -94,7 +94,9 @@ private slots: void replaceTransitionOnDrop(); void overlapAutoAppliesCrossfade(); void separateAudioFromCombinedClip(); + void separatedAudioTracksMirrorVideoHierarchy(); void linkedAudioUnlinkAndMove(); + void deleteLinkedPairTogetherAndUnlinkedClipAlone(); void linkedFadeCurveSyncsPartner(); void customFadeCurveSessionApplyAndCancel(); void keyframeGraphPropertySelection(); @@ -1838,6 +1840,154 @@ void EditorStateTest::separateAudioFromCombinedClip() QVERIFY(!state.canSeparateAudioSelection()); } + +// Delete follows the same relationship semantics as move/trim: +// +// linked video + audio +// deleting either side removes the complete pair. +// +// unlinked video + audio +// deleting one side leaves the other side untouched. +void EditorStateTest::deleteLinkedPairTogetherAndUnlinkedClipAlone() +{ + auto countClipsOfType = + [](const drift::Project &project, + drift::ClipType type) { + + int count = 0; + + for (const drift::Track &track : project.tracks()) { + for (const drift::Clip &clip : track.clips) { + if (clip.type == type) + ++count; + } + } + + return count; + }; + + AssetLibrary library; + AppController state(&library); + + appendLinkedVideoAudioPair( + *state.project()); + + QCOMPARE( + countClipsOfType( + *state.project(), + drift::ClipType::Video), + 1); + + QCOMPARE( + countClipsOfType( + *state.project(), + drift::ClipType::Audio), + 1); + + // -------------------------------------------------------- + // LINKED: + // selecting the audio and deleting it must remove BOTH. + // -------------------------------------------------------- + + state.selectClip(1, 0); + + QVERIFY( + state.canUnlinkSelection()); + + state.deleteSelectedClip(); + + QCOMPARE( + countClipsOfType( + *state.project(), + drift::ClipType::Video), + 0); + + QCOMPARE( + countClipsOfType( + *state.project(), + drift::ClipType::Audio), + 0); + + // The pair deletion is one project edit. + state.undo(); + + QCOMPARE( + countClipsOfType( + *state.project(), + drift::ClipType::Video), + 1); + + QCOMPARE( + countClipsOfType( + *state.project(), + drift::ClipType::Audio), + 1); + + // -------------------------------------------------------- + // UNLINK: + // break the relationship first. + // -------------------------------------------------------- + + state.selectClip(0, 0); + + QVERIFY( + state.canUnlinkSelection()); + + state.unlinkSelectedClips(); + + QVERIFY( + !state.canUnlinkSelection()); + + // Re-select ONLY the audio after unlinking. + // + // The track indexes remain Video 0 / Audio 1 here. + state.selectClip(1, 0); + + QCOMPARE( + state.selection().size(), + 1); + + // -------------------------------------------------------- + // UNLINKED: + // Delete must remove ONLY the audio. + // -------------------------------------------------------- + + state.deleteSelectedClip(); + + QCOMPARE( + countClipsOfType( + *state.project(), + drift::ClipType::Video), + 1); + + QCOMPARE( + countClipsOfType( + *state.project(), + drift::ClipType::Audio), + 0); + + // Undo restores just that audio deletion. + state.undo(); + + QCOMPARE( + countClipsOfType( + *state.project(), + drift::ClipType::Video), + 1); + + QCOMPARE( + countClipsOfType( + *state.project(), + drift::ClipType::Audio), + 1); + + // They must remain unlinked after undoing only the deletion. + state.selectClip(0, 0); + + QVERIFY( + !state.canUnlinkSelection()); +} + void EditorStateTest::linkedFadeCurveSyncsPartner() { AssetLibrary library; @@ -1898,6 +2048,153 @@ void EditorStateTest::customFadeCurveSessionApplyAndCancel() QVERIFY(qAbs(state.project()->tracks().at(1).clips.at(0).fadeShape.gainAt(0.5) - 0.75) < 1e-6); } + +// Audio separation must mirror the vertical video hierarchy, regardless of the +// order in which the user performs the operation. +// +// Separate in deliberately scrambled order: +// +// Video 3 +// Video 1 +// Video 2 +// +// The final layout must still be: +// +// Video 1 +// Video 2 +// Video 3 +// Audio 1 +// Audio 2 +// Audio 3 +// +// Each audio clip also keeps the linkId of its corresponding video. +void EditorStateTest::separatedAudioTracksMirrorVideoHierarchy() +{ + AssetLibrary library; + AppController state(&library); + + drift::Project *project = state.project(); + project->tracks().clear(); + + for (int i = 0; i < 3; ++i) { + const QString suffix = QString::number(i + 1); + const QString assetId = + QStringLiteral("hierarchy-asset-%1").arg(suffix); + + drift::MediaAsset asset; + asset.id = assetId; + asset.name = + QStringLiteral("video-%1.mp4").arg(suffix); + asset.path = + QStringLiteral("/tmp/video-%1.mp4").arg(suffix); + asset.kind = drift::MediaKind::Video; + asset.durationUs = drift::secondsToUs(5.0); + asset.hasAudioKnown = true; + asset.hasAudio = true; + asset.channels = 2; + asset.sampleRate = 48000; + + project->assets().insert(asset.id, asset); + project->assetOrder().append(asset.id); + + drift::Clip clip; + clip.id = + QStringLiteral("hierarchy-video-%1").arg(suffix); + clip.assetId = asset.id; + clip.name = asset.name; + clip.path = asset.path; + clip.type = drift::ClipType::Video; + clip.timelineStart = 0; + clip.timelineDuration = drift::secondsToUs(5.0); + clip.srcIn = 0; + clip.srcOut = drift::secondsToUs(5.0); + + drift::Track track; + track.type = drift::TrackType::Video; + track.clips.append(clip); + + project->tracks().append(track); + } + + library.syncToProject(); + + QCOMPARE(project->tracks().size(), 3); + + // Intentionally separate out of visual order. + state.selectClip(2, 0); + state.separateAudioFromSelection(); + + state.selectClip(0, 0); + state.separateAudioFromSelection(); + + state.selectClip(1, 0); + state.separateAudioFromSelection(); + + QCOMPARE(project->tracks().size(), 6); + + // Videos stay grouped at the top. + for (int i = 0; i < 3; ++i) { + QCOMPARE( + project->tracks().at(i).type, + drift::TrackType::Video); + + QCOMPARE( + project->tracks().at(i).clips.size(), + 1); + } + + // Audio stays grouped below the videos. + for (int i = 0; i < 3; ++i) { + const int audioTrackIndex = 3 + i; + + QCOMPARE( + project->tracks().at(audioTrackIndex).type, + drift::TrackType::Audio); + + QCOMPARE( + project->tracks().at(audioTrackIndex).clips.size(), + 1); + + const drift::Clip &video = + project->tracks().at(i).clips.at(0); + + const drift::Clip &audio = + project->tracks().at(audioTrackIndex).clips.at(0); + + QCOMPARE(audio.assetId, video.assetId); + + QVERIFY(!video.linkId.isEmpty()); + + QCOMPARE(audio.linkId, video.linkId); + + QVERIFY(video.suppressEmbeddedAudio); + } + + // One undo reverts only the most recent separation. + state.undo(); + + QCOMPARE(project->tracks().size(), 5); + + // Redo must reconstruct exactly the same hierarchy. + state.redo(); + + QCOMPARE(project->tracks().size(), 6); + + for (int i = 0; i < 3; ++i) { + QCOMPARE( + project->tracks().at(i).type, + drift::TrackType::Video); + + QCOMPARE( + project->tracks().at(3 + i).type, + drift::TrackType::Audio); + + QCOMPARE( + project->tracks().at(3 + i).clips.at(0).linkId, + project->tracks().at(i).clips.at(0).linkId); + } +} + void EditorStateTest::linkedAudioUnlinkAndMove() { AssetLibrary library;