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

Changes pruning algorithm for audio visualizer. #7139

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
70 changes: 41 additions & 29 deletions src/ui/Controls/AudioVisualizer.cs
Expand Up @@ -431,40 +431,52 @@ private void LoadParagraphs(Subtitle subtitle, int primarySelectedIndex, ListVie
const double additionalSeconds = 15.0; // Helps when scrolling
var startThresholdMilliseconds = (_startPositionSeconds - additionalSeconds) * TimeCode.BaseUnit;
var endThresholdMilliseconds = (EndPositionSeconds + additionalSeconds) * TimeCode.BaseUnit;
var displayableParagraphs = new List<Paragraph>();
for (var i = 0; i < subtitle.Paragraphs.Count; i++)
{
var p = subtitle.Paragraphs[i];

if (p.StartTime.IsMaxTime)
{
continue;
}

_subtitle.Paragraphs.Add(p);
if (p.EndTime.TotalMilliseconds >= startThresholdMilliseconds && p.StartTime.TotalMilliseconds <= endThresholdMilliseconds)
{
displayableParagraphs.Add(p);
if (displayableParagraphs.Count > 99)
{
break;
}
}
List<Paragraph> thresholdParagraphs = subtitle.Paragraphs.Where(p => !p.StartTime.IsMaxTime)
.Where(p => p.EndTime.TotalMilliseconds >= startThresholdMilliseconds && p.StartTime.TotalMilliseconds <= endThresholdMilliseconds).ToList();
_subtitle.Paragraphs.AddRange(thresholdParagraphs);

double startVisibleMilliseconds = _startPositionSeconds * TimeCode.BaseUnit;
double endVisibleMilliseconds = EndPositionSeconds * TimeCode.BaseUnit;
List<Paragraph> visibleParagraphs = thresholdParagraphs.Where(
p => p.EndTime.TotalMilliseconds >= startVisibleMilliseconds && p.StartTime.TotalMilliseconds <= endVisibleMilliseconds).ToList();
List<Paragraph> invisibleParagraphs = thresholdParagraphs.Except(visibleParagraphs).ToList();

const int maxDisplayableParagraphs = 100;
IEnumerable<Paragraph> displayableParagraphs = visibleParagraphs;
if (visibleParagraphs.Count > maxDisplayableParagraphs)
{
/*
* Group & select is done so that it draws paragraphs across the entire timeline before drawing overlapping paragraphs, up to the display limit.
* Materialize to list so that it can be counted below without pruning twice.
*/
displayableParagraphs = visibleParagraphs.Where(p => p.DurationTotalMilliseconds >= 0.01)
Copy link
Member

Choose a reason for hiding this comment

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

Too many LINQ. This can impact the performance!

Same for the rest of the code!

Every time you => an allocation will happen for action/func that is without considering the closure if you are using any variable from outside!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for taking a look! I didn't think the performance impact was too large with LINQ but I don't mind rewriting it to use traditional loops.

After creating this PR I noticed there's a SecondsToXPosition method that looks better than grouping by the same number of milliseconds no matter the zoom level. So I'll get both of these changes in this PR.

Copy link
Member

Choose a reason for hiding this comment

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

@niksedk is this something of your interest?

.GroupBy(p => Math.Floor(SecondsToXPosition(p.StartTime.TotalSeconds) / 5.0))
.SelectMany(group => group,
(group, p) =>
{
int index = group.ToList().IndexOf(p);
return new { index, p };
})
.OrderBy(items => items.index)
.Select(item => item.p)
.Take(maxDisplayableParagraphs)
.ToList();
}

displayableParagraphs = displayableParagraphs.OrderBy(p => p.StartTime.TotalMilliseconds).ToList();
var lastStartTime = -1d;
foreach (var p in displayableParagraphs)
if (displayableParagraphs.Count() < maxDisplayableParagraphs && invisibleParagraphs.Count + displayableParagraphs.Count() > maxDisplayableParagraphs)
{
if (displayableParagraphs.Count > 30 &&
(p.DurationTotalMilliseconds < 0.01 || p.StartTime.TotalMilliseconds - lastStartTime < 90))
{
continue;
}

_displayableParagraphs.Add(p);
lastStartTime = p.StartTime.TotalMilliseconds;
// These paragraphs won't be visible in the timeline and are in addition to visible paragraphs, so use simpler pruning algorithm to save time.
IEnumerable<Paragraph> additionalParagraphs = invisibleParagraphs.Where(p => p.DurationTotalMilliseconds >= 0.01)
.GroupBy(p => Math.Floor(SecondsToXPosition(p.StartTime.TotalSeconds) / 5.0))
.Select(p => p.First());
displayableParagraphs = displayableParagraphs.Concat(additionalParagraphs);
}
else
{
displayableParagraphs = displayableParagraphs.Concat(invisibleParagraphs);
}
_displayableParagraphs.AddRange(displayableParagraphs.Take(maxDisplayableParagraphs));

var primaryParagraph = subtitle.GetParagraphOrDefault(primarySelectedIndex);
if (primaryParagraph != null && !primaryParagraph.StartTime.IsMaxTime)
Expand Down