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

Video heatmap visuals #576

Merged
merged 30 commits into from
Feb 14, 2023
Merged

Video heatmap visuals #576

merged 30 commits into from
Feb 14, 2023

Conversation

mono424
Copy link
Collaborator

@mono424 mono424 commented Jul 4, 2022

Splitted the heatmap implementation into two parts:

  • Logging algorithm to track where people seek to
  • Visuals to display the heatmap
    This Pullrequest only contain the visuals to display the heatmap

Many Data Points:
Bildschirmfoto 2022-07-10 um 16 20 51

Few Data Points:
Bildschirmfoto 2022-07-10 um 16 22 53

@mono424 mono424 marked this pull request as ready for review December 3, 2022 20:05
Copy link
Collaborator

@MatthiasReumann MatthiasReumann left a comment

Choose a reason for hiding this comment

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

I think this is one of the coolest features implemented in the last couple of months. Great job. 🫡

Potential bug I found: When skipping forward (or backwards) the heat map wiggles a bit (Using Chrome):

wiggle.mov

Also make sure to merge dev properly into this branch, so that the bug fixed in #852 isn't reintroduced.

api/seek_stats.go Outdated Show resolved Hide resolved
web/ts/watch.ts Outdated Show resolved Hide resolved
web/ts/watch.ts Outdated Show resolved Hide resolved
web/ts/watch.ts Outdated Show resolved Hide resolved
web/template/watch.gohtml Outdated Show resolved Hide resolved
@mono424
Copy link
Collaborator Author

mono424 commented Feb 10, 2023

@MatthiasReumann the wiggeling is because of the time label that changes its length. It also changes the length of the seekbar, thats why the heatmap changes its size too. I didnt want to change so much inside video.js, but I see that this can be somehow anoying, what do you think, should we mess with videojs css and fix it, what do you think? :)

@MatthiasReumann
Copy link
Collaborator

@MatthiasReumann the wiggeling is because of the time label that changes its length. It also changes the length of the seekbar, thats why the heatmap changes its size too. I didnt want to change so much inside video.js, but I see that this can be somehow anoying, what do you think, should we mess with videojs css and fix it, what do you think? :)

Ouh. Definitely not worth the time.

@mono424
Copy link
Collaborator Author

mono424 commented Feb 12, 2023

@MatthiasReumann the wiggeling is because of the time label that changes its length. It also changes the length of the seekbar, thats why the heatmap changes its size too. I didnt want to change so much inside video.js, but I see that this can be somehow anoying, what do you think, should we mess with videojs css and fix it, what do you think? :)

Ouh. Definitely not worth the time.

Yes lets go for a followup PR 👍

Copy link
Collaborator

@MatthiasReumann MatthiasReumann left a comment

Choose a reason for hiding this comment

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

Looks great. Thanks again 🚀

@joschahenningsen joschahenningsen merged commit 8aabfa5 into dev Feb 14, 2023
@joschahenningsen joschahenningsen deleted the video-heatmap-visuals branch February 14, 2023 13:35
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.

3 participants