Adding A-B Loop indicator on progress bar #548

braineo · 2017-04-02T01:58:12Z

System and IINA version:

macOS 10.12.4
IINA 0.0.7

Expected behavior:

Actual behavior:

It will be intuitive if we can highlight the loop section on progress bar and provide handle for adjusting loop duration

Easy to know the looping status.
No need to clear loop and start all over again.

Crash report:

MPV log:

Steps to reproduce:

MPV does not have this problem.

How often does this happen?

The text was updated successfully, but these errors were encountered:

braineo · 2017-04-17T05:29:54Z

Take a look at this. It seems we need a custom slider. NSSlider does not support this directly.

low-batt · 2021-10-30T17:22:45Z

Four years later and this excellent proposal from @braineo has still not been implemented. So I decided to take a stab at it. I will shortly submit a pull request for a proposed implementation for this feature. That is the good news.

The bad news is that I am not a graphical user interface programmer and have no talent for UI design, therefore my submission may be deemed as putrid by the core developers and unworthy of inclusion in IINA. Nevertheless I am blindly plunging head since no one else has had the time to implement this. Hopefully my submission will be considered adequate as an initial implementation with a more stylish UI slated as a future enhancement.

Needless to say a vicious brutal savage review is definitely called for. Everything is up for criticism. Have at it.

@braineo: Does the following line up with what you envisioned?:

User Experience

When the user clicks on the A-B Loop command in the Playback menu or presses the Shift(⇧)-Command(⌘)-L key combination to set the A point an additional thumb appears on the progress bar's slider in front of the primary thumb:

Hovering over the thumb displays a short help tag:

When the user invokes the A-B Loop command a second time another thumb appears on the progress bar's slider:

Moving either thumb changes the associated loop point in mpv. Unlike the progress thumb, moving either of the loop thumbs does not pause play. While a loop thumb is being moved the onscreen display shows the position within the video:

When the user invokes the A-B Loop command for a third time the loop is cleared and both thumbs will disappear.

The state of the A-B Loop feature is stored by mpv in the watch later config files. If a video is played again loop thumbs will be displayed if the respective loop point was set when the video was last played.

Dragging a loop thumb requires carefully clicking directly on the thumb. If the cursor is not over the thumb then the slider's bar will be clicked which moves the progress thumb. Requiring exact clicking on the loop thumb is intentional in order to avoid accidental changing of the loop points.

For the same reason when thumbs overlap the thumb that is selected by a click is based on the following order:

Progress thumb
B loop point
A loop point

The thumbs conform to the functionality provided by the mpv A-B Loop feature which is very forgiving and allows the B point to be set before the A point. From the mpv manual:

--ab-loop-a=<time>, --ab-loop-b=<time>

Set loop points. If playback passes the b timestamp, it will seek to the a timestamp. Seeking past the b point doesn't loop (this is intentional).

If a is after b, the behavior is as if the points were given in the right order, and the player will seek to b after crossing through a.

So the movement of the thumbs is unrestricted.

Implementation

The commit will:

Add a new class PlaySliderLoopKnob which adds a thumb to the slider to represent a loop point. This is a subclass of NSView to gain support for tool tips.
Add new class PlaySlider, a subclass of NSSlider. This class creates two PlaySliderLoopKnob objects in order to add two thumbs to the progress slider. This class also has a side effect of suppressing needless drawing when the slider is hidden which spindumps reveal is currently happening.
Change MainWindowController.xib to use the custom class PlaySlider for the progress slider.
Change PlayerCore to expose the A and B loop set points and allow them to be updated
Change MainWindowController to observe the location of the loop knobs on the play slider and update the new properties exposed by PlayerCore. This class also establishes tracking areas to know when the mouse is moving these knobs.
Change PlayerWindowController to synchronize the thumbs with the mpv loop points
Change MainMenuActions to call the window controller for the A-B Loop command instead of PlayerCore to keep the thumbs synchronized with the mpv state
Add a LoopStatus enum to PlaybackInfo to replace the use of integers for the value of abLoopStatus
Add a new message to OSDMessage for when the new thumbs are moved

