From c2dd129b2089a2cfb2719bd425c08e92fd3e1742 Mon Sep 17 00:00:00 2001 From: "Martijn van Berkel (Flitskikker)" Date: Sun, 17 Dec 2023 22:35:54 +0100 Subject: [PATCH] Fix exceptions for edgecase with overlapping subtitles or a large "Treat as connected" setting --- src/Test/Logic/BeautifyTimeCodesTest.cs | 43 +++++++++++++++- src/libse/Common/ListExtensions.cs | 25 ++++++++- src/libse/Forms/TimeCodesBeautifier.cs | 67 ++++++++++++++++++++++--- 3 files changed, 125 insertions(+), 10 deletions(-) diff --git a/src/Test/Logic/BeautifyTimeCodesTest.cs b/src/Test/Logic/BeautifyTimeCodesTest.cs index 5d816aadf..f02966e24 100644 --- a/src/Test/Logic/BeautifyTimeCodesTest.cs +++ b/src/Test/Logic/BeautifyTimeCodesTest.cs @@ -425,7 +425,7 @@ namespace Test.Logic [TestMethod] public void TestFirstWithin() { - var shotChangesFrames = new List() { 1000, 10000, 20000 }; + var shotChangesFrames = new List() { 50, 1000, 3000, 5000, 10000, 11000, 14000, 15000, 16000, 20000, 22000 }; int? First(int start, int end) { @@ -455,6 +455,47 @@ namespace Test.Logic Assert.AreEqual(First(20001, 20001), shotChangesFrames.FirstWithin(20001, 20001)); Assert.AreEqual(First(30000, 30000), shotChangesFrames.FirstWithin(30000, 30000)); Assert.AreEqual(First(30000, 40000), shotChangesFrames.FirstWithin(30000, 40000)); + Assert.AreEqual(First(25000, 5000), shotChangesFrames.FirstWithin(25000, 5000)); + Assert.AreEqual(First(50, 60), shotChangesFrames.FirstWithin(50, 60)); + Assert.AreEqual(First(50, 1100), shotChangesFrames.FirstWithin(50, 1100)); + } + + [TestMethod] + public void TestLastWithin() + { + var shotChangesFrames = new List() { 50, 1000, 3000, 5000, 10000, 11000, 14000, 15000, 16000, 20000, 22000 }; + + int? Last(int start, int end) + { + try + { + return shotChangesFrames.Last(x => x >= start && x <= end); + } + catch (InvalidOperationException) + { + return null; + } + } + + Assert.AreEqual(Last(-100, 100000), shotChangesFrames.LastWithin(-100, 100000)); + Assert.AreEqual(Last(0, 100), shotChangesFrames.LastWithin(0, 100)); + Assert.AreEqual(Last(0, 1000), shotChangesFrames.LastWithin(0, 1000)); + Assert.AreEqual(Last(1000, 1000), shotChangesFrames.LastWithin(1000, 1000)); + Assert.AreEqual(Last(1000, 1100), shotChangesFrames.LastWithin(1000, 1100)); + Assert.AreEqual(Last(1100, 900), shotChangesFrames.LastWithin(1100, 900)); + Assert.AreEqual(Last(900, 15000), shotChangesFrames.LastWithin(900, 15000)); + Assert.AreEqual(Last(900, 30000), shotChangesFrames.LastWithin(900, 30000)); + Assert.AreEqual(Last(19999, 19999), shotChangesFrames.LastWithin(19999, 19999)); + Assert.AreEqual(Last(19999, 20000), shotChangesFrames.LastWithin(19999, 20000)); + Assert.AreEqual(Last(20000, 20000), shotChangesFrames.LastWithin(20000, 20000)); + Assert.AreEqual(Last(20000, 20001), shotChangesFrames.LastWithin(20000, 20001)); + Assert.AreEqual(Last(19999, 20001), shotChangesFrames.LastWithin(19999, 20001)); + Assert.AreEqual(Last(20001, 20001), shotChangesFrames.LastWithin(20001, 20001)); + Assert.AreEqual(Last(30000, 30000), shotChangesFrames.LastWithin(30000, 30000)); + Assert.AreEqual(Last(30000, 40000), shotChangesFrames.LastWithin(30000, 40000)); + Assert.AreEqual(Last(25000, 5000), shotChangesFrames.LastWithin(25000, 5000)); + Assert.AreEqual(Last(50, 60), shotChangesFrames.LastWithin(50, 60)); + Assert.AreEqual(Last(50, 1100), shotChangesFrames.LastWithin(50, 1100)); } } } diff --git a/src/libse/Common/ListExtensions.cs b/src/libse/Common/ListExtensions.cs index 4202cdecc..9eb5f54ea 100644 --- a/src/libse/Common/ListExtensions.cs +++ b/src/libse/Common/ListExtensions.cs @@ -153,7 +153,7 @@ namespace Nikse.SubtitleEdit.Core.Common } - // First within (int) + // First / last within (int) public static int? FirstWithin(this List orderedList, int start, int end) { @@ -177,5 +177,28 @@ namespace Nikse.SubtitleEdit.Core.Common return orderedList[startIndex]; } + + public static int? LastWithin(this List orderedList, int start, int end) + { + int startIndex = orderedList.BinarySearch(start); + int endIndex = orderedList.BinarySearch(end); + + if (startIndex < 0) + { + startIndex = ~startIndex; + } + + if (endIndex < 0) + { + endIndex = ~endIndex - 1; + } + + if (startIndex > endIndex || startIndex >= orderedList.Count || orderedList[startIndex] > end || endIndex >= orderedList.Count) + { + return null; + } + + return orderedList[endIndex]; + } } } \ No newline at end of file diff --git a/src/libse/Forms/TimeCodesBeautifier.cs b/src/libse/Forms/TimeCodesBeautifier.cs index b2fd215da..d28e48db7 100644 --- a/src/libse/Forms/TimeCodesBeautifier.cs +++ b/src/libse/Forms/TimeCodesBeautifier.cs @@ -133,14 +133,14 @@ namespace Nikse.SubtitleEdit.Core.Forms var leftInCueFrame = MillisecondsToFrames(leftParagraph.StartTime.TotalMilliseconds); var rightOutCueFrame = MillisecondsToFrames(rightParagraph.EndTime.TotalMilliseconds); - // Check result - if (bestLeftOutCueFrameInfo.result == FindBestCueResult.SnappedToRedZone && bestRightInCueFrameInfo.result == FindBestCueResult.SnappedToRedZone) + // Define align function for reusing + void AlignCuesAroundClosestShotChange(int leftShotChangeFrame, int rightShotChangeFrame) { - var fixInfoForLeft = GetFixedConnectedSubtitlesCueFrames(leftParagraph, rightParagraph, bestLeftOutCueFrameInfo.cueFrame); - var fixInfoForRight = GetFixedConnectedSubtitlesCueFrames(leftParagraph, rightParagraph, bestRightInCueFrameInfo.cueFrame); + var fixInfoForLeft = GetFixedConnectedSubtitlesCueFrames(leftParagraph, rightParagraph, leftShotChangeFrame); + var fixInfoForRight = GetFixedConnectedSubtitlesCueFrames(leftParagraph, rightParagraph, rightShotChangeFrame); - // Both are in red zones! We will use the closest shot change to align the cues around - if (Math.Abs(newLeftOutCueFrame - bestLeftOutCueFrameInfo.cueFrame) <= Math.Abs(newRightInCueFrame - bestRightInCueFrameInfo.cueFrame)) + // Calculate which shot change is closer + if (Math.Abs(newLeftOutCueFrame - leftShotChangeFrame) <= Math.Abs(newRightInCueFrame - rightShotChangeFrame)) { // Align around the left shot change // Except, when the left subtitle now becomes invalid (negative duration) and the right subtitle won't, we will use the right shot change anyway @@ -175,6 +175,13 @@ namespace Nikse.SubtitleEdit.Core.Forms } } } + + // Check result + if (bestLeftOutCueFrameInfo.result == FindBestCueResult.SnappedToRedZone && bestRightInCueFrameInfo.result == FindBestCueResult.SnappedToRedZone) + { + // Both are in red zones! We will use the closest shot change to align the cues around + AlignCuesAroundClosestShotChange(bestLeftOutCueFrameInfo.cueFrame, bestRightInCueFrameInfo.cueFrame); + } else if ((bestLeftOutCueFrameInfo.result == FindBestCueResult.SnappedToLeftGreenZone || bestLeftOutCueFrameInfo.result == FindBestCueResult.SnappedToRightGreenZone) && (bestRightInCueFrameInfo.result == FindBestCueResult.SnappedToLeftGreenZone || bestRightInCueFrameInfo.result == FindBestCueResult.SnappedToRightGreenZone)) { @@ -280,7 +287,24 @@ namespace Nikse.SubtitleEdit.Core.Forms } else if (bestLeftOutCueFrameInfo.result == FindBestCueResult.SnappedToLeftGreenZone) { - throw new InvalidOperationException("The left out cue cannot be snapped to the left side of a green zone while the right in cue is unaffected at the same time."); + // The left out cue wants to go backward, while the right in cue requires no action... The "Treat as connected" setting is probably really high. + // We'll use this function in case there are any shot changes closer to the right in cue. + var lastShotChangeInBetween = GetLastShotChangeFrameInBetween(bestLeftOutCueFrameInfo.cueFrame, bestRightInCueFrameInfo.cueFrame); + if (lastShotChangeInBetween != null) + { + // Derive left out cue shot change + var leftOutCueShotChange = bestLeftOutCueFrameInfo.cueFrame + Configuration.Settings.BeautifyTimeCodes.Profile.ConnectedSubtitlesLeftGreenZone; + + // We will use the closest shot change to align the cues around + AlignCuesAroundClosestShotChange(leftOutCueShotChange, lastShotChangeInBetween.Value); + } + else + { + // There are no shot changes at all between the two cues! That likely means they are overlapping. + // Since the left out cue requires a change, we'll accommodate that: put the right in cue on the edge of the green zone, and push the previous subtitle backward. + newRightInCueFrame = bestLeftOutCueFrameInfo.cueFrame; + newLeftOutCueFrame = newRightInCueFrame - Configuration.Settings.BeautifyTimeCodes.Profile.Gap; + } } else if (bestLeftOutCueFrameInfo.result == FindBestCueResult.SnappedToRightGreenZone) { @@ -296,7 +320,24 @@ namespace Nikse.SubtitleEdit.Core.Forms } else if (bestRightInCueFrameInfo.result == FindBestCueResult.SnappedToRightGreenZone) { - throw new InvalidOperationException("The right in cue cannot be snapped to the right side of a green zone while the left out cue is unaffected at the same time."); + // The right in cue wants to go forward, while the left out cue requires no action... The "Treat as connected" setting is probably really high. + // We'll use this function in case there are any shot changes closer to the left out cue. + var firstShotChangeInBetween = GetFirstShotChangeFrameInBetween(bestLeftOutCueFrameInfo.cueFrame, bestRightInCueFrameInfo.cueFrame); + if (firstShotChangeInBetween != null) + { + // Derive right in cue shot change + var rightInCueShotChange = bestRightInCueFrameInfo.cueFrame - Configuration.Settings.BeautifyTimeCodes.Profile.ConnectedSubtitlesRightGreenZone; + + // We will use the closest shot change to align the cues around + AlignCuesAroundClosestShotChange(firstShotChangeInBetween.Value, rightInCueShotChange); + } + else + { + // There are no shot changes at all between the two cues! That likely means they are overlapping. + // Since the right in cue requires a change, we'll accommodate that: put the left out cue on the edge of the green zone, and push the next subtitle forward. + newLeftOutCueFrame = bestRightInCueFrameInfo.cueFrame; + newRightInCueFrame = newLeftOutCueFrame + Configuration.Settings.BeautifyTimeCodes.Profile.Gap; + } } else { @@ -939,6 +980,16 @@ namespace Nikse.SubtitleEdit.Core.Forms return _shotChangesFrames.FirstWithin(leftCueFrame, rightCueFrame); } + private int? GetLastShotChangeFrameInBetween(int leftCueFrame, int rightCueFrame) + { + if (_shotChangesFrames == null || _shotChangesFrames.Count == 0) + { + return null; + } + + return _shotChangesFrames.LastWithin(leftCueFrame, rightCueFrame); + } + private int? GetClosestShotChangeFrame(int cueFrame) { if (_shotChangesFrames == null || _shotChangesFrames.Count == 0)