Add chapter markings to video player slider #3745

Viperinius · 2022-07-03T15:55:04Z

Motivation
I wanted to be able to see when movie chapters start without moving the slider bubble through the whole timeline. Also inspired by this feature request.

Changes

Added container for chapter marks to the video OSD page.
Added functions to add and calculate the icon positions.
Those calculations are done in updateValues of the slider.
The markings are shown or hidden together with / the same way as the slider bubble.
When the chapter mark has been passed / watched, the colour changes to blue.
The icon is set with CSS and can thus be easily customised by the user.
Customisation is further possible by being able to use different icons depending on the chapter name (e. g. to use a special icon if the chapter name is something like "intro").

What it looks like
(shown with the Jungle Book from the demo server)

Possible next steps
The customisation "per-chapter" could define some default CSS values to use different icons for common chapter "types", e.g.:

.sliderChapterMark.scm-intro::before {
    content: "\e8da"; /* theaters */
}
.sliderChapterMark.scm-credits::before {
    content: "\ef42"; /* article */
}

These labels show the start of each chapter when interacting with the slider the same way that activates the slider bubble. They follow the same color scheme as the slider (watched chapters turn blue). Inspired by https://features.jellyfin.org/posts/397/chapter-markers-in-timeline

Allows easier customisation of what icon should be displayed.

In order to use different icons depending on the chapter name, the name is provided as a class with the prefix scm-.

src/controllers/playback/video/index.js

dmitrylyzo · 2022-07-04T08:46:33Z

src/elements/emby-slider/emby-slider.js

+            if (!bubbleTrackRect.width || !chapterMarkRect.width) {
+                // width is not set, most probably because the OSD is currently hidden
+                return;
+            }
+
+            let chapterMarkPos = (bubbleTrackRect.width * valueChapterMark / 100) - chapterMarkRect.width / 2;
+            chapterMarkPos = Math.min(Math.max(chapterMarkPos, - chapterMarkRect.width / 2), bubbleTrackRect.width - chapterMarkRect.width / 2);


I guess that's why you did showOsd in getChapterFractions.
What about resizing?

Well, the early return if the width is 0 was just meant to prevent recalculating the marker positions even if the OSD is hidden. This lead to noticeable jumps of the markers when the OSD is shown again and the width is correct again.

About the resizing: Could you elaborate?
Do you mean if the positioning can handle a resize during playback? If so, then yes, as long as updateValues is continued to be called.

src/controllers/playback/video/index.js

More generic name without the inclusion of "chapters" allows potential reuse in the future.

sonarcloud · 2022-07-05T15:19:13Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

thornbill

First off this is a really cool feature! 🎉

I did not finish reviewing the emby-slider.js changes, because I'm not a fan of how the hiding works currently. I think the markers should just be lines that show up inline with the slider that are just visible anytime the slider is shown. The large icons make me think I should be able to click them, and they are kind intrusive and difficult to get to display.

src/controllers/playback/video/index.js

Viperinius · 2022-10-01T15:28:13Z

I think the markers should just be lines that show up inline with the slider that are just visible anytime the slider is shown. The large icons make me think I should be able to click them, and they are kind intrusive and difficult to get to display.

Yeah, I get that, especially being able to click them. I quickly experimented with the CSS a bit and got this:

Is this what you had in mind?

(As these are just CSS changes, the "icon way" can still be used with custom server CSS.)

thornbill · 2022-10-01T17:07:10Z

That looks great!

Now, the markers are displayed as ticks instead of icons above the slider

Viperinius · 2022-10-01T18:14:18Z

Alright, I added the changed CSS.

Concerning the hiding / positioning, I am open to any suggestions to improve it

thornbill · 2022-10-03T14:41:14Z

The changes look great!

I think I would just leave the markers visible when the OSD is shown instead of requiring hover.

(Does not require hovering over the slider anymore.)

Viperinius · 2022-10-05T16:03:01Z

Good suggestion, saves a bit of code... and done :)

thornbill

This is looking great! I just had a couple naming suggestions.