Thoughts? Critical reviews including criticism of style and naming are welcome.

This commit adds two additional thumbs to the progress bar representing the A and B loop points when the A-B loop feature is active. This allows the user to adjust the loop points. This commit will: - Add a new class PlaySliderLoopKnob that adds an additional thumb - Add a new class PlaySlider that customizes NSSlider to add the thumbs - Change MainWindowController.xib to use PlaySlider for the OSC - Change PlayerCore to allow mpv loop points to be updated - Change MainWindowController to observe the thumbs and update mpv - Change PlayerWindowController to synchronize the thumbs with mpv - Change MainMenuActions to call the window controller for A-B Loop - Change PlaybackInfo to use an enum class for abLoopStatus - Add a new message to OSDMessage

braineo · 2021-11-01T05:55:13Z

@low-batt WOW! I would say it actually goes beyond! Thank you!

I hope this can pass the review soon and get merged!

This commit adds two additional thumbs to the progress bar representing the A and B loop points when the A-B loop feature is active. This allows the user to adjust the loop points. This commit will: - Add a new class PlaySliderLoopKnob that adds an additional thumb - Add a new class PlaySlider that customizes NSSlider to add the thumbs - Change MainWindowController.xib to use PlaySlider for the OSC - Change PlayerCore to allow mpv loop points to be updated - Change MainWindowController to observe the thumbs and update mpv - Change PlayerWindowController to synchronize the thumbs with mpv - Change MainMenuActions to call the window controller for A-B Loop - Change PlaybackInfo to use an enum class for abLoopStatus - Add a new message to OSDMessage

Ralph-y · 2021-11-17T20:28:31Z

Can I get this, like, now? I'm still looking for that elusive QT7Pro replacement in my workflow (I maintain a virtual machine for it but it's clunky to use) and this fits perfectly.

low-batt · 2021-11-17T22:23:25Z

I'm merely an IINA user, not a part of the IINA development team. I've been investigating issues and proposing solutions to help out the development team. The changes for this issue are awaiting their review. The latest status from the development team is that the new plug-in system is nearing completion. Hopefully once that is done and merged they will look into the outstanding pull requests.

As you can see above CarterLi has merged these changes into his IINA fork along with his changes to add HDR support for M1 Pro/Max. He has published a release you could try.

Ralph-y · 2021-11-17T22:31:42Z

I'll check that out. Thanks!

EDIT: looks like I need to compile it, which is above my pay grade. I'm a mere end user. Ah, well. Hopefully IINA incorporates this change someday. Thanks.

This commit adds two additional thumbs to the progress bar representing the A and B loop points when the A-B loop feature is active. This allows the user to adjust the loop points. This commit will: - Add a new class PlaySliderLoopKnob that adds an additional thumb - Add a new class PlaySlider that customizes NSSlider to add the thumbs - Change MainWindowController.xib to use PlaySlider for the OSC - Change PlayerCore to allow mpv loop points to be updated - Change MainWindowController to observe the thumbs and update mpv - Change PlayerWindowController to synchronize the thumbs with mpv - Change MainMenuActions to call the window controller for A-B Loop - Change PlaybackInfo to use an enum class for abLoopStatus - Add a new message to OSDMessage

Rebased with develop to resolve conflicts with plugin system. This commit adds two additional thumbs to the progress bar representing the A and B loop points when the A-B loop feature is active. This allows the user to adjust the loop points. This commit will: - Add a new class PlaySliderLoopKnob that adds an additional thumb - Add a new class PlaySlider that customizes NSSlider to add the thumbs - Change MainWindowController.xib to use PlaySlider for the OSC - Change PlayerCore to allow mpv loop points to be updated - Change MainWindowController to observe the thumbs and update mpv - Change PlayerWindowController to synchronize the thumbs with mpv - Change MainMenuActions to call the window controller for A-B Loop - Change PlaybackInfo to use an enum class for abLoopStatus - Add a new message to OSDMessage

