mutexes are in just trying to refactor the rest and make it a smaller
and more robust system
--Done
Change-Id: If64807c3e0ee1966f7593795f26f1f538caf831b
get_filename() changes the seek pos with out restoring it
seek back to the beginning or after the BOM if utf8
--
the other option is to open our own file descriptor this will
remove the need for the mutex but it would no longer block get_filename
from getting potential stale / bad data
Change-Id: I0d2b8a1a297c7aaf453b3bc558b2b5b53dbe591b
I'm pretty sure this is a very old bug I traced it down to the
current_playlist getting changed out from under add_indices_to_playlist
causing myriad of issues from buffer full to invalid control file to shifting indices
this only appears to happen with the dircache on
I still get an incorrect resume state with the wrong song very rarely -- turns out get_filename seeks the file FIXED
Some debugging left in for now till we can verify there are no other instances
Change-Id: I289a775462eddfe93da4a326dc9e38605af06816
There's probably little benefit to using core_alloc_maximum() for
loading playlists since they are parsed incrementally. I/O speed
does not increase with increased read sizes beyond a certain point.
Read by 32 KiB chunks since that is what the buffering thread does.
Fall back to core_alloc_maximum() if a small allocation fails so
that buflib will try harder to free up space.
Change-Id: I08b94317d12b98af09ef2bd84aa1195c4c51d1b1
replace applicable calls to strlcpy with calls to strmemccpy
which null terminates on truncation
in theory the strmemccpy calls should be slightly faster since they
don't traverse the rest of the source string on truncation
but I seriously doubt there is too much of that going on in the code base
Change-Id: Ia0251514e36a6242bbf3f03c5e0df123aba60ed2
the loading track splash flashes and is ugly
add a function to display a progressbar along with the splash message
spruce up database commit message as well
Change-Id: I2749b958c1ee5dad2631a5f999a4b00ddca7f225
Shuffle and Unshuffle commands are not flushed to
disk when control data is updated. The same applies
to Delete and Reset commands, unless HAVE_DIRCACHE
is undefined (see update_control() function in playlist.c)
playlist_resume() discards cached control data.
This resulted in a bug where (e.g.) removed tracks
from the current playlist would reappear if you stopped
and resumed playing immediately afterwards (instead of
restarting in between).
Change-Id: I273f61e823a1d99426a18079b81aa07915620f30
A couple of places use sizeof(int) for allocations and copying but
the indices are longs, which causes bugs in the simulator on 64-bit.
Change-Id: Ie101ac57d44217c4b1657cf0152c97e276bd7043
The comment regarding Windows paths was fairly confusing as to
its intent. What the code is trying to do is replace the drive
letter with the volume containing the playlist, which appears
reasonable. The middle of the comment was devoted to explaining
why the code below was potentially incorrect which only served
to add to the confusion.
AFAICT the last volume specifier in a path will cause search to
start from the root of that volume, so any incorrect result can
be avoided by using the new function path_strip_last_volume().
Finally, add a comment at the top of the function that explains
what it does.
Change-Id: If4e4938801f2f81eb52f5d32b5461872995e5e83
with root redirect and even relative paths eventually we need the dircache
to get files from the disk
Change-Id: Ia443f473f09dd534674d5fdb71251214ce01eed7
I ocassionally see Playlist Invalid messages but which part is failing?
eventually it works so its probably a race
Change-Id: Ib2b09ab13ce09a55130430a2e94bc0498443b6ec
There are various allocations that can't be moved or shrunk.
Provide a global callback struct for this use case instead of
making each caller declare its own dummy struct.
Also fixed ROLO and x1000 installer code which incorrectly
used movable allocations.
Change-Id: I00088396b9826e02e69a4a33477fe1a7816374f1
the splash suffices as warning let the calling code decide what to do with it
everything I saw calling expected failure with a -1 return so it shouldn't
cause any issue removing the panic
Change-Id: Idc1fba91c3ab4f1fd78e1d6a07dc2e6a0bfd8970
failure to close file handles
reading memory prior to buffer on error
loop variable integer overflow on error
Change-Id: I2893c34cd041d085fd7f56a88cb4cb14131cea11
The function to check whether a playlist has been modified
will now only try to resume a playlist, if the global resume index
is not -1.
This also means that replacing a finished (modified) playlist will not
produce a warning anymore if Rockbox has been restarted in
between. In that respect, the behavior is identical to
what it was *before* 46085c8978.
Change-Id: Ib95f89043274f1c72c75023d1506fc093aaf4cf9
Music currently doesn't begin playing after the user has selected a new song from a playlist or using the database/file browser, unless another item was already playing or the previous player state was "stopped" but not "paused". This results in an inconsistent user experience. The user's likely intention is to immediately listen to a song they select regardless of the previous player state.
Change-Id: I574c3fe5c12fee544da0569182fc9573d307ac2f
Rockbox would previously not present a warning when you were about
to replace a modified playlist after restarting the player,
unless you first resumed playback.
Change-Id: I9752228f84b105747e41ce6e1c8f95f0f9f14c10
Discovered on ClipZip but applicable to other LOW MEM targets
Back off voice buffer on playlist_resume
Change-Id: Ia316c4b7fc1bcb089d3069a13dd7c193edf2ba1e
After using “Insert Next” to insert multiple songs at once (e.g. an album from the database browser or folder from the file browser), subsequent Insert operations will incorrectly insert items after the first song of all items that were previously inserted, instead of after the last song of the previously inserted items.
A bug fix was originally written by Costas Calamvokis for the file browser only. I adopted the original fix and added code analogous to it so that it works from the database browser as well.
See FS#7898, FS#7363 or this forum post for more info:
https://forums.rockbox.org/index.php/topic,53741.0.html
Change-Id: Ie2718e136df0b340000f7a171e9e806cf23a27b4
Music currently doesn't begin playing after the user has selected a new song in a playlist, unless another item was already playing or the previous player state was "stopped" (but not "paused"). This results in an inconsistent user experience. The user's likely intention is to immediately listen to the song they select for playback regardless of the previous player state.
Change-Id: I68a8da01b06a81d8c3b61e351710431152bfdfaa
Note: I left behind lcd_bitmap in features.txt, because removing it
would require considerable work in the manual and the translations.
Change-Id: Ia8ca7761f610d9332a0d22a7d189775fb15ec88a
'swcodec' is now always set (and recording_swcodec for recording-capable
units) in feature.txt so the manual and language strings don't need to
all be fixed up.
Change-Id: Ib2c9d5d157af8d33653e2d4b4a12881b9aa6ddb0
While playing a track the playlist viewer may not
have a big enough temporary buffer to load and display
'max_files_in_playlist' entries
This patch attempts to load as many entries as possible
If tracks were already playing (dynamic playlist or otherwise)
The original code only gave half the plugin buffer to a playlist
loaded from file
On some targets half the plugin buffer is not enough to load all entries…
Now we attempt to get as many entries possible while at least leaving a
small buffer (MAX_PATH) for the name buffer
Change-Id: Ic06eaabc4e2550f076d625957d6d073790852743
I observed a crash on buflib>move_block
after dumping ram I noticed that the buffer for filetypes was being corrupted
tree_get_entry_at returns a entry from the buflib 'tree entry' buffer
filetree.c->ft_load writes data to this buffer before checking if it has
reached the last entry resulting in buffer overflow that overwrites the
next entry in the buffer ['filetypes']
Patch checks that the index passed to tree_get_entry_at() is in range
otherwise it returns NULL
Added checks + panic in other functions using tree_get_entry_at()
Fixed tree_lock_cache() calls in playlist and filetree
Change-Id: Ibf9e65652b4e00445e8e509629aebbcddffcfd4d
Playlist was CRC-ing the path from the id3, which may have been
modified to remove "bogus dirs". This would cause a CRC mismatch
in the resume information.
Now, just use the current playlist's current index and call
playlist_get_filename_crc32() to get the original path when
updating resume info.
While technically correct, if this causes any issue(s) it's just
a one-line change and painless to revert.
Change-Id: Ie595ef6c40349c342bd7acac8c542829f9cd5d76
Playlist dircache references should be back in working order.
Reenabling dircache references in the database ramcache is not
yet done as it requires quite a bit of rework. Otherwise, the
database in RAM is functional again.
Some buffer compatibility changes have been made for database
commit because the dircache buffer can no longer be stolen, only
freed by an API call.
Change-Id: Ib57c3e98cb23e798d4439e9da7ebd73826e733a4
Some changes in behavior were made with filesystem code commit
for the sake of compatibility that changed expected behavior.
* Restore substitution of drive spec in fully-qualified DOS paths
with the playlists's volume spec (or root on univolume targets).
Drive-relative paths of the form "c:foo" (no separator after
':') will be treated as purely relative.
* Restore old behavior of preserving leading whitespace in the
source path and trimming only trailing tabs and spaces.
* Multivolume: Volume substition on fully-qualified UNIX/RB paths
has NOT been reintroduced (and perhaps wasn't intended in the
first place). They will not be modified because there is no
ambiguity to resolve. Doing so would prevent a playlist on
external storage from referencing a file on main storage without
qualifying it with "/<0>...".
* Plain relative paths are and always have been interpreted as
relative to the location of the playlist.
Change-Id: Ic0800cea79c59563b7bac20f8b08abb5051906c7
This should only be a problem if the last line
is not terminated by \r or \n though.
cppcheck reported:
[rockbox/apps/playlist.c:234]: (style) Array index 'i' is used before limits check.
Change-Id: I8182b66272ba9c024984c81588bd2a6dbb8255b8
This patch redoes the filesystem code from the FAT driver up to the
clipboard code in onplay.c.
Not every aspect of this is finished therefore it is still "WIP". I
don't wish to do too much at once (haha!). What is left to do is get
dircache back in the sim and find an implementation for the dircache
indicies in the tagcache and playlist code or do something else that
has the same benefit. Leaving these out for now does not make anything
unusable. All the basics are done.
Phone app code should probably get vetted (and app path handling
just plain rewritten as environment expansions); the SDL app and
Android run well.
Main things addressed:
1) Thread safety: There is none right now in the trunk code. Most of
what currently works is luck when multiple threads are involved or
multiple descriptors to the same file are open.
2) POSIX compliance: Many of the functions behave nothing like their
counterparts on a host system. This leads to inconsistent code or very
different behavior from native to hosted. One huge offender was
rename(). Going point by point would fill a book.
3) Actual running RAM usage: Many targets will use less RAM and less
stack space (some more RAM because I upped the number of cache buffers
for large memory). There's very little memory lying fallow in rarely-used
areas (see 'Key core changes' below). Also, all targets may open the same
number of directory streams whereas before those with less than 8MB RAM
were limited to 8, not 12 implying those targets will save slightly
less.
4) Performance: The test_disk plugin shows markedly improved performance,
particularly in the area of (uncached) directory scanning, due partly to
more optimal directory reading and to a better sector cache algorithm.
Uncached times tend to be better while there is a bit of a slowdown in
dircache due to it being a bit heavier of an implementation. It's not
noticeable by a human as far as I can say.
Key core changes:
1) Files and directories share core code and data structures.
2) The filesystem code knows which descriptors refer to same file.
This ensures that changes from one stream are appropriately reflected
in every open descriptor for that file (fileobj_mgr.c).
3) File and directory cache buffers are borrowed from the main sector
cache. This means that when they are not in use by a file, they are not
wasted, but used for the cache. Most of the time, only a few of them
are needed. It also means that adding more file and directory handles
is less expensive. All one must do in ensure a large enough cache to
borrow from.
4) Relative path components are supported and the namespace is unified.
It does not support full relative paths to an implied current directory;
what is does support is use of "." and "..". Adding the former would
not be very difficult. The namespace is unified in the sense that
volumes may be specified several times along with relative parts, e.g.:
"/<0>/foo/../../<1>/bar" :<=> "/<1>/bar".
5) Stack usage is down due to sharing of data, static allocation and
less duplication of strings on the stack. This requires more
serialization than I would like but since the number of threads is
limited to a low number, the tradoff in favor of the stack seems
reasonable.
6) Separates and heirarchicalizes (sic) the SIM and APP filesystem
code. SIM path and volume handling is just like the target. Some
aspects of the APP file code get more straightforward (e.g. no path
hashing is needed).
Dircache:
Deserves its own section. Dircache is new but pays homage to the old.
The old one was not compatible and so it, since it got redone, does
all the stuff it always should have done such as:
1) It may be update and used at any time during the build process.
No longer has one to wait for it to finish building to do basic file
management (create, remove, rename, etc.).
2) It does not need to be either fully scanned or completely disabled;
it can be incomplete (i.e. overfilled, missing paths), still be
of benefit and be correct.
3) Handles mounting and dismounting of individual volumes which means
a full rebuild is not needed just because you pop a new SD card in the
slot. Now, because it reuses its freed entry data, may rebuild only
that volume.
4) Much more fundamental to the file code. When it is built, it is
the keeper of the master file list whether enabled or not ("disabled"
is just a state of the cache). Its must always to ready to be started
and bind all streams opened prior to being enabled.
5) Maintains any short filenames in OEM format which means that it does
not need to be rebuilt when changing the default codepage.
Miscellaneous Compatibility:
1) Update any other code that would otherwise not work such as the
hotswap mounting code in various card drivers.
2) File management: Clipboard needed updating because of the behavioral
changes. Still needs a little more work on some finer points.
3) Remove now-obsolete functionality such as the mutex's "no preempt"
flag (which was only for the prior FAT driver).
4) struct dirinfo uses time_t rather than raw FAT directory entry
time fields. I plan to follow up on genericizing everything there
(i.e. no FAT attributes).
5) unicode.c needed some redoing so that the file code does not try
try to load codepages during a scan, which is actually a problem with
the current code. The default codepage, if any is required, is now
kept in RAM separarately (bufalloced) from codepages specified to
iso_decode() (which must not be bufalloced because the conversion
may be done by playback threads).
Brings with it some additional reusable core code:
1) Revised file functions: Reusable code that does things such as
safe path concatenation and parsing without buffer limitations or
data duplication. Variants that copy or alter the input path may be
based off these.
To do:
1) Put dircache functionality back in the sim. Treating it internally
as a different kind of file system seems the best approach at this
time.
2) Restore use of dircache indexes in the playlist and database or
something effectively the same. Since the cache doesn't have to be
complete in order to be used, not getting a hit on the cache doesn't
unambiguously say if the path exists or not.
Change-Id: Ia30f3082a136253e3a0eae0784e3091d138915c8
Reviewed-on: http://gerrit.rockbox.org/566
Reviewed-by: Michael Sevakis <jethead71@rockbox.org>
Tested: Michael Sevakis <jethead71@rockbox.org>
