Add stream segments to player controls

[vkay94]

What is it?

• Bugfix (user facing)
• Feature (user facing)
• Codebase improvement (dev facing)
• Meta improvement to the project (dev facing)

Description of the changes in your PR

This PR adds stream segments for the YouTube service to player controls. The feature provides the following behaviors:

• Highlights the current segment depending on the current position of the video (see Preview) / by click
• Scrolls automatically to the current playing segment when the list is opened
• Segments list is visible and the video is changed: if the new video has also segments then the new segments are loaded and it scrolls to the correct position, otherwise the list closes.

Code changes:

• Adds a new button near to the top controls bar
• Uses same layout elements like the play queue => they've been renamed to more generic id/field names, for example queueLayout to itemsListLayout

Fixes the following issue(s)

• closes [Feature] Segmented videos #5098

APK testing

Check the build artifact page for the debug apk.

Preview

Demo (dynamically changing at 0:22) : https://streamable.com/e/0v66wn

Due diligence

• I read the contribution guidelines.

[Orion983]

eyy i opened the issue for this

[Orion983]

Fun fact: these segments are named "YouTube Chapters"

[TobiGr]

Thanks for implementing that!

As you are using the layout used by the queue, please make sure to handlde swipe events differently. E.g. it is currently possible to remove segments from the list by swiping them to the right.

[Orion983]

Is this being added in the next version?

[vkay94]

As you are using the layout used by the queue, please make sure to handlde swipe events differently. E.g. it is currently possible to remove segments from the list by swiping them to the right.

You're right, it starts to be possible whenever the queue was opened before. I'll address it tomorrow ;)
The overall layout (thumbnail size etc) is fine so far?

[Orion983]

The apk is corrupted it says if I click "app artifact" and If i extract the zip it says it's corrupted

[Orion983]

Hmm it let's ME review this. I won't do that unless you guys are ok with it

[vkay94]

@HTSamurai Hmm, I tried it and the zip seems to be fine (I just copied the URL to the file which is built automatically => checks tab). Probably trying to download it from there?

[Orion983]

I'll try that

[Orion983]

Yeah now it's fixed

[Orion983]

Am I allowed to do a review? It let's me, but I won't do it without consent

[TobiGr]

@HTSamurai sure

@vkay94 Layout looks good to me! One small suggestion: Add a title to the view, so users quickly understand what it is about

Screenshot

[Orion983]

Looks great to me!

[vkay94]

@TobiGr Done. I used TextAppearance.AppCompat.Medium as style, it's a little bit smaller than in your mock-up. I think it fits well.
@HTSamurai I just noticed that you've reviewed the code before the push (unfortunately I can't see them now, did you find something that should be changed?)

[Redirion]

can you rebase your PR please? the upcoming release lacks of noteworthy changes/features. This PR could be it

[vkay94]

@Redirion Sure, I planed to do it today anyway

[Orion983]

@TobiGr Done. I used TextAppearance.AppCompat.Medium as style, it's a little bit smaller than in your mock-up. I think it fits well.
@HTSamurai I just noticed that you've reviewed the code before the push (unfortunately I can't see them now, did you find something that should be changed?)

No I didn't find anything that should be changed. All was well

[Stypox]

Happy rebasing! ;-)
(sorry, don't kill me for making #5371, please)

[vkay94]

@Stypox No problem, at least it wasn't that much (but I had to do it twice because I clicked on "skip commit" during rebasing accidentally :') )

[Redirion]

startTimeSeconds.toLong()
fun secondsToString(seconds: Long): String
startTimeSeconds * 1000L
getStartTimeSeconds() * 1000
int getNearestStreamSegmentPosition(final long playbackPosition)

I think these can safely reduced to int. We don't need the ms precision that is returned by simpleExoPlayer.getCurrentPosition() and we use int everywhere else.

[vkay94]

@Redirion Good point, I didn't pay attention to it in the other places, I'll adjust it then.
Edit: I changed the passed startTimeSeconds to int. For the other places the milliseconds value is required for seeking and comparison (I pass 2 of 3 times 'simpleExoPlayer.getCurrentPosition()' to getNearestStreamSegmentPosition which is Long, so it should be fine).

[Stypox]

I like the changes, just a few small issues. Thank you! :-D
Run Ctrl+Shift+Alt+L in the player.xml files after you are done, to reorder attributes

[Stypox]

Looks good to me, thank you for this great feature!
Maybe let's wait for somebody else to review, though, this is a pretty big change ;-)
Edit: I didn't test 😨

[Redirion]

Tested, looking good

[opusforlife2]

the upcoming release lacks of noteworthy changes/features

@Redirion "Am I dead to you!?" ~ PeerTube Sepia Search.

[opusforlife2]

@vkay94 When did you last open Matrix? Please check your messages.