Rebased with develop to resolve merge conflicts. Rebased with develop to resolve conflicts with plugin system. This commit adds two additional thumbs to the progress bar representing the A and B loop points when the A-B loop feature is active. This allows the user to adjust the loop points. This commit will: - Add a new class PlaySliderLoopKnob that adds an additional thumb - Add a new class PlaySlider that customizes NSSlider to add the thumbs - Change MainWindowController.xib to use PlaySlider for the OSC - Change PlayerCore to allow mpv loop points to be updated - Change MainWindowController to observe the thumbs and update mpv - Change PlayerWindowController to synchronize the thumbs with mpv - Change MainMenuActions to call the window controller for A-B Loop - Change PlaybackInfo to use an enum class for abLoopStatus - Add a new message to OSDMessage

Rebased with develop to resolve conflicts with Xcode project. Rebased with develop to resolve merge conflicts. Rebased with develop to resolve conflicts with plugin system. This commit adds two additional thumbs to the progress bar representing the A and B loop points when the A-B loop feature is active. This allows the user to adjust the loop points. This commit will: - Add a new class PlaySliderLoopKnob that adds an additional thumb - Add a new class PlaySlider that customizes NSSlider to add the thumbs - Change MainWindowController.xib to use PlaySlider for the OSC - Change PlayerCore to allow mpv loop points to be updated - Change MainWindowController to observe the thumbs and update mpv - Change PlayerWindowController to synchronize the thumbs with mpv - Change MainMenuActions to call the window controller for A-B Loop - Change PlaybackInfo to use an enum class for abLoopStatus - Add a new message to OSDMessage

Corrected merge error in Xcode project. Rebased with develop to resolve conflicts with Xcode project. Rebased with develop to resolve merge conflicts. Rebased with develop to resolve conflicts with plugin system. This commit adds two additional thumbs to the progress bar representing the A and B loop points when the A-B loop feature is active. This allows the user to adjust the loop points. This commit will: - Add a new class PlaySliderLoopKnob that adds an additional thumb - Add a new class PlaySlider that customizes NSSlider to add the thumbs - Change MainWindowController.xib to use PlaySlider for the OSC - Change PlayerCore to allow mpv loop points to be updated - Change MainWindowController to observe the thumbs and update mpv - Change PlayerWindowController to synchronize the thumbs with mpv - Change MainMenuActions to call the window controller for A-B Loop - Change PlaybackInfo to use an enum class for abLoopStatus - Add a new message to OSDMessage

Ralph-y · 2023-02-26T19:02:16Z

Is this dead? All IINA lacks is a-b loop handles to be the better-than-perfect QTP7 replacement.

low-batt · 2023-02-26T22:23:39Z

The last 3 commits you see above are me updating the proposed changes so that they will work with the latest changes that were added to IINA, keeping these changes ready to merge. The next step is for other developers to review, approve and merge these changes.

The holdup is that this is a new feature and fixing defects has been the priority. I'm expecting that fixing defects will still be a priority for the next release. I will be pushing the team to include this feature in that release. I can't say if it will make it in or be delayed until the next feature release, that is up to the other developers.

But definitely not dead!

Rebased with develop to resolve conflicts with Xcode project. Rebased with develop to resolve merge conflicts. Rebased with develop to resolve conflicts with plugin system. This commit adds two additional thumbs to the progress bar representing the A and B loop points when the A-B loop feature is active. This allows the user to adjust the loop points. This commit will: - Add a new class PlaySliderLoopKnob that adds an additional thumb - Add a new class PlaySlider that customizes NSSlider to add the thumbs - Change MainWindowController.xib to use PlaySlider for the OSC - Change PlayerCore to allow mpv loop points to be updated - Change MainWindowController to observe the thumbs and update mpv - Change PlayerWindowController to synchronize the thumbs with mpv - Change MainMenuActions to call the window controller for A-B Loop - Change PlaybackInfo to use an enum class for abLoopStatus - Add a new message to OSDMessage

