Unless you had manually resumed the playlist, you
didn't get any warning after Rockbox rebooted, when
replacing a modified current playlist.
Change-Id: Ibc83e3db4e6ee51e94204ac7732760894d2fb602
- After rebooting, unless the current playlist
had been manually resumed already, the "Save
Current Playlist" option in the root menu's/
playlist catalogue's context menu failed to work.
- Eliminate error message attempting to view current
playlist when no control file present.
Change-Id: I58f1d3dc6f256fcee106ff5a17f69fa46d67b2e4
Reset fd for the on-disk playlist at the place where
ownership is transferred in playlist_set_current (which
already has a note, that the playlist will be effectively
closed, but did not reset the fd).
Also ensure, that calling playlist_load closes the fd for
the on-disk playlist (and its control file), if any were
left open - even though this is not supposed to happen.
Generate name of control file in playlist_init to be more
consistent with the behavior for the current playlist.
Only a single playlist can be loaded in the playlist viewer
at the same time, so generating random names shouldn't be
needed.
Change-Id: I65e0fc07ce608c1d333a90447e18482787a98b8c
Borrowing the index buffer of the current playlist
can be problematic, since it is possible to start
playback of a different playlist from the playlist
viewer without leaving the viewer, thereby causing
a collision.
As long as we have a sufficiently large plugin
buffer, take advantage of it, regardless of
playback state.
When playback is stopped, always resume the playlist
from the control file before loading it, even if the
playlist has finished playing, to prevent running
into invalid indices.
Note:
dcfrefs_handle is initialized to 0 automatically for
the static on_disk_playlist struct.
Change-Id: I2a05a6a51a088ea9ba73858c353525db9e61c36e
When creating a new control file for the current playlist,
the existing file will now be renamed to serve as a backup,
until the next time the control file is replaced again,
allowing you to restore the previous state of the playlist,
in case you accidentally cleared it.
This also means that an entirely new file is created each
time, instead of truncating the existing one.
Change-Id: I2fd96eb8c3940b85df807fd7646dc0359a036351
In the following scenario, a garbage file name would
be written to the control file for the playing track
(resulting in a failure to resume correctly):
- "Keep Current Track When Replacing Playlist" option is set
- A track is playing that was not inserted but instead comes
from a playlist file on disk
- User performs "Playing Next..." -> "Play" on some tracks, so
that the current playlist is replaced (but leaving the playing
track queued)
- User saves the playlist while queued track is still playing
(the offer to remove queued tracks is declined)
The failure occurs because the pl_save_update_control function
assumes that the seek offset for queued files always points into
the control file. Meanwhile, the remove_all_tracks_unlocked
function adds the PLAYLIST_QUEUED flag indiscriminately, even if
a track comes from a playlist file instead of having been inserted.
Theoretically, this could be addressed by adding the playing
track's file name as a parameter to PLAYLIST_COMMAND_CLEAR,
which the track is then updated to point to, unless it
was already inserted (alternatively, we could seek within
the playlist file for such tracks).
Unless I'm missing something, it may be preferable, though, to
get rid of PLAYLIST_COMMAND_CLEAR completely, and instead have the
remove_all_tracks_unlocked function start over with a fresh
control file, into which we insert a single ('P' and) 'Q' command.
This seems to have several advantages:
- When resuming, we eliminate the need to parse and handle all of
the outdated entries again that end up being cleared anyway
- It is ensured that the control file doesn't rely on the existence
of a playlist file anymore after the playlist has been cleared
- We can reset the playlist's file name, which should make it less
likely for the user to overwrite their previous (now unconnected)
playlist that was still displayed in the Save dialog
- Unrelated bookmarks for the previous playlist aren't displayed
anymore
- Improved consistency with existing behavior when the "Keep
Current Track When Replacing Playlist" was disabled.
Change-Id: I41a89295bbac878807d65db9cf67b8a485daf0e5
On usb plug system_flush() gets called before the other
threads get a chance to ACK the insert
system_flush() calls playlist_shutdown() (amongst other things)
playlist_shutdown closes the control file
the audio thread acks USB insert
audio_stop_playback is called which now can't look up the
filename from the now closed handle for control the control file
I left the guard for checking for USB insert but it appears to be
fine without it
Change-Id: I935dbf7aed38d4a57413c0063ad953f427e9b3bb
format_track_path() uses path_append_ex() which might use strlen on the
existing buffer likely a false positive but cheap enough to guard against
Change-Id: I7061a434460f235fc58016f9dcf2dfe4633b9f59
Track Info can now be displayed for the set of
all tracks contained in a playlist. This lets
you calculate a playlist's length, for example,
even if it is not currently playing.
This functionality can be accessed from the
existing "Properties" screen for a selected
playlist file. A line has been added at the
very bottom to show Track Info.
Change-Id: I311532b7cfa9e29d46c0cd5623ba4c06c1dd5b5f
When "Repeat Shuffle" is enabled, it may make sense
to visibly turn on the Shuffle setting once a playlist
ends and starts from the beginning again.
This seems to fix a few issues:
- After (an unmodified) playlist has been shuffled,
bookmarks now behave correctly in terms of restoring
the playlist's shuffled state and using the correct resume
index. This wasn't the case before. The alternative may be
to set the playlist as modified once it is shuffled, to
prevent creation of bookmarks from that point on.
- Lets you return to the un-shuffled state of the playlist at
any point
- Icon makes it easy to tell if the "unmodified" playlist has
been shuffled already, or not, which wasn't obvious before.
Change-Id: I41af04b041fdc7774c9d9267aaf983ec9d402e13
The "Reload After Saving" setting was added in
g3347 (4f83e66) to solve FS#13287, by allowing
you to bookmark a modified playlist after saving,
without having to manually reload it first.
Since the rewrite of playlist_save in g5192
(90e3571), a modified playlist doesn't have to be
reloaded anymore in order to be bookmarked after
it's been saved, unless it contains queued tracks.
To cover the remaining use cases of the previously
available option, Rockbox will now offer to remove
any queued tracks from a playlist when saving it.
Change-Id: I2d6f12bcce14d8ff41a4d921ce84d628774103ac
remove some duplicated strings previously allocd off the stack
just removing string duplications that are easily handled with truncation
now available with path_append_ex()
this also has an advantage of less stack used in worst case scenarios
Change-Id: I3a43e33ef8a8c36599e4c6c036a0ccdd8ed0c883
When saving the current playlist, entries written out
to disk were rotated, but its indices were not, resulting
in first_index being out of sync between the two. Thus,
an incorrect index was used for any playlist commands,
from the moment you saved the playlist until the next
time you resumed, which didn't produce the right playlist.
Change-Id: Ie4b02ce9e07e565b1b15c938cc4b820db08e8f1f
This should have probably been included in e9b4275,
since a control file generated with that build will
potentially cause issues when used with older versions,
in case -8 appears as an insert position.
Change-Id: I4a7097a2bd33c0dbbdad481a341b49be505f34f7
PLAYLIST_COMMAND_CLEAR needs to save the index of the
currently playing track, or a track with index 0 will
be left queued after resuming from control commands
Change-Id: If7449bff92acd556b03c46e82301e8fec5c997d7
add_track_to_playlist_unlocked only increased a playlist's
first index as necessary when its position parameter was
negative (i.e. one of the special insert positions was
specified).
A negative value was not stored in the control file, but
was always converted into an absolute position. Thus, any
adjustments to first_index weren't repeated when resuming
from the control file.
In particular, shuffled playlists were affected (in case of
first_index > 0), when inserting at positions <= first_index,
including appending a track to the end of a playlist. This
works by inserting at first_index and increasing first_index
by 1 afterwards.
Similarly, adding tracks in a shuffled fashion could increase
first index, whenever that was the value randomly calculated
for a track position, effectively appending it (I assume this
is on purpose).
To make sure that first_index adjustments are recovered when
resuming from the control file, and to be able to differentiate
between a prepended or appended track, store the special value
PLAYLIST_INSERT_LAST_ROTATED as the insert position in the
control file whenever first_index would have been used before,
and a special position (other than PLAYLIST_PREPEND) was
provided to the function.
Change-Id: I31f26796627fb136daeddd046cb1892bdf1b4014
I noticed that setting first_index to the
current track when shuffling a playlist
could fail in the Simulator.
Change-Id: Ic9467bd46a93aa2d2b765271110b0baee7058208
The resume index into the playlist file
that was used for bookmarks created immediately
after saving a shuffled playlist, or for reloading
the saved playlist (in case "Reload After Saving"
was enabled), tended to be incorrect.
The playlist file effectively isn't shuffled
anymore after saving it to a file, but the
resume index may still have to be rotated unless
playback has been stopped and resumed before
bookmarking, due to indices that are shifted
by first_index.
Change-Id: Id335a7a71adc216989d7b415bfa48237d92fd7b0
This was added in 5e757b4 as a band-aid to
prevent control data from being discarded,
but has since become superfluous after
the removal of the control file cache in
2e99e21.
Change-Id: Ia7bd84f9442ec1103aee8d3c4454216719aa2d66
Entries from a previously playing playlist may not
have been replaced after selecting a track from
a different playlist in the Playlist Viewer when
playback was stopped (not paused), since the index
buffer of a playlist opened in the Playlist Viewer
is shared with the current playlist when nothing is
playing.
Change-Id: I939e302c73cabd0e9d969550143635e54db32bf0
playlist_save() was a poorly thought out mess. This fixes the
glaring issues and hopefully ensures that saving the playlist
never loses state (such as queued tracks or modified status)
after save+resume.
Indices are now updated on the fly, which is faster and needs
no extra memory. But if an error occurs, the playlist will be
corrupted. There is currently no attempt to handle this since
errors should be unlikely, but some error handling needs to be
added in the future.
Change-Id: If8a5dbd6a596460be08ee0b7bab9f24337886ea4
A playlist's explicit 'modified' flag is now used
for keeping track of whether it's been modified
in the Playlist Viewer, not just in case of the
currently playing list, but for other playlists
as well.
When you start playback of a track from the
Playlist Viewer, a playlist's 'modified'
status is now carried over to the current
playlist, so as to produce a warning when
there is an attempt to replace the list at
a later point. This also prevents (auto)
bookmarking of the playlist if it had been
modified in the Playlist Viewer prior to
becoming the current playlist. (Bugfix)
Change-Id: Ibc391fd69285f8a67d6ffb6d8c274df3d223974c
The bookmark menu with the option to create a bookmark
was inadvertently displayed for new dynamic playlists,
that had no associated folder or playlist file on disk.
(e.g. after selecting some track from the database for
playback), until the playlist was modified by the user.
Change-Id: I9d6809e4d03603c651459415327f28e38162ad53
The modified state is now an explicit flag that has to be
set whenever a user-triggered modification occurs. This is
recorded in the control file to ensure it doesn't get lost
after resume. There may be some places I missed where the
modified flag should be set/cleared, but it seems to work
well enough right now.
Change-Id: I3bdba358fc495b4ca84e389ac6e7bcbef820c219
Still having problems with determining the type of track change
lets try just watching the audio_next/prev functions
Change-Id: Ie4233ff4d4bf49792a6549d7bcd169ff4b1afd20
I recently added track skipping while REPEAT_ONE was set
currently by registering a track skip callback
I'm not entirely happy with the additional constant overhead
of the event callback
Instead I went looking for a way to distinguish
a pending track skip from some limited testing
it appears to work just as well to compare
playback's skip_pending == TRACK_SKIP_AUTO
but the lack of lifetime control worries me slightly
Change-Id: Ic71b4c3925e991f5a1216d16ecd3af6cc777ef1e
open an insert context
add tracks using the opened context
release opened context and sync the playlist
Change-Id: Idea700ffcf42a38dc23e131c92a6c5d325833170
add_indices_to_playlist() wrongly assumes playlist->utf8 means
the playlist has a UTF-8 BOM. It only means the file is UTF-8
encoded, and says nothing about the presence of a BOM.
Trying to seek past a BOM when there is none mangles the first
filename in the playlist. Avoid this by closing & reopening the
playlist, which ensures the BOM (or lack of it) gets detected
properly.
Change-Id: I222fa6fc31e941d0252f1895b393a51694b93b7c
Have get_track_filename() read track seek offset & insert type
itself to avoid duplicating code. Most callers are not interested
in that information, and only use it to call get_track_filename().
Change-Id: I46d50fda10e515f74e27eebdded8e281bbf4456f
It was only used in filetree.c. It's still implemented in Lua so
scripts using rb.playlist_add() won't break, but has been removed
from the Lua API "backend".
Change-Id: I5625a47f0692456008c6b10dee14755151d22f29
Control cache entries cost 24 bytes per command, but cacheable
commands are always less than that when written out to the file.
We can actually cache *more* data by writing commands directly
to the fd (native Rockbox has a 512-byte cache per fd) and it's
much simpler.
Change-Id: Ibb1b9ffa56ef17431b281419a04082e14d0cbd85
The intent behind the check is to only do this if the filename
comes from an on-disk file. With the RAM buffer is gone this is
now "always" the case.
Change-Id: I4c26e32c482dde176b69a6071e9562e9955d7171
