Eliminate special case for transition to the Shortcuts menu.
The SBS refresh doesn't cause a screen update anymore, and
has negligible cost, so do it regardless.
Reverts the changes to quickscreen.c introduced in commit
dfd9c10 ("Eliminate skin updates in between activities")
+ simplify quickscreen_draw_item (no effect on behavior)
Change-Id: I3fca976de554c720cb69a41566fd0d0a7e6ea92d
Fixes possible crash in case of pathological
UI vp dimensions, when attempting to clear a
QS item's vp.
E.g. https://github.com/federicoplg/musicOS
(commit c5e8570) configures a 1x1 UI viewport
at x=319 y=239, resulting in left/right item
viewports that have negative width or height.
Minor additional simplifications with no
effect on behavior:
- Combine actions requiring cleanup in a setup function.
Put it below the existing cleanup function
- Rename quick_screen_quick to quickscreen_show, and
quickscreen_run to quickscreen_main
- Iterate over items using FOR_QS_ITEMS macro
- Remove quickscreen_draw_item's int_value
- quickscreen_update: immediately continue if qs skinned
- Remove some obsolete #includes
Change-Id: I49aea3be837b861bdcd5132e84fef858df72327b
not sure if there's a reason these weren't included
in g#3229
- simple lists
- playlist viewer
- browse_id3
- option select
Change-Id: I0e15aaeb563e68ea21bf5df667ce134d01a95195
add get_metadata_afmt function so we don't have to extra functions
remove unneeded bounds check on audio_format in rbcodec_format_is_atomic()
add bounds check on audio_format in get_metadata_afmt()
Change-Id: I76bd869100b000579c6546f0670ba4ba2c541f22
tagcache.c add_tagcache() and potentially
skin_tokens.c wps_playlist_percent_prepare()
make calls to probe_file_format() prior to calling get_metadata_ex
resulting in some small amout of duplicated work
especially in the case of add_tagcache this can add
up to a lot of duplicated work
breaks out audio_fmt so these can supply the afmt other callers just
supply probe_file_format(trackname) in the function call
Change-Id: I8084213b8ee7e04d76dce0986beb83d443ac804b
If Playlist Viewer can't open instantaneously due
to the retrieval of metadata, or on devices with disk
storage, make sure the UI doesn't feel unresponsive
Change-Id: Ib7a837515b4fb89ee0ee34ec04541200248b1d6f
Reduces GNU Complexity score from 33 to 2.
Additionally:
- Allocate MAX_PATH, instead of MAX_PATH+1,
for formatted id3 info (NULL terminator is
already included)
- Ensure minimum name buffer size of 2*MAX_PATH,
instead of MAX_PATH+1, so that there is space for
both the formatted track name and, if enabled, the
id3 info of a single track
Change-Id: Ie60590f6c0da4fe3a515b5643494a29cfc56de50
For some reason this resolves an apparently-unrelated warning about a const
violation inside playlist_viewer_init().
Change-Id: Ifd4eb008335876707abe64da04a3afd0d1c46633
Metadata for formatting the trackname can
be read from the database instead of from
disk. I think this was changed by mistake,
when a single function was added for both
Show Track Info and track name formatting.
Change-Id: I903d36bb513898242505f01562340924bf642a12
* Funky macro-based definitions for memchr and strstr
which require an #undef before we use our own in codecs & plugins
* Return value of of strstr is const
Still have several more warnings and link failure with some plugins
but this is a good start.
Change-Id: Ife1f2d3e6f0e0629e3125a9058abc39c6102f452
Addresses issue where it became difficult to find the
currently playing track once you scrolled away from it,
either with icons disabled in the Playlist Viewer, or if
the theme's icon set used whitespace for the Music icon.
An example scenario would be when you're moving some track
and are trying to put it next to the currently playing one.
Change-Id: I073a7e55fb723eeff755e32a78f88dcc7db1c245
When deleting files or directories, you will now see
the path's basename in the previously empty title of
an SBS, making it a bit easier to immediately know
whether you've selected the correct item.
Matching titles were also added everywhere else that
confirm_delete_yesno is used.
The full path of the item is still displayed below,
and continues to scroll, so that themes without a
title, such as the default cabbiev2, will look the
same.
Change-Id: I32422cfbbf6e680f58456237380176617789cac3
Add voice equivalent to splashf messages that require
pauses between individual announcements, or that need
to include their arguments
Change-Id: Ie72326a2371f242de8999761278b22080ba33c81
Use "All" prefix when the final item count is announced
in display_playlist_count and search_playlist
("All 2000 tracks saved"), to make it possible to tell
when the operation has finished.
Change-Id: I90419599fb30da511ee4671b8f0f7f92e2098fd0
* Rename LANG_RESET_ASK to LANG_ARE_YOU_SURE,
so that it matches the actual language string
(translations remain valid), and can be repurposed
for generic confirmation prompts, where the
first line says "Are you sure?", and the second
line reiterates the selected action
* Add "Reset Settings" as second line to the prompt
shown before resetting settings, instead of just
asking "Are you sure?"
* Make Shuffle prompt consistent between WPS and
Playlist Viewer, and ask whether user is sure they
want to Shuffle instead of warning them that the
current playlist will be erased, which was a bit
misleading
* Explicitly say "Cancelled" when user answers NO to
erasing the current playlist or to overwriting a file.
Improves consistency with other prompts that are
displayed for potentially destructive actions, e.g.
before items are deleted, renamed, saved, or reset.
* PictureFlow: Prompt before rebuilding/updating cache
Change-Id: Id8ae36db7187315eb1a374701307e6ab4dcdbd1d
Basically, this just replaces str(STRID) with ID2P(STRID).
The voiced version of these strings cannot not have any format
specifiers (enforced by the language tooling) and are instead more
generic.
As many of these are error conditions, it is doubly important for
them to be voiced in some way.
There are some places in the code that perform their own voicing
for splash messages (eg the shutdown code); those are left alone.
Change-Id: I7d51af351e8fa5c4beee42fbfc02719f1d6591b8
With no control file present, the message
"Error accessing playlist control file"
was displayed, due to playlist_resume being
called from gui_usb_screen_run.
You were likely to only run into this after a clean
install, if you hadn't listened to any music yet.
It probably makes sense to move the check for the
existence of the control file into playlist_resume,
and not even have it show a separate access error
message when the file doesn't exist.
Change-Id: Ib3a643c43d3d4d499fa5a13c01955779d69cd357
- 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
When playback was stopped (not paused), deleting
the first track of the current playlist started
playback.
Change-Id: I0bd09e1254399d003109215e4e8363a0cd045ac0
Missed this in 6a865f2430
If you entered the Playlist Viewer using the
context menu option "View", instead of from
the Playlist Catalogue, a playlist file that was
deleted due to the removal of all of its tracks
would still be displayed in the directory when
returning
Change-Id: I54ca34f70344e289d9e2cb413ee2a38e5bd7a92b
1) Don't re-initialize the Playlist Viewer before
closing it, after making an on-disk playlist the
current playlist. It's obviously wasteful and this
seems to have only historical reasons that don't
apply anymore.
2) Naming adjustments:
- change "onplay" prefix to "context" (e.g.
pv_context_result context_menu, instead of
pv_onplay_result onplay_menu), since onplay requires
being aware of the history for it to make any sense
- rename update_lists to update_gui, to differentiate
it from update_playlist
- change update_viewer_with_changes to update_viewer
3) gui_synclist_del_item(playlist_lists) is not
needed anymore after removing a track, because we
already indiscriminately reset the synclist's
number of items and reselect within the list's
new bounds from update_viewer_with_changes.
As a result, MODIFIED is applicable to both
reshuffling and to removing a track, so
ITEM_REMOVED can be eliminated as its own
enum value.
Plus, SAVED does not require a special case and
can be renamed to PL_UPDATE, so it is usable
when track display is adjusted, in order to
force an update of the playlist buffer.
Change-Id: Ic5202e03223a1ddbb9ff87e388a16ecde39165ef
When you removed all tracks from an on-disk playlist,
you were asked whether you wanted to save it, which
would fail, but did not show any accompanying message.
Instead of trying to save the playlist, after getting
the confirmation, delete the playlist file from disk.
Change-Id: Iad54f2f490b15dd9c8a8fdfb8f12e58fd17d5e36
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
It makes no sense to prevent the current playlist
from resuming when some other playlist becomes empty.
Change-Id: Ibfb7a15b0826ed3baeefbc5060b13d73894b81e6
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
Playlist Viewer falls back to splashf if there is not
enough plugin buffer space left for running the
view_text plugin .
Change-Id: I418731018b03f396270b68e5e2d2e69635df1af0
Playlist Viewer hangs indefinitely on tracks with long Comment fields
Commit 2156d98 (metadata: mp3: Improve support for long tags) has
allowed the length of parsed tags to exceed ID3V2_MAX_ITEM_SIZE.
Overly long tags may blow the stack due to a stack-allocated buffer
in the metadata parsing routine, that can reach 3 times the size of
the id3v2 and id3v1 buffers combined.
This patch moves the mp3entry struct used by the Playlist Viewer
off the stack and onto the plugin buffer instead, which helps prevent
a stack overflow in cases such as the example posted in FS#13518.
Future patches to either constrain ID3V2_BUF_SIZE to ~1120 bytes or
to lower the stack usage of metadata parsing will still be required
to fully address the issue.
Change-Id: I2f436195bc8df4d785b1880f6ab167783a162020
Offer new options to show elegantly your entries in any playlist/dynamic playlist viewer. This is especially important if you dual boot an iPod with Stock OS and want to sync with iTunes; with this very popular setup, file names are obfuscated which results in any Rockbox playlist viewer difficult to enjoy, and it was a long standing issue reported by several Rockbox users over the years. The only way to show the title was to open a contextual menu on each song to get infos about the selected song, which is a very long and anti-ergonomic process to understand what is on your current playlist/randomized playlist. The idea of this patch is to provide new alternatives that the user can select. I personally selected the Title & Album view which provides excellent readability.
This patch was built with performance in mind using lazy loading to load one by one the tags then cache a string and use the little cache as much as possible to make scrolling in the same area as smooth as possible. Performance remains very acceptable even on an iPod 4G with its original hard drive. Using a real compact flash with my iPod Mini 2G reduces the latency even more.
Those new options are disabled by default because they impact noticeably the scrolling performance and are less relevant if your files are decently properly named.
Unfortunately, the search feature in a playlist will need to continue to use the raw filename because reading the tags for a whole playlist is a performance disaster. This works decently while viewing just because I made a code that load those one by one as much as possible.
I focused also on testing that the opening of the playlist viewer UI remained fast, and loading one by one with lazy loading allows to get very little overhead in this regard.
Change-Id: I00d9c802e29f8372447813b035bbae207a016467
This improvement brings a huge performance improvement to start a random mix of your library. Previously, the only way to do this was to increase the size of a playlist with absurd sizes number. Now it will respect the limitation but will insert random songs from the current view.
Database: Add true random songs in playlist if it is gonna exceed its maximum capacity
More context is available here : https://www.reddit.com/r/rockbox/comments/1ez0mq4/i_developped_true_full_library_shuffle_for/
Also :
- Improved layout in the DB browser
- New default max playlists capacity is now 2000 on old PortalPlayer targets to give a better user experience and not having to wait dozens of seconds while creating a playlist
- "Show insert shuffled" option is now true by default
- Add a new shortcut to play all songs shuffled in the DB browser
- Now the feature is fully optional and enabled only on targets that have more than 2MB of RAM
- Add entries about this feature in the manual to explain it to the users
Change-Id: I1aebaf7ebcff2bf907080f1861027d530619097c
Change-Id: I3354923b148eeef1975171990e814a1a505d1df0
reading the disk works fine for on disk playlist but
trying to read from the disk with the current playlist
becomes unbearably slow
removes the static playlist_track_info prefering the unused one
already on the stack from search_playlist()
Change-Id: I01b836b4fe46bb51ef6a28d5db6b3f9cdc7d1e51
the extra title pointer and alignment adds around 1k to the bss area
since we already have a pointer to track->name we can just save an offset
for the title data
Change-Id: I3a19857631d70276134bcc97940824a3e2f80e4a
Not sure this is a great idea from disk and battery standpoint
but there is no reason you can't..
using the name buffer to fill title data
prevent hitting the disk for each screen scroll
add get_metadata_ex to allow flags
METADATA_EXCLUDE_ID3_PATH
prevent copying the filename to the ID3 struct
METADATA_CLOSE_FD_ON_EXIT
instead of seeking to the beginning the file is closed before
get_metadata returns
add logic to allow a invalid fd to signal that get_metadata
should open and close the file within its call
bugfix per Chris_s don't use the tagcache for the trackinfo
Change-Id: Ic7a595b39a8d7a57f975312bc9c8bb4111f22a88
A playlist that hasn't finished and is resumed without starting
playback has its index set to 0 by empty_playlist_unlocked.
So ignore the index for stopped playlists.
Change-Id: I575ceacbcd2369eaa3ae4e47de7b2f7e999248f5
Moves currently playing track to the front of
the playlist when shuffling.
Also fixes issues with an incorrect resume index
after repeatedly applying shuffle.
Change-Id: Ieff76e47318a07ee5004b063ded439f2af4bae2f
For any selected track that is part of a playlist,
additional info about the list is now provided in
the [Playlist] field of the Track Info screen.
1) Asterisk indicates if playlist has been modified.
2) Playlist filename is visible, unless the current
playlist is *not* associated with a file on disk, in
which case the following will be shown instead:
- (Folder) for unmodified folder playlists.
- (Dynamic) for playlists that are neither associated
with a playlist file, nor with a folder.
Change-Id: I9dcf7cbba4ac2e37b23413180a2b2bf4bbe5ee2a
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 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
