Skip to content

(Feature) SeekBar Highlighting: Highlight section of seekbar where subtitles have been generated #97

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

KRSHH
Copy link
Contributor

@KRSHH KRSHH commented Apr 27, 2025

image

Copy link
Owner

@umlx5h umlx5h left a comment

Choose a reason for hiding this comment

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

Hi @KRSHH, Thanks for the PR!
I've made some comments, it would be appreciated if you could fix it.


When ASR is started in the middle, the first half of the seek bar seems to be highlighted as well.
So the starting point may also need to be managed. Is this difficult to solve?

(You have to move the seek bar to the left most to see it.)

I want to add the ability to wrap ASR in the future, so it may be possible to fix it at that time.

VerticalAlignment="Center"
Visibility="Collapsed"
Margin="0,0,0,0"
Panel.ZIndex="2" />
Copy link
Owner

Choose a reason for hiding this comment

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

In the case of the same Row and Column, the one written on the bottom has priority and is displayed on the top.

Therefore, if you want to place this Border at the bottom, it is better to define it at the top.
Please control zindex in the order you define.

// Update the latest subtitle time if this one is later
if (data.EndTime.Ticks > _latestSubtitleTime)
{
LatestSubtitleTime = data.EndTime.Ticks;
Copy link
Owner

Choose a reason for hiding this comment

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

It may be better to add a timer and update when X seconds have elapsed.
Frequent UI updates using dispatcher should be avoided for performance reasons.


// When ASR completes successfully, set the latest subtitle time to the full duration
// This will fill the entire seekbar with the yellow highlight
if (_config?.Subtitles?.player?.Duration > 0)
Copy link
Owner

Choose a reason for hiding this comment

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

Is this if statement necessary?

if (_latestSubtitleTime != value)
{
_latestSubtitleTime = value;
Utils.UI(() => Raise(nameof(LatestSubtitleTime)));
Copy link
Owner

Choose a reason for hiding this comment

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

SetUI(ref _latestSubtitleTime, value);

<Border
x:Name="subtitleProcessingTrack"
Grid.Row="1"
Background="#FFEE22"
Copy link
Owner

Choose a reason for hiding this comment

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

It would be better to avoid hard-corded colors for custom theme.

The following colors may be a good choice

// white color
Background="{DynamicResource MaterialDesignBody}"

// lighter primary color
Background="{DynamicResource MaterialDesign.Brush.Primary.Light}"

@@ -210,6 +236,12 @@ public bool Execute(int subIndex, string url, int streamIndex, MediaType type, T

lock (_lockerSubs)
{
// Update the latest subtitle time if this one is later
if (data.EndTime.Ticks > _latestSubtitleTime)
Copy link
Owner

Choose a reason for hiding this comment

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

This if statement may not be necessary since currently it removes everything after the added subtitle.

KRSHH added 2 commits April 28, 2025 17:16
…ering, removed hardcoding in color of highlight, removed redundant statements
@KRSHH
Copy link
Contributor Author

KRSHH commented Apr 28, 2025

Hi @umlx5h I have made few changes in my previous commit

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.

2 participants