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

Add an option to show relative time to current time in timeline tooltip #760

Open
kasper93 opened this issue Nov 1, 2023 · 8 comments
Open

Comments

@kasper93
Copy link

kasper93 commented Nov 1, 2023

My request is to make tooltip more customizable. Like not only showing absolute time, but having option to show relative to current time. Or both of them like abs time (+-rel time)

Just an idea, I sometimes want to jump backward few minutes in long streams, it would save me doing the math in head.

Thanks

@kasper93 kasper93 changed the title Add an option to show relative time to current point in timeline tooltip Add an option to show relative time to current time in timeline tooltip Nov 1, 2023
@christoph-heinrich
Copy link
Contributor

christoph-heinrich commented Nov 1, 2023

Here is what the variant with both would look like
image
image

diff
diff --git a/src/uosc/elements/Timeline.lua b/src/uosc/elements/Timeline.lua
index 1bd1740..42c2bd5 100644
--- a/src/uosc/elements/Timeline.lua
+++ b/src/uosc/elements/Timeline.lua
@@ -401,8 +401,14 @@ function Timeline:render()
 			size = self.font_size, offset = timestamp_gap, margin = tooltip_gap, timestamp = options.time_precision > 0
 		}
 		local hovered_time_human = format_time(hovered_seconds, state.duration)
-		opts.width_overwrite = timestamp_width(hovered_time_human, opts)
-		tooltip_anchor = ass:tooltip(tooltip_anchor, hovered_time_human, opts)
+		-- due to floating point inaccuracies hovering the progress indicator switches between +0 and -0 without rounding
+		local rel_hovered_seconds = round((hovered_seconds - state.time) * 1e9) / 1e9
+		local rel_hovered_time_human = format_time(rel_hovered_seconds, state.duration)
+		local time_tooltip = rel_hovered_seconds >= 0 and
+			string.format('%s (+%s)', hovered_time_human, rel_hovered_time_human) or
+			string.format('%s (−%s)', hovered_time_human, rel_hovered_time_human:sub(2))
+		opts.width_overwrite = timestamp_width(time_tooltip, opts)
+		tooltip_anchor = ass:tooltip(tooltip_anchor, time_tooltip, opts)
 
 		-- Thumbnail
 		if not thumbnail.disabled
diff --git a/src/uosc/lib/ass.lua b/src/uosc/lib/ass.lua
index aff5e22..050c853 100644
--- a/src/uosc/lib/ass.lua
+++ b/src/uosc/lib/ass.lua
@@ -121,8 +121,8 @@ end
 function ass_mt:timestamp(x, y, align, timestamp, opts)
 	local widths, width_total = {}, 0
 	zero_rep = timestamp_zero_rep(timestamp)
-	for i = 1, #zero_rep do
-		local width = text_width(zero_rep:sub(i, i), opts)
+	for i, char in utf8_iter(zero_rep) do
+		local width = text_width(char, opts)
 		widths[i] = width
 		width_total = width_total + width
 	end
@@ -149,14 +149,16 @@ function ass_mt:timestamp(x, y, align, timestamp, opts)
 		opts.opacity = {main = opacity, primary = 0}
 		primary_opacity = opacity
 	end
-	for i, width in ipairs(widths) do
-		self:txt(x + width / 2, y, 5, timestamp:sub(i, i), opts)
+	for i, char in utf8_iter(timestamp) do
+		local width = widths[i]
+		self:txt(x + width / 2, y, 5, char, opts)
 		x = x + width
 	end
 	x = x - width_total
 	opts.opacity = {main = 0, primary = primary_opacity or 1}
-	for i, width in ipairs(widths) do
-		self:txt(x + width / 2, y, 5, timestamp:sub(i, i), opts)
+	for i, char in utf8_iter(timestamp) do
+		local width = widths[i]
+		self:txt(x + width / 2, y, 5, char, opts)
 		x = x + width
 	end
 	opts.opacity = opacity
diff --git a/src/uosc/lib/text.lua b/src/uosc/lib/text.lua
index 95852e2..f832bf5 100644
--- a/src/uosc/lib/text.lua
+++ b/src/uosc/lib/text.lua
@@ -69,7 +69,7 @@ end
 ---Iterates over utf-8 characters instead of bytes
 ---@param str string
 ---@return fun(): integer?, string?