src/elements/emby-slider/emby-slider.js

sonarcloud · 2022-10-06T15:01:00Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
0.0% Duplication

DoTheSneedful · 2024-03-12T02:50:47Z

I really want this feature but I noticed it's not in any release since being merged to master over a year ago, certainly not available in the 10.8 version I'm currently using. I am not familiar with Jellyfin's process, so I hope it's not too much trouble to ask what the ETA for something like this is?

Viperinius and others added 6 commits June 26, 2022 01:10

Set chapter mark icon via CSS instead of class.

c0b092e

Allows easier customisation of what icon should be displayed.

Merge branch 'jellyfin:master' into chapter-markers

ca84407

Add name to CONTRIBUTORS

cf22dd0

Add generic font family to chapter mark

20a1c34

Change chapter mark classes to include name info.

b78d643

In order to use different icons depending on the chapter name, the name is provided as a class with the prefix scm-.

dmitrylyzo reviewed Jul 5, 2022

View reviewed changes

Viperinius added 3 commits July 5, 2022 16:31

Use object to return multiple info instead of array.

ba2c365

Update function and class names as suggested.

0407252

More generic name without the inclusion of "chapters" allows potential reuse in the future.

Remove explicit call to show the OSD.

3aff300

thornbill added the feature New feature or request label Jul 11, 2022

jellyfin-bot added the merge conflict Conflicts prevent merging label Sep 13, 2022

Merge branch 'master' into chapter-markers

33fe2c5

jellyfin-bot removed the merge conflict Conflicts prevent merging label Sep 17, 2022

thornbill requested changes Sep 30, 2022

View reviewed changes

Update getMarkerInfo syntax as suggested

9aea7e3

Change slider marker for chapters appearance.

b43c9e0

Now, the markers are displayed as ticks instead of icons above the slider

Keep chapter markers visible whenever the OSD is shown.

c4bd4d0

(Does not require hovering over the slider anymore.)

thornbill requested changes Oct 6, 2022

View reviewed changes

src/elements/emby-slider/emby-slider.js Outdated Show resolved Hide resolved

src/elements/emby-slider/emby-slider.js Outdated Show resolved Hide resolved

Rename some things as suggested.

596ee47

thornbill approved these changes Oct 6, 2022

View reviewed changes

thornbill merged commit 1ac326d into jellyfin:master Oct 6, 2022

Viperinius deleted the chapter-markers branch October 6, 2022 15:52

dmitrylyzo mentioned this pull request May 20, 2024

Fix chapter name XSS injection in progress bar #5563

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add chapter markings to video player slider #3745

Add chapter markings to video player slider #3745

Viperinius commented Jul 3, 2022 •

edited

Loading

dmitrylyzo Jul 4, 2022

Viperinius Jul 5, 2022 •

edited

Loading

sonarcloud bot commented Jul 5, 2022

thornbill left a comment

Viperinius commented Oct 1, 2022

thornbill commented Oct 1, 2022

Viperinius commented Oct 1, 2022

thornbill commented Oct 3, 2022

Viperinius commented Oct 5, 2022

thornbill left a comment

sonarcloud bot commented Oct 6, 2022

DoTheSneedful commented Mar 12, 2024

Add chapter markings to video player slider #3745

Add chapter markings to video player slider #3745

Conversation

Viperinius commented Jul 3, 2022 • edited Loading

dmitrylyzo Jul 4, 2022

Choose a reason for hiding this comment

Viperinius Jul 5, 2022 • edited Loading

Choose a reason for hiding this comment

sonarcloud bot commented Jul 5, 2022

thornbill left a comment

Choose a reason for hiding this comment

Viperinius commented Oct 1, 2022

thornbill commented Oct 1, 2022

Viperinius commented Oct 1, 2022

thornbill commented Oct 3, 2022

Viperinius commented Oct 5, 2022

thornbill left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Oct 6, 2022

DoTheSneedful commented Mar 12, 2024

Viperinius commented Jul 3, 2022 •

edited

Loading

Viperinius Jul 5, 2022 •

edited

Loading