This commit adds two additional thumbs to the progress bar representing the A and B loop points when the A-B loop feature is active. This allows the user to adjust the loop points. This commit will: - Add a new class PlaySliderLoopKnob that adds an additional thumb - Add a new class PlaySlider that customizes NSSlider to add the thumbs - Change MainWindowController.xib to use PlaySlider for the OSC - Change PlayerCore to allow mpv loop points to be updated - Change MainWindowController to observe the thumbs and update mpv - Change PlayerWindowController to synchronize the thumbs with mpv - Change MainMenuActions to call the window controller for A-B Loop - Change PlaybackInfo to use an enum class for abLoopStatus - Add a new message to OSDMessage Rebased with develop to resolve conflicts with Xcode project. Rebased with develop to resolve merge conflicts. Rebased with develop to resolve conflicts with plugin system. Corrected typo found in review.

low-batt · 2023-03-05T02:28:00Z

YES. This was one of my early PRs. I tried hard to not change the existing code. I remember it seemed to me positioning of the main slider knob seemed off. It was the knob going past the B knob that I noticed. And yes, that is easier to see in the floating OSC. I missed the other issues you noted. I was thinking any existing issues with the OSC should be handled in a separate PR.

With this PR I wanted to establish base code that was working. I was expecting an actual UI programer could then tune it up. Initially I was trying to use arrows for the A and B knobs. They looked awful due to the small size of the progress bar. I ended up with what you see. I did not try and implement some of the UI details, like the shadow the main knob has.

I've been a little slow to respond as I have been distracted by the need to shovel snow after a winter storm. I did notice the PR updates. I will try and test them out soon.

svobs · 2023-03-05T08:49:39Z

I ended up with what you see. I did not try and implement some of the UI details, like the shadow the main knob has.

I think they look good in Dark Mode. In Light Mode I think they also look fine -especially when compared to nothing at all. Merging this does not preclude further style changes some point in the future so I don't think it's necessary to get snagged on that.

I don't see a shadow on the main knob...(?)

The more I think about it though, the more I think the issue of the main knob's X offset should be addressed. I did do a medium dive into the issue, and it's not trivial. It might be Apple which broke it. When I reverted to the default box-shaped knob, I could see why it is doing what it is: it's adjusting the slider position toward the ends so that the edge of the "box" touches the end of the slider "track" rather than the center of the box. I'll file an issue for this soon.

I've been a little slow to respond as I have been distracted by the need to shovel snow after a winter storm.

I got a sense you might be located in the Midwest, heh. That is a joy I have yet to appreciate. Though the storms here were shockingly violent at times, frequent brief power outages aren't too bothersome when you've got a full laptop battery and decent cell service.

low-batt · 2023-03-05T13:29:46Z

I see I messed up and replied to the issue when I intended to reply to the PR. The code for drawing a shadow is here in PlaySliderCell.

I grew up in the Midwest. I'm currently in New England. Not frigid like the Midwest, but cold enough for a snowy Winter.

This commit adds two additional thumbs to the progress bar representing the A and B loop points when the A-B loop feature is active. This allows the user to adjust the loop points. This commit will: - Add a new class PlaySliderLoopKnob that adds an additional thumb - Add a new class PlaySlider that customizes NSSlider to add the thumbs - Change MainWindowController.xib to use PlaySlider for the OSC - Change PlayerCore to allow mpv loop points to be updated - Change MainWindowController to observe the thumbs and update mpv - Change PlayerWindowController to synchronize the thumbs with mpv - Change MainMenuActions to call the window controller for A-B Loop - Change PlaybackInfo to use an enum class for abLoopStatus - Add a new message to OSDMessage Rebased with develop to resolve conflicts with Xcode project. Rebased with develop to resolve merge conflicts. Rebased with develop to resolve conflicts with plugin system. Rebased with develop and corrected merge conflicts.

