| 00:11:43 | _bilgus | dconrad sounds good let me know if you want need desire anything else out of that patch and build on top we can push them as a package when you are ready |
| 00:31:05 | | Quit dconrad (Remote host closed the connection) |
| 00:55:07 | | Quit othello7 (Quit: othello7) |
| 01:00 |
| 01:24:24 | *** | Saving seen data "./dancer.seen" |
| 03:00 |
| 03:24:26 | *** | No seen item changed, no save performed. |
| 04:00 |
| 04:14:01 | _bilgus | OlaroFR (logs) I pushed a new version of your patch that should do what you want |
| 04:15:14 | _bilgus | You shouldn't need to even keep track of unique files in this context because you are searching by filename in insert_all_playlist() and ditto with directories of files too |
| 04:16:59 | _bilgus | After looking for a bit I do believe the only time that unique buffer is needed is when you do searches thru the db with tags selected because then you might get multiple tags linking back to a file but searching by filename is going to be unique in the db |
| 04:18:32 | _bilgus | so anyway on the code I think the code that re-calculates could probably use some work still but I got it pretty close to hitting the very end of the db pretty much every time I ran it |
| 05:00 |
| 05:04:18 | | Quit jacobk (Ping timeout: 248 seconds) |
| 05:05:11 | | Join jacobk [0] (~quassel@47-186-105-237.dlls.tx.frontiernet.net) |
| 05:24:30 | *** | Saving seen data "./dancer.seen" |
| 06:00 |
| 06:05:18 | | Join OlsroFR [0] (~OlsroFR@user/OlsroFR) |
| 06:28:21 | | Quit Nezumi-sama (Ping timeout: 276 seconds) |
| 06:32:20 | | Join Nezumi-sama [0] (~narf@syn-067-053-148-069.biz.spectrum.com) |
| 07:00 |
| 07:24:31 | *** | Saving seen data "./dancer.seen" |
| 08:00 |
| 08:23:14 | | Join dconrad [0] (~dconrad@152.117.104.217) |
| 08:33:51 | | Quit dconrad (Remote host closed the connection) |
| 08:33:59 | rb-bluebot | Build Server message: New build round started. Revision f6e8c20188, 337 builds, 9 clients. |
| 08:33:59 | rb-bluebot | FS #13475: Updated Polish translation (Adam Rak) by Solomon Peachy |
| 08:36:27 | OlsroFR | Hey _bilgus, |
| 08:36:27 | OlsroFR | I've read and tested your proposal on my iPod Mini. |
| 08:36:28 | OlsroFR | Advantages of your patch : |
| 08:36:28 | DBUG | Enqueued KICK OlsroFR |
| 08:36:28 | OlsroFR | - Memory footprint of your patch is close to 0 |
| 08:36:29 | OlsroFR | - I couldn't find any performance regression; your code avoid duplicates |
| 08:36:29 | *** | Alert Mode level 1 |
| 08:36:29 | OlsroFR | - It's still an improvement compared to the default and current Rockbox behavior about this, which is to ignore and drop what exceeds the playlist size. |
| 08:36:30 | *** | Alert Mode level 2 |
| 08:36:30 | OlsroFR | Disavantages of your patch : |
| 08:36:30 | *** | Alert Mode level 3 |
| 08:36:30 | OlsroFR | - Your code always trigger an error userside upon completion (at least during my tests) |
| 08:36:31 | *** | Alert Mode level 4 |
| 08:36:31 | OlsroFR | - You use very small segments, which greatly affects the quality of the randomness. During each tries, you will never have entire artists ignored. |
| 08:36:31 | *** | Alert Mode level 5 |
| 08:36:31 | OlsroFR | - Since the size of the segment is calculated and increasing slightly during the "for" loop, your code will tend to ignore randomly less songs at the beginning of the fill process. |
| 08:36:32 | *** | Alert Mode level 6 |
| 08:36:32 | OlsroFR | Here is one of my result with your patch : |
| 08:36:32 | *** | Alert Mode level 7 |
| 08:36:32 | OlsroFR | last 19938 |
| 08:36:33 | *** | Alert Mode level 8 |
| 08:36:33 | OlsroFR | total 20937 |
| 08:36:33 | *** | Alert Mode level 9 |
| 08:36:33 | OlsroFR | max rand 19 |
| 08:36:34 | *** | Alert Mode level 10 |
| 08:36:34 | OlsroFR | With a max rand of 19, in this context your code means like in human language : "take almost always one random song from each album of your library". |
| 08:36:34 | *** | Alert Mode level 11 |
| 08:36:34 | OlsroFR | By building a truly random playlist to be put on a limited storage space, we should expect some (or even many depending of the luck) albums to be completely ignored. Depending of the randomness, multiple songs from one single album should be able to be picked. |
| 08:36:35 | *** | Alert Mode level 12 |
| 08:36:35 | OlsroFR | After testing yours, I want to maintain my proposal because I feel like it's a better one and the memory footprint for this is now very reasonable. It's about 256x lower than before, the little static buffer is now just 1kb. I also like my proposal because you can adjust it easily from a static const if you wanna ever use larger segments. In my |
| 08:36:35 | *** | Alert Mode level 13 |
| 08:36:35 | OlsroFR | opinion, a 1024 segment seems to be a very good compromise to get an high quality randomness feeling each time while, keeping the static memory usage very low compared to the first proposal which was doing true randomness. |
| 08:37:34 | OlsroFR | https://gerrit.rockbox.org/r/c/rockbox/+/5911 ready to review |
| 08:45:35 | rb-bluebot | Build Server message: Build round completed after 697 seconds. |
| 08:45:37 | rb-bluebot | Build Server message: Revision f6e8c20188 result: All green |
| 08:46:36 | *** | Alert Mode OFF |
| 09:00 |
| 09:17:29 | OlsroFR | I just noticed that I need to implement a bit my implementation about how to handle the latest segment. Currently in my code I just take the rest but this is bad for true randomness. I should balance all segments to the possible closest sizes. So don't merge it now |
| 09:24:33 | *** | Saving seen data "./dancer.seen" |
| 10:00 |
| 10:12:49 | _bilgus | OlsroFR, thats just a tuning mode we can get similar performance using a peroper sampling algo |
| 10:12:54 | _bilgus | proper* |
| 10:13:43 | _bilgus | and its still not going to be near 2k bin size increase with similar performance |
| 10:31:56 | _bilgus | the other option I'd be ok with is double checking that the uniq buffer is indeed unused (pretty sure it is in these contexts) and use that as your reservoir |
| 10:32:14 | OlsroFR | OK. I am looking that ways |
| 11:00 |
| 11:24:37 | *** | No seen item changed, no save performed. |
| 13:00 |
| 13:12:02 | | Join davisr [0] (~davisr@fsf/emeritus/davisr) |
| 13:24:39 | *** | No seen item changed, no save performed. |
| 13:31:33 | | Quit Galois (Ping timeout: 248 seconds) |
| 13:39:29 | | Join Galois [0] (djao@efnet.math.uwaterloo.ca) |
| 14:00 |
| 14:21:45 | | Quit user890104 (Ping timeout: 260 seconds) |
| 14:22:02 | | Join user890104 [0] (~Venci@freemyipod/user890104) |
| 15:00 |
| 15:03:36 | OlsroFR | _bilgus : https://gerrit.rockbox.org/r/c/rockbox/+/5911 pushed new improvements and refactoring. Dynamic allocation is here now |
| 15:05:34 | OlsroFR | I improved also the flaw in my randomness ; the last segment could be very unbalanced depending of the size of the music library. Now the last segment will be balanced with the others as possible |
| 15:13:52 | OlsroFR | On my device, I have 20937 songs. After the rebalancing process, it use perfectly balanced segments. 21 segments of 997 tracks. In each 997 tracks, 95 tracks will be choosen with pure random |
| 15:24:42 | *** | Saving seen data "./dancer.seen" |
| 15:29:09 | OlsroFR | Well I found another things to improve/fix, I will push another amend in 30 minutes |
| 15:54:31 | OlsroFR | Pushed. Should be perfect now |
| 16:00 |
| 16:12:35 | | Quit davisr (Quit: yeehaw) |
| 16:12:58 | | Join davisr [0] (~davisr@fsf/emeritus/davisr) |
| 16:53:44 | | Quit kugel_ (Ping timeout: 245 seconds) |
| 16:57:53 | | Join kugel_ [0] (~kugel@ip4d146a3a.dynamic.kabel-deutschland.de) |
| 17:00 |
| 17:00:48 | | Quit kugel_ (Client Quit) |
| 17:02:11 | | Join kugel_ [0] (~kugel@ip4d146a3a.dynamic.kabel-deutschland.de) |
| 17:07:42 | | Join bilgusphone [0] (~bilguspho@107.123.53.47) |
| 17:09:49 | bilgusphone | OlsroFR ill have to look closer when i get to a PC but that looks pretty good one note you can still yield with that allocation in hand you just need to either pin it, or give it a FN that can move and or shrink the alloc |
| 17:10:44 | bilgusphone | Former is probably good enough for your use, i take it the UNIQ buffer was in use in one of your contexts? |
| 17:16:55 | | Quit bilgusphone (Ping timeout: 244 seconds) |
| 17:21:52 | | Quit kugel_ (Remote host closed the connection) |
| 17:24:44 | *** | Saving seen data "./dancer.seen" |
| 17:28:50 | OlsroFR | bilgusphone I cannot use the uniqbuffer, it's initialized again each time "retrieve_entries" is called |
| 17:29:24 | OlsroFR | and retrieve_entries is called when tagtree_get_entry is called, and we call this to build the playlist |
| 17:30:52 | OlsroFR | so using the uniqbuf in this context can't work, and forcing making it work will not generate clean code and I think like a future dev in some year will hate me by doing that hehe |
| 17:35:56 | OlsroFR | I don't know how to do pinning with that kind of buffers, and am afraid of creating bugs by trying to make code without understanding the implications. If there's 2 or 3 lines to modify about that, could you do it and push a patch ? |
| 17:38:40 | | Join Moriar [0] (~moriar@107-200-193-159.lightspeed.stlsmo.sbcglobal.net) |
| 17:44:42 | | Join kugel__ [0] (~kugel@ip4d146a3a.dynamic.kabel-deutschland.de) |
| 17:55:54 | | Quit OlsroFR (Ping timeout: 246 seconds) |
| 18:00 |
| 18:02:21 | | Join dconrad [0] (~dconrad@152.117.104.217) |
| 18:07:14 | | Quit jacobk (Ping timeout: 248 seconds) |
| 18:08:09 | | Quit kugel__ (Quit: Lost terminal) |
| 18:08:34 | | Join kugel_ [0] (~kugel@ip4d146a3a.dynamic.kabel-deutschland.de) |
| 18:10:11 | | Quit kugel_ (Client Quit) |
| 18:10:40 | | Join kugel [0] (~kugel@77.20.106.58) |
| 18:18:25 | | Quit cstine (Quit: The Lounge - https://thelounge.chat) |
| 18:18:56 | | Join cstine [0] (~cstine@150.136.136.191) |
| 18:45:29 | dconrad | One final question on g#5877, regarding how best to add the v3 entry to tools/configure - is using the same target ID and modelname, etc. going to cause an issue? |
| 18:45:32 | rb-bluebot | Gerrit review #5877 at https://gerrit.rockbox.org/r/c/rockbox/+/5877 : ErosQNative: Add v3 LCD support, conditional on bootloader by Dana Conrad |
| 18:45:45 | dconrad | Other than that I'm pretty happy with it I think |
| 18:58:15 | | Join massiveH [0] (~massiveH@2600:4040:a982:dc00:9040:6246:5618:3f8d) |
| 19:00 |
| 19:03:09 | | Quit dconrad (Remote host closed the connection) |
| 19:19:14 | | Quit davisr (Remote host closed the connection) |
| 19:24:48 | *** | Saving seen data "./dancer.seen" |
| 19:35:12 | | Join dconrad [0] (~dconrad@152.117.104.217) |
| 19:35:30 | | Quit Nezumi-sama (Ping timeout: 252 seconds) |
| 19:41:45 | | Join Nezumi-sama [0] (~narf@syn-067-053-148-069.biz.spectrum.com) |
| 20:00 |
| 20:05:10 | | Join OlsroFR [0] (~OlsroFR@user/OlsroFR) |
| 20:37:47 | _bilgus | dconrad uncharted territory for me there we might have to f around and find out together |
| 20:38:25 | _bilgus | OlsroFR, very well |
| 20:39:19 | OlsroFR | Yeah, I was very positively surprised to get so much feedbacks and by another person that came give some interest in this subject. I am currently working on improving the things with the feedbacks :) |
| 20:42:28 | | Quit dconrad (Remote host closed the connection) |
| 20:47:16 | _bilgus | re core_alloc I think you'd be fine asking for the plugin buffer in this context |
| 20:48:47 | _bilgus | void* plugin_get_buffer(size_t *buffer_size); |
| 20:49:15 | OlsroFR | are the plugins using their own buffer ? |
| 20:49:28 | OlsroFR | if yes, what is its space ? |
| 20:49:34 | _bilgus | shouldn't be while in the tagtree |
| 20:50:08 | _bilgus | well you get back a buffer size and the TSRs have a function to reserve what they want |
| 20:50:11 | OlsroFR | what about plugins like the battery plugin which is running all time in the background ? |
| 20:50:18 | OlsroFR | batterybench* |
| 20:50:45 | _bilgus | ATM all TSRs (terminate and stay resident) have a sensible limit |
| 20:51:11 | _bilgus | but be aware someone in the future might alloc all so you still need a way to abort |
| 20:51:53 | _bilgus | I'd say the min you'd get is probably 100k |
| 20:52:44 | OlsroFR | right now I handle errors only at alloc time, which should be enough |
| 20:52:54 | _bilgus | yes same idea |
| 20:52:56 | OlsroFR | When I cannot alloc, the program do not crash, it even warns the user |
| 20:53:21 | OlsroFR | and playlist is then created but without full random, by using the old method of simple iteration |
| 20:53:39 | _bilgus | and in that case you can expand or contract to fill what you get |
| 20:54:39 | OlsroFR | what is great if you looked on my code is that I never have to shrink or grow. The code do all the calculation then specify just one time the size because the code knows the size of each segments |
| 20:55:05 | OlsroFR | and the size of each segment is defined now to be 1024 at maximum |
| 20:55:15 | OlsroFR | so 1024 means 1024 bits which is just 1k |
| 20:55:25 | _bilgus | what ever you choose.. |
| 20:55:45 | OlsroFR | so I will crash and have problem only if someone really use all in the buffer |
| 20:56:06 | OlsroFR | the program* I meant, sorry, I am starting to try to merge with my own code haha |
| 21:00 |
| 21:23:50 | | Quit rogeliodh91 (Quit: The Lounge - https://thelounge.chat) |
| 21:24:11 | | Join rogeliodh91 [0] (~rogeliodh@rogeliodh.dev) |
| 21:24:49 | *** | Saving seen data "./dancer.seen" |
| 21:33:46 | | Join dconrad [0] (~dconrad@152.117.104.217) |
| 21:34:10 | OlsroFR | _bilgus I feel like it's not a good idea to use the plugin buffer. It does not have any kind of safety protection. If someone ever use the buffer in the same time of me (during yield operation or what else), it will create very strange issues |
| 21:35:20 | OlsroFR | I will continue to use core buffer but am working on some modifications to apply the knowledge that dconrad shared with me |
| 21:40:11 | OlsroFR | Mmm he told me that it interrupts any audio playback to allocate things with this. While you are building a playlist, you don't want this to happen to continue to enjoy the music during the 10/15 seconds required to build the playlist with random. |
| 21:41:05 | OlsroFR | I guess the easiest way to get around all of this is just to do a static allocation, it is relevant and that new system is disabled on low-ram players anyway so 1K more won't be an issue for anyone |
| 22:00 |
| 22:08:49 | OlsroFR | https://gerrit.rockbox.org/r/c/rockbox/+/5911 new patch pushed |
| 22:08:50 | OlsroFR | Changelog : |
| 22:08:50 | OlsroFR | - Back to static allocation, which allowed also to reduce the code complexity |
| 22:08:51 | DBUG | Enqueued KICK OlsroFR |
| 22:08:51 | OlsroFR | - Update translations (shorter texts) |
| 22:08:51 | OlsroFR | - Shuffling the array with the algorithm suggested by dconrad to get a predictable complexity on this process |
| 22:10:30 | dconrad | I think you may be confusing me with somebody else haha |
| 22:11:22 | OlsroFR | Oh I am sorry xD |
| 22:14:17 | dconrad | :-P |
| 22:18:15 | OlsroFR | good night folks |
| 22:18:16 | | Quit OlsroFR (Quit: Connection closed) |
| 22:41:34 | | Quit dconrad (Remote host closed the connection) |
| 22:58:16 | | Join dconrad [0] (~dconrad@152.117.104.217) |
| 23:00 |
| 23:02:45 | | Quit dconrad (Ping timeout: 248 seconds) |
| 23:19:50 | | Join dconrad [0] (~dconrad@152.117.104.217) |
| 23:24:50 | *** | Saving seen data "./dancer.seen" |
| 23:45:28 | | Join dconrad_ [0] (~dconrad@152.117.104.217) |
| 23:45:29 | | Quit dconrad (Read error: Connection reset by peer) |
| 23:55:45 | | Quit Moriar (Ping timeout: 260 seconds) |
icon identifies that the person is a core developer (has commit access).