Skip to content

Add a zapping mode. - #2045

Open
Vinzzzze wants to merge 1 commit into
strawberrymusicplayer:masterfrom
Vinzzzze:play_limited_time
Open

Add a zapping mode.#2045
Vinzzzze wants to merge 1 commit into
strawberrymusicplayer:masterfrom
Vinzzzze:play_limited_time

Conversation

@Vinzzzze

@Vinzzzze Vinzzzze commented Mar 21, 2026

Copy link
Copy Markdown
Contributor

A zapping mode consists in selecting a time in the track and play the specified amount of seconds before and after this selected time.
If the amount of seconds is big enough to start before the beginning of the track, then the amount of seconds before the selected time is reduced to start at the beginning of the track.
Same thing for the end of the track : the amount of seconds after the selected time will be reduced to end at the end of the track.
This function is associated to the playlist itself and is hidden by default ; a button had been added at the end of the search bar in the playlist tab. This button toggle the display of these two values : one value to select the amount of seconds (0 to disable the zapping, it's the default value) before and after the time in percent of the track length.
If you choose 10 seconds around 90% of the track, on a 5mn long track, the play will start at 4mn20s and will end at 4mn40s. On a 1mn long track, the play will start at 44s and will end at the end of the track.

Technical part :

  • playlist had been updated to own the two zapping values (playlist, playlistmanager & smartplaylistsearchpreview).
  • playlistcontainer had been updated to add the button used to toggle the display of the values and the two values had been added with the initialization from the playlist object (translations had been added for the tooltip and button text).
  • the db scheme had been updated to store the zapping values with the playlist (data.qrc, schema-22.sql, database.cpp, playlistbackend)
  • The engine had been updated (mainwindow, player)

Summary by CodeRabbit

  • New Features
    • Added per-playlist playback-time/interest settings with new UI controls, tooltips, and a toggle to show/hide the options.
    • Persisted the new settings and use them to determine the active track/offset during playback.
    • Added end-of-track advancement behavior driven by the configured position threshold.
  • Chores
    • Updated the database schema to version 24 and included the new schema resource.

@Vinzzzze
Vinzzzze force-pushed the play_limited_time branch 2 times, most recently from db0ee73 to 09acb3d Compare March 29, 2026 16:57
@Vinzzzze
Vinzzzze force-pushed the play_limited_time branch 4 times, most recently from e50fed7 to 132a4dc Compare April 10, 2026 19:29
@Vinzzzze
Vinzzzze force-pushed the play_limited_time branch 7 times, most recently from d8d5f18 to f3e7762 Compare April 24, 2026 20:38
@Vinzzzze
Vinzzzze force-pushed the play_limited_time branch from f3e7762 to 16eac67 Compare April 26, 2026 09:38
@Vinzzzze
Vinzzzze force-pushed the play_limited_time branch 3 times, most recently from 93a11a7 to 7c4414f Compare May 10, 2026 21:41
@Vinzzzze
Vinzzzze force-pushed the play_limited_time branch from 7c4414f to b1d6093 Compare May 17, 2026 14:14
@Vinzzzze
Vinzzzze force-pushed the play_limited_time branch 4 times, most recently from fed1593 to f688ec1 Compare May 31, 2026 14:46
@Vinzzzze
Vinzzzze force-pushed the play_limited_time branch 2 times, most recently from 11bdb37 to fd6607a Compare June 5, 2026 19:19
@Vinzzzze
Vinzzzze force-pushed the play_limited_time branch 3 times, most recently from 1a292c3 to bf90b11 Compare June 15, 2026 07:17
@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds two per-playlist playback window fields (half_playing_time_s and percent_interest_song/position_playing_time) threaded from a new database schema v24 migration through PlaylistBackend, Playlist, PlaylistManager, and Player. The player computes an intra-track window for playback offset and auto-advances to the next track at the computed end position. New UI spin boxes in PlaylistContainer expose the settings.

Changes

Playlist Playback Window Feature

Layer / File(s) Summary
Database schema v24 migration
data/schema/schema-24.sql, data/schema/schema.sql, src/core/database.cpp, data/data.qrc
Incremental migration script adds half_playing_time_s and position_playing_time columns to playlists with NOT NULL defaults; canonical schema and kSchemaVersion constant bumped to 24; migration resource registered.
PlaylistBackend: read and write new fields
src/playlist/playlistbackend.h, src/playlist/playlistbackend.cpp
PlaylistBackend::Playlist struct gains two integer fields; GetPlaylists/GetPlaylist SELECT and populate them; SavePlaylistAsync and SavePlaylist accept and bind two new integer parameters in the UPDATE statement.
Playlist: playback window computation and persistence
src/playlist/playlist.h, src/playlist/playlist.cpp
Constructor gains half_playing_time_s and percent_interest_song parameters with defaults; new current_item(start_offset_ns, end_offset_s) overload computes an intra-track window from stored settings and track length; Save() forwards values to backend; accessors and update methods added.
PlaylistManager: new-field propagation and accessors
src/playlist/playlistmanager.h, src/playlist/playlistmanager.cpp, src/smartplaylists/smartplaylistsearchpreview.cpp
AddPlaylist extended with the two new parameters; init/open paths forward backend values; new/load paths supply defaults (0, 50); public inline accessors and update methods delegate to the current Playlist; smart playlist preview updated.
Player: offset-aware playback start and end-position auto-advance
src/core/player.h, src/core/player.cpp, src/core/mainwindow.cpp
Player stores play_end_sec_; PlayAt calls the new current_item overload to obtain offset and end bounds; EndPositionNext(position) calls Next() when play_end_sec_ threshold is reached; MainWindow::UpdateTrackPosition invokes EndPositionNext on each position tick while playing.
PlaylistContainer UI controls for playback window settings
src/playlist/playlistcontainer.ui, src/playlist/playlistcontainer.h, src/playlist/playlistcontainer.cpp
Adds a checkable display_option toolbar button, two initially hidden spin boxes, and a show_zapping_values action to the UI; wires tooltips, icon sizes, valueChanged signals to PlaylistManager update slots, populates spin box values in SetViewModel with blocked signals, and DisplayPlayingOption toggles their visibility.

Sequence Diagrams

sequenceDiagram
  participant MW as MainWindow::UpdateTrackPosition
  participant Player as Player
  participant Playlist as Playlist::current_item
  participant Engine as Engine

  Note over MW,Player: On each position tick while Playing
  MW->>Player: EndPositionNext(position)
  Player->>Player: if play_end_sec_ > 0 && position <= play_end_sec_
  Player->>Player: Next()
  Player->>Playlist: current_item(play_offset_nanosec_, play_end_sec_)
  Playlist->>Playlist: compute center, start, end from half_playing_time_s_ / percent_interest_song_
  Playlist-->>Player: PlaylistItemPtr + updated start_offset_ns, end_offset_s
  Player->>Engine: Play(play_offset_nanosec_)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • jonaski

Poem

🐇 Hippity-hop through the song's middle beat,
A window of music, a half-time treat!
percent_interest_song marks the sweet spot,
The schema ticks up, version twenty-four's got
New columns to store where the good parts lie —
Then EndPositionNext hops the rabbit on by! 🎵

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add a zapping mode' clearly and concisely describes the main feature being introduced across all the changes in this PR.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/playlist/playlistbackend.h (1)

54-67: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Initialize the new playback-window fields in the default constructor.

At Line 54, Playlist() does not initialize half_playing_time_s_ and percent_interest_song_ (Lines 66-67). Default-return paths can propagate undefined values.

Suggested fix
-    Playlist() : id(-1), favorite(false), last_played(0) {}
+    Playlist() : id(-1), favorite(false), last_played(0), half_playing_time_s_(0), percent_interest_song_(50) {}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/playlist/playlistbackend.h` around lines 54 - 67, The Playlist() default
constructor on line 54 initializes some member variables but does not initialize
the newly added member variables half_playing_time_s_ and percent_interest_song_
declared on lines 66-67, which can result in undefined values. Add
initialization for both half_playing_time_s_ and percent_interest_song_ in the
Playlist() constructor's initialization list, assigning them appropriate default
values (such as 0 for integer fields).
🧹 Nitpick comments (1)
src/playlist/playlist.h (1)

238-242: 💤 Low value

Consider using ScheduleSave() instead of immediate Save() for consistency.

The update methods call Save() directly, bypassing the debounced ScheduleSave() pattern used elsewhere in this class (e.g., line 182). If the user rapidly adjusts the spinbox values, this triggers multiple immediate database writes. Consider using ScheduleSave() for consistency with the existing save pattern.

♻️ Suggested change
-  void UpdatePlayingTime(const int time_s) { half_playing_time_s_ = time_s; Save(); }
+  void UpdatePlayingTime(const int time_s) { half_playing_time_s_ = time_s; ScheduleSaveAsync(); }

-  void UpdatePlayingPosition(const int percent_time) { percent_interest_song_ = percent_time; Save(); }
+  void UpdatePlayingPosition(const int percent_time) { percent_interest_song_ = percent_time; ScheduleSaveAsync(); }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/playlist/playlist.h` around lines 238 - 242, The UpdatePlayingTime() and
UpdatePlayingPosition() methods currently call Save() directly, which causes
immediate database writes when values change rapidly. Replace the Save() calls
with ScheduleSave() calls in both methods to use the debounced save pattern that
is already used elsewhere in the class, ensuring consistency and preventing
multiple rapid database writes when users adjust spinbox values quickly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@data/schema/schema-23.sql`:
- Around line 1-5: The new columns half_playing_time_s and position_playing_time
are being added as nullable without defaults, which will leave migrated rows
with NULL values and break expected behavior when the feature logic reads these
values expecting the baseline (0, 50) values. Modify the ALTER TABLE statements
to set appropriate DEFAULT values on each column (with half_playing_time_s
defaulting to one baseline value and position_playing_time to the other), and
add a backfill UPDATE statement to set the default values for all existing rows
in the playlists table before updating the schema_version.

In `@src/core/mainwindow.cpp`:
- Around line 1826-1827: The EndPositionNext(position) method is being called
unconditionally from a timer that runs regardless of playback state, causing the
player to skip to the next track even while paused. Add a guard condition to
check if the player is currently in a playing state before calling
EndPositionNext(position). This ensures track advancement only occurs during
active playback, not when the player is paused.

In `@src/playlist/playlistcontainer.cpp`:
- Around line 485-490: The DisplayPlayingOption() method only toggles the
visibility of UI controls but does not disable the zapping mode when hiding
them. When the controls are hidden (visibility becomes false), also reset the
zapping state by clearing or resetting the playing_time_before_or_after value to
ensure zapping is actually disabled and not just hidden from view. This ensures
the toggle accurately reflects the actual state of the zapping feature.

In `@src/playlist/playlistcontainer.ui`:
- Around line 231-234: The user-facing text for the action
show_collection_option in the playlistcontainer.ui file (at the property
name="text" string element) currently says "Show the collection option" which
does not accurately describe what this playback-window control actually does.
Update the string element to use feature-accurate user text that accurately
describes the functionality of this playback control, ensuring the text clearly
conveys the actual purpose and behavior of toggling or managing the collection
option during playback.

---

Outside diff comments:
In `@src/playlist/playlistbackend.h`:
- Around line 54-67: The Playlist() default constructor on line 54 initializes
some member variables but does not initialize the newly added member variables
half_playing_time_s_ and percent_interest_song_ declared on lines 66-67, which
can result in undefined values. Add initialization for both half_playing_time_s_
and percent_interest_song_ in the Playlist() constructor's initialization list,
assigning them appropriate default values (such as 0 for integer fields).

---

Nitpick comments:
In `@src/playlist/playlist.h`:
- Around line 238-242: The UpdatePlayingTime() and UpdatePlayingPosition()
methods currently call Save() directly, which causes immediate database writes
when values change rapidly. Replace the Save() calls with ScheduleSave() calls
in both methods to use the debounced save pattern that is already used elsewhere
in the class, ensuring consistency and preventing multiple rapid database writes
when users adjust spinbox values quickly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 016f9dc2-5a2b-48ca-9384-0a5b7a18f0d0

📥 Commits

Reviewing files that changed from the base of the PR and between de3ad29 and bf90b11.

📒 Files selected for processing (17)
  • data/data.qrc
  • data/schema/schema-23.sql
  • data/schema/schema.sql
  • src/core/database.cpp
  • src/core/mainwindow.cpp
  • src/core/player.cpp
  • src/core/player.h
  • src/playlist/playlist.cpp
  • src/playlist/playlist.h
  • src/playlist/playlistbackend.cpp
  • src/playlist/playlistbackend.h
  • src/playlist/playlistcontainer.cpp
  • src/playlist/playlistcontainer.h
  • src/playlist/playlistcontainer.ui
  • src/playlist/playlistmanager.cpp
  • src/playlist/playlistmanager.h
  • src/smartplaylists/smartplaylistsearchpreview.cpp

Comment thread data/schema/schema-23.sql Outdated
Comment thread src/core/mainwindow.cpp Outdated
Comment thread src/playlist/playlistcontainer.cpp
Comment thread src/playlist/playlistcontainer.ui Outdated
@Vinzzzze
Vinzzzze force-pushed the play_limited_time branch from bf90b11 to fcc09a9 Compare June 15, 2026 09:26

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/playlist/playlist.h (1)

239-242: ⚡ Quick win

Use debounced persistence for spinbox-driven updates.

Line 239 and Line 242 call Save() directly, which can trigger a DB write per incremental UI value change. Switching to ScheduleSave() keeps behavior but coalesces writes.

[recommendation]

Suggested patch
-  void UpdatePlayingTime(const int time_s) { half_playing_time_s_ = time_s; Save(); }
+  void UpdatePlayingTime(const int time_s) { half_playing_time_s_ = time_s; ScheduleSave(); }
...
-  void UpdatePlayingPosition(const int percent_time) { percent_interest_song_ = percent_time; Save(); }
+  void UpdatePlayingPosition(const int percent_time) { percent_interest_song_ = percent_time; ScheduleSave(); }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/playlist/playlist.h` around lines 239 - 242, The UpdatePlayingTime and
UpdatePlayingPosition methods in the playlist class are calling Save() directly
in response to spinbox UI changes, which can cause frequent database writes for
each incremental value change. Replace the direct Save() calls with
ScheduleSave() in both methods to enable debounced persistence that coalesces
multiple rapid updates into fewer database writes while maintaining the same
overall behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/playlist/playlist.cpp`:
- Around line 1968-1989: In the current_item method, initialize start_offset_ns
to a default value (0) immediately after resetting end_offset_s, before the
conditional logic that checks if start_time_ns should overwrite it. Currently,
start_offset_ns is only assigned when a specific condition is met at line 1981,
which allows previous values to persist across function calls and leak into
subsequent playback operations. Add an initialization line right after
end_offset_s = 0 to ensure the parameter is always reset on every invocation.

---

Nitpick comments:
In `@src/playlist/playlist.h`:
- Around line 239-242: The UpdatePlayingTime and UpdatePlayingPosition methods
in the playlist class are calling Save() directly in response to spinbox UI
changes, which can cause frequent database writes for each incremental value
change. Replace the direct Save() calls with ScheduleSave() in both methods to
enable debounced persistence that coalesces multiple rapid updates into fewer
database writes while maintaining the same overall behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8cc0340f-6252-40fd-bb8a-48df81590e58

📥 Commits

Reviewing files that changed from the base of the PR and between bf90b11 and fcc09a9.

📒 Files selected for processing (17)
  • data/data.qrc
  • data/schema/schema-23.sql
  • data/schema/schema.sql
  • src/core/database.cpp
  • src/core/mainwindow.cpp
  • src/core/player.cpp
  • src/core/player.h
  • src/playlist/playlist.cpp
  • src/playlist/playlist.h
  • src/playlist/playlistbackend.cpp
  • src/playlist/playlistbackend.h
  • src/playlist/playlistcontainer.cpp
  • src/playlist/playlistcontainer.h
  • src/playlist/playlistcontainer.ui
  • src/playlist/playlistmanager.cpp
  • src/playlist/playlistmanager.h
  • src/smartplaylists/smartplaylistsearchpreview.cpp
✅ Files skipped from review due to trivial changes (1)
  • data/data.qrc
🚧 Files skipped from review as they are similar to previous changes (13)
  • data/schema/schema-23.sql
  • src/core/mainwindow.cpp
  • data/schema/schema.sql
  • src/core/player.h
  • src/playlist/playlistbackend.h
  • src/playlist/playlistmanager.cpp
  • src/playlist/playlistcontainer.h
  • src/smartplaylists/smartplaylistsearchpreview.cpp
  • src/playlist/playlistmanager.h
  • src/playlist/playlistcontainer.cpp
  • src/playlist/playlistbackend.cpp
  • src/core/player.cpp
  • src/core/database.cpp

Comment thread src/playlist/playlist.cpp
@Vinzzzze
Vinzzzze force-pushed the play_limited_time branch 3 times, most recently from a2798aa to 80ca07a Compare June 21, 2026 16:51
@Vinzzzze
Vinzzzze force-pushed the play_limited_time branch 5 times, most recently from 5599f00 to 96d6001 Compare June 29, 2026 21:13
@Vinzzzze
Vinzzzze force-pushed the play_limited_time branch 2 times, most recently from 9eda5c6 to 2f7281c Compare July 5, 2026 15:44
@Vinzzzze
Vinzzzze force-pushed the play_limited_time branch 3 times, most recently from f2bfaca to bc1582d Compare July 15, 2026 18:32
@Vinzzzze
Vinzzzze force-pushed the play_limited_time branch 4 times, most recently from a20fb59 to 33c39b9 Compare July 26, 2026 15:31
@Vinzzzze
Vinzzzze force-pushed the play_limited_time branch 2 times, most recently from b447e42 to 9dbc73b Compare July 30, 2026 20:00
@Vinzzzze
Vinzzzze force-pushed the play_limited_time branch from 9dbc73b to e12efde Compare August 2, 2026 12:12
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.

1 participant