This commit adds two additional thumbs to the progress bar representing the A and B loop points when the A-B loop feature is active. This allows the user to adjust the loop points. This commit will: - Add a new class PlaySliderLoopKnob that adds an additional thumb - Add a new class PlaySlider that customizes NSSlider to add the thumbs - Change MainWindowController.xib to use PlaySlider for the OSC - Change PlayerCore to allow mpv loop points to be updated - Change MainWindowController to observe the thumbs and update mpv - Change PlayerWindowController to synchronize the thumbs with mpv - Change MainMenuActions to call the window controller for A-B Loop - Change PlaybackInfo to use an enum class for abLoopStatus - Add a new message to OSDMessage Corrected merge error in Xcode project. Rebased with develop to resolve conflicts with Xcode project. Rebased with develop to resolve merge conflicts. Rebased with develop to resolve conflicts with plugin system. Rebased with develop and corrected merge conflicts.

This commit adds two additional thumbs to the progress bar representing the A and B loop points when the A-B loop feature is active. This allows the user to adjust the loop points. This commit will: - Add a new class PlaySliderLoopKnob that adds an additional thumb - Add a new class PlaySlider that customizes NSSlider to add the thumbs - Change MainWindowController.xib to use PlaySlider for the OSC - Change PlayerCore to allow mpv loop points to be updated - Change MainWindowController to observe the thumbs and update mpv - Change PlayerWindowController to synchronize the thumbs with mpv - Change MainMenuActions to call the window controller for A-B Loop - Change PlaybackInfo to use an enum class for abLoopStatus - Add a new message to OSDMessage Rebased with develop to resolve conflicts with Xcode project. Rebased with develop to resolve merge conflicts. Rebased with develop to resolve conflicts with plugin system. Corrected typo found in review.

This commit adds two additional thumbs to the progress bar representing the A and B loop points when the A-B loop feature is active. This allows the user to adjust the loop points. This commit will: - Add a new class PlaySliderLoopKnob that adds an additional thumb - Add a new class PlaySlider that customizes NSSlider to add the thumbs - Change MainWindowController.xib to use PlaySlider for the OSC - Change PlayerCore to allow mpv loop points to be updated - Change MainWindowController to observe the thumbs and update mpv - Change PlayerWindowController to synchronize the thumbs with mpv - Change MainMenuActions to call the window controller for A-B Loop - Change PlaybackInfo to use an enum class for abLoopStatus - Add a new message to OSDMessage Rebased with develop to resolve conflicts with Xcode project. Rebased with develop to resolve merge conflicts. Rebased with develop to resolve conflicts with plugin system. Rebased with develop and corrected merge conflicts.

This commit adds two additional thumbs to the progress bar representing the A and B loop points when the A-B loop feature is active. This allows the user to adjust the loop points. This commit will: - Add a new class PlaySliderLoopKnob that adds an additional thumb - Add a new class PlaySlider that customizes NSSlider to add the thumbs - Change MainWindowController.xib to use PlaySlider for the OSC - Change PlayerCore to allow mpv loop points to be updated - Change MainWindowController to observe the thumbs and update mpv - Change PlayerWindowController to synchronize the thumbs with mpv - Change MainMenuActions to call the window controller for A-B Loop - Change PlaybackInfo to use an enum class for abLoopStatus - Add a new message to OSDMessage Corrected merge error in Xcode project. Rebased with develop to resolve conflicts with Xcode project. Rebased with develop to resolve merge conflicts. Rebased with develop to resolve conflicts with plugin system. Rebased with develop and corrected merge conflicts. Rebased with develop and resolved Xcode project merge conflicts.

