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

Most replayed Heatmap (Logging) #575

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 contains the logging logic.

@mono424 mono424 linked an issue Jul 4, 2022 that may be closed by this pull request
@mono424 mono424 marked this pull request as ready for review July 4, 2022 19:29
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 am not sure if this PR is reviewable yet. However, I was super interested in how you implemented it, so, if it isn't opened officially let's call this early feedback. 😅

Nevertheless, looks pretty cool 💯

dao/video-seek.go Show resolved Hide resolved
api/progress.go Outdated Show resolved Hide resolved
web/ts/global.ts Outdated Show resolved Hide resolved
dao/video-seek.go Outdated Show resolved Hide resolved
@mono424
Copy link
Collaborator Author

mono424 commented Jul 5, 2022

@MatthiasReumann Thank you for your feedback!

@MatthiasReumann @joschahenningsen Lets discuss if its probably better to use progressReport to increment section counters for the video :).

@mono424
Copy link
Collaborator Author

mono424 commented Jul 9, 2022

I am not sure if this PR is reviewable yet. However, I was super interested in how you implemented it, so, if it isn't opened officially let's call this early feedback. 😅

Nevertheless, looks pretty cool 💯

Thanks for your feedback! The testutils you have written are pretty sick!! 🚀

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 💯

If this isn't merged before #549 you can even use the **new and improved** testutils. Then testutils.First(...) and json.Marshal(...) are not necessary anymore and you can specify the body as

{
    ...
    Body: gin.H{"position": testPosition}
    ...
}

I've left a comment regarding security concerns. But we might want to discuss that in a meeting.

api/seek_stats.go Show resolved Hide resolved
@mono424 mono424 merged commit 8560855 into dev Jul 16, 2022
@mono424 mono424 deleted the 519-most-replayed-feature-for-identifying-most-popular-parts-of-videos branch July 16, 2022 13:37
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.

'Most Replayed' Feature for Identifying Most Popular Parts of Videos
2 participants