-local function utf8_iter(str)
+function utf8_iter(str)
 	local byte_start = 1
 	return function()
 		local start = byte_start

I used the actual minus sign because it has the same width as the +, at least in my font, and that gets us right back to non monospace fonts causing things to shift around (#752).

Also that makes the minus inconsistent with the minus in the relative timeline time, but ofc we could use the "correct" minus there too.

@Sneakpeakcss
Copy link
Contributor

Here is what the variant with both would look like image image
diff

I used the actual minus sign because it has the same width as the +, at least in my font, and that gets us right back to non monospace fonts causing things to shift around (#752).

Also that makes the minus inconsistent with the minus in the relative timeline time, but ofc we could use the "correct" minus there too.

This is quite useful for livestreams. Would it be possible to limit displaying this additional tooltip only to cached parts of the timeline ?

@christoph-heinrich
Copy link
Contributor

Here is what the variant with both would look like image image
diff
I used the actual minus sign because it has the same width as the +, at least in my font, and that gets us right back to non monospace fonts causing things to shift around (#752).
Also that makes the minus inconsistent with the minus in the relative timeline time, but ofc we could use the "correct" minus there too.

This is quite useful for livestreams. Would it be possible to limit displaying this additional tooltip only to cached parts of the timeline ?

It's possible, but why is it desirable?

@Sneakpeakcss
Copy link
Contributor

Sneakpeakcss commented Nov 2, 2023

It's possible, but why is it desirable?

In case of the mentioned livestreams it's only possible to seek over the cached parts, so there's a lot of deadzone that's unusable:

image

And while this doesn't look that bad at the start, when the streams reaches hours of uptime, the cached part gets squished to the far right (around red mark), making this additional tooltip slightly redundant for 80% of the timeline.
From my perspective this tooltip isn't that useful in case of local files, since seeking is fast enough and you have complete control over the video, so my focus is mostly on streams where it would be used as additional visual indicator on top of functionality it provides.

@kasper93
Copy link
Author

kasper93 commented Nov 3, 2023

Making logic tied to cached parts would only make thing confusing for users. Also I don't think this feature should be particularity specific to one use-case.

Seeking outside cached region in live streams is completely different topic and if any solutions are implemented it wouldn't be tool tip related.

@Sneakpeakcss
Copy link
Contributor

Sneakpeakcss commented Nov 3, 2023

Making logic tied to cached parts would only make thing confusing for users. Also I don't think this feature should be particularity specific to one use-case.

Seeking outside cached region in live streams is completely different topic and if any solutions are implemented it wouldn't be tool tip related.

Oh no, i didn't meant to imply to make it completely exclusive to livestreamed content. And this being confusing to users is exactly my point.

Since uosc already takes streamed content into consideration with the default timeline_cache=yes setting, then i would argue that it would be more confusing to show this additional information outside of seekable parts.

And if any solution was ever implemented to block seeking outside the cached area, then i would think that it would also end up with this additional information hidden because it seems redundant to show it on parts that you cannot jump backwards to. Basically avoiding scenarios like this:

1h stream tooltip

[edit]
However, thinking about this more… There are cases like youtube where you technically can seek outside of cached parts (at least i think so, since for me it always ends up with endless buffering while using yt-dlp) so this might be a big issue towards what i'm suggesting.


Also the posted snippet is probably not the final solution, but i figured that i'll mention this, just in case:
For some reason the minus sign bugs out when time_precision is set above 0:

precision1

And weirdly enough, this only happens in mpv, but not in my mpvnet setup as seen on previous screenshot, while they both use the same font folder provided by uosc.

@christoph-heinrich
Copy link
Contributor

Also the posted snippet is probably not the final solution, but i figured that i'll mention this, just in case: For some reason the minus sign bugs out when time_precision is set above 0:

precision1

And weirdly enough, this only happens in mpv, but not in my mpvnet setup as seen on previous screenshot, while they both use the same font folder provided by uosc.

Thanks for reporting. That's because the code that's used when time_precision > 0 doesn't handle multi-byte utf-8 characters. I'll have to change that in the final implementation, if it comes to that.
It's weird that it doesn't happen in mpv.net.

@christoph-heinrich
Copy link
Contributor

The diff above has been updated to handle UTF-8 and deal with sign flickering when hovering the progress indicator.

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

No branches or pull requests

3 participants