This commit adds two additional thumbs to the progress bar representing the A and B loop points when the A-B loop feature is active. This allows the user to adjust the loop points. This commit will: - Add a new class PlaySliderLoopKnob that adds an additional thumb - Add a new class PlaySlider that customizes NSSlider to add the thumbs - Change MainWindowController.xib to use PlaySlider for the OSC - Change PlayerCore to allow mpv loop points to be updated - Change MainWindowController to observe the thumbs and update mpv - Change PlayerWindowController to synchronize the thumbs with mpv - Change MainMenuActions to call the window controller for A-B Loop - Change PlaybackInfo to use an enum class for abLoopStatus - Add a new message to OSDMessage Rebased with develop to resolve conflicts with Xcode project. Rebased with develop to resolve merge conflicts. Rebased with develop to resolve conflicts with plugin system. Corrected typo found in review.

This commit adds two additional thumbs to the progress bar representing the A and B loop points when the A-B loop feature is active. This allows the user to adjust the loop points. This commit will: - Add a new class PlaySliderLoopKnob that adds an additional thumb - Add a new class PlaySlider that customizes NSSlider to add the thumbs - Change MainWindowController.xib to use PlaySlider for the OSC - Change PlayerCore to allow mpv loop points to be updated - Change MainWindowController to observe the thumbs and update mpv - Change PlayerWindowController to synchronize the thumbs with mpv - Change MainMenuActions to call the window controller for A-B Loop - Change PlaybackInfo to use an enum class for abLoopStatus - Add a new message to OSDMessage - Move changing of the slider control size from PlaySliderCell to PlaySlider - Change PlaySliderCell.knobRect to keep thumb within bar - Remove PlaySliderCell.barRect that increased the width of the bar

lhc70000 added the type: feature request label Apr 2, 2017

uiryuu added the feature: ui/ux label May 29, 2017

low-batt mentioned this issue Oct 30, 2021

Close add A-B Loop indicator on progress bar, #548 #3529

Merged

1 task

low-batt mentioned this issue Nov 3, 2021

High CPU usage on M1 MBA #3343

Open

low-batt mentioned this issue Aug 18, 2022

Top of progress bar not behaving as such #3911

Closed

low-batt self-assigned this Nov 7, 2022

low-batt linked a pull request Nov 7, 2022 that will close this issue

Close add A-B Loop indicator on progress bar, #548 #3529

Merged

1 task

low-batt added the progress: pr ready label Nov 7, 2022

uiryuu closed this as completed in #3529 Apr 11, 2023

low-batt added progress: fixed/implemented and removed progress: pr ready labels May 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding A-B Loop indicator on progress bar #548

Adding A-B Loop indicator on progress bar #548

braineo commented Apr 2, 2017

braineo commented Apr 17, 2017

low-batt commented Oct 30, 2021

braineo commented Nov 1, 2021

Ralph-y commented Nov 17, 2021

low-batt commented Nov 17, 2021

Ralph-y commented Nov 17, 2021 •

edited

Ralph-y commented Feb 26, 2023

low-batt commented Feb 26, 2023

low-batt commented Mar 5, 2023

svobs commented Mar 5, 2023

low-batt commented Mar 5, 2023

Adding A-B Loop indicator on progress bar #548

Adding A-B Loop indicator on progress bar #548

Comments

braineo commented Apr 2, 2017

braineo commented Apr 17, 2017

low-batt commented Oct 30, 2021

User Experience

Implementation

braineo commented Nov 1, 2021

Ralph-y commented Nov 17, 2021

low-batt commented Nov 17, 2021

Ralph-y commented Nov 17, 2021 • edited

Ralph-y commented Feb 26, 2023

low-batt commented Feb 26, 2023

low-batt commented Mar 5, 2023

svobs commented Mar 5, 2023

low-batt commented Mar 5, 2023

Ralph-y commented Nov 17, 2021 •

edited