Rockbox Technical Forums
Support and General Use => User Interface and Voice => Topic started by: Desertway on May 17, 2020, 02:28:13 PM
-
I hope I'm in the right place for this question. I'm new to Rockbox and looking for a way to view the entire embedded comment field for an MP3. I have seen some themes that incorporate the tag, but it is truncated. Can it be viewed in a way other than the WPS? Thanks very much.
-
Rockbox internally probably truncates it due to memory constraints
you could parse it yourself in Lua check out the hex viewer example
-
Thanks very much.
-
Ideally some smart coding-capable person would be able to modify the main code so as to add an additional function to retrieve the second 230 characters of the comment (the existing function only gives you the first 230 characters). I could then rewrite my theme below so as to report the full comment.
http://themes.rockbox.org/index.php?themeid=2529&target=ipod6g
-
Ideally some smart coding-capable person would be able to modify the main code so as to add an additional function to retrieve the second 230 characters of the comment (the existing function only gives you the first 230 characters). I could then rewrite my theme below so as to report the full comment.
http://themes.rockbox.org/index.php?themeid=2529&target=ipod6g
I was looking at your theme. It would be perfect as I primarily listen to podcasts, and they have a lot of notes. A change to expand those limitations in the code would be great. Do you think there is enough interest at this point?
-
The ability to see the full comment, neatly formatted, is something that the Original firmware has that Rockbox lacks - and as you say it struck me as particularly important for podcasts. But adding a function to retrieve characters 231-460, would surely not be that complicated, because one could just duplicate with slight changes the existing code that gets the first 230 characters, and give it a different identifier in the WPS engine to the existing comment field (maybe iC2 or iCb rather than iC?). Unfortunately I am not skilled enough to be able to figure out the existing code-base and fear breaking something if I try and meddle with it, so was hoping someone more confident about it might do it for me!
-
ID3V2_MAX_ITEM_SIZE is defined to be 240 if MEMORYSIZE > 2 (MB) in /lib/rbcodec/metadata/metadata.h
In theory you could simply increase that value and possibly the buffer size for "very" large memory sizes (the iPod video has 64MB, for example) and have it just work.
#if MEMORYSIZE > 2
#define ID3V2_BUF_SIZE 900
#define ID3V2_MAX_ITEM_SIZE 240
#else
#define ID3V2_BUF_SIZE 300
#define ID3V2_MAX_ITEM_SIZE 90
#endif
The constraint was only introduced in dedde47 (https://github.com/Rockbox/rockbox/commit/dedde474248d1e75396b9e90e141284901e588f1) and then again adjusted in 37a9a20 (https://github.com/Rockbox/rockbox/commit/37a9a200b2ab93f02b8085e78aac91b8ff68419c)
-
It's probably safe to bump this up for targets with >8MB RAM.
-
If I can get my "development environment" working again (on my linux box with the dodgy hard-drive!) I will try compiling a version with that number boosted a bit (where MEMORYSIZE>8MB), and see if it works/makes a difference.
(Not all ipod videos have 64MB, the 30gb models I believe have 32MB, but it sounds as if the string length was set very conservatively to be safe for the lower-end targets?)
I notice the change chris_s found says " Limit the size of each ID3 metadata item to avoid that the metadata buffer is filled by single items"
Does that mean there's a specific, defined "metadata buffer" that would need to be increased in size?
-
(Not all ipod videos have 64MB, the 30gb models I believe have 32MB, but it sounds as if the string length was set very conservatively to be safe for the lower-end targets?)
Yep, a _lot_ of rockbox is the way it is in order to keep the memory footprint as small (and runtime as deterministic) as possible.
-
(Not all ipod videos have 64MB, the 30gb models I believe have 32MB
I guess you're right. Was just going by what Rockbox had defined for the model (https://github.com/Rockbox/rockbox/blob/ca4d63d4d903e3de356afb8d129ae61c660ff9b4/tools/configure#L1840).
I notice the change chris_s found says " Limit the size of each ID3 metadata item to avoid that the metadata buffer is filled by single items"
Does that mean there's a specific, defined "metadata buffer" that would need to be increased in size?
ID3V2_BUF_SIZE is currently 900 bytes (for memory sizes > 2 MB) and needs to fit all of the tags. So you may need to increase that as well.
-
If I can get my "development environment" working again (on my linux box with the dodgy hard-drive!) I will try compiling a version with that number boosted a bit (where MEMORYSIZE>8MB), and see if it works/makes a difference.
It's been changed in the latest daily build if you want to check it out.
-
Thanks - good work, provisionally it seems to be working.
Modified my theme to make use of it and it _seems_to work. Having an unrelated problem in demonstrating that it works, as the simulator is not up to date so I can only test it on the ipod itself - and for some reason I can't seem to get the 'take a screenshot' feature to actually do what it's supposed to (and take a screenshot of the comment being displayed). But it seems to be working as it's supposed to, with no side-effects noticed so far.
OK, got a screenshot (still slightly truncated for this one, but a lot better than before)
(the full comment is:
"In the waning days of China's Qing Empire, a riot broke out in Changsha, the capital of Hunan Province. After two years of flooding, a starving woman had drowned herself in desperation after an unscrupulous merchant refused to sell her food at a price she could afford. Three days of rioting followed during which symbols of Qing power were destroyed by an angry mob, which then turned its sights on Changsha's Western compound. Historians have long assumed the mob was controlled by the landed gentry, but as nearly every dictator knows, a crowd has a mind of its own. James Joshua Hudson, Visiting Assistant Professor of History at Knox College, describes the riots and some surprising finds he made conducting fieldwork in Hunan that offer a glimpse into the deeply layered tensions on the eve of the downfall of the Qing dynasty." - so I guess that's about 230 characters short, still...but I guess it might be too risky to increase the space allocated any further? Will test this version for a while and see if there's any issue with memory capacity)
-
The patch I proposed changed the total id3v2 buffer size to 1800 bytes and the max size for individual items to 500 bytes. There wasn't any deep thought behind choosing those exact values though. I'm not aware of any reason you couldn't increase that even further, if necessary, e.g. change the max item size to 800 bytes. You could probably leave the total buffer size unchanged in that case, since the likelihood of another tag being that long and filling the buffer, seems low. Idk, someone tell me if they disagree, bilgus, speachy?
-
where does it end 1000 char comments or 1000 megs?
IDK after that maybe make a chunking display in the theme engine?
or just call it good enough for most cases
-
After a little more thought and not looking too closely at the current code :P
I think we could just use buflib to manage the static buffer and then you just need
to worry about which buffers are higher priority
but if you are going that far you might as well take it all the way and just make it buflib allocated memory too
I really dislike the idea of extend tags for comments
-
where does it end 1000 char comments or 1000 megs?
IDK after that maybe make a chunking display in the theme engine?
or just call it good enough for most cases
My impression is most podcasts have 300-1000 character comments. Maybe there's a way to write a script to look at a lot of them and find what the range/average is (but at the moment I can only copy the comments to a text editor and do a character count, which is time-consuming. Typical BBC podcasts I've looked at seem to have comments 700-1500 characters long). The current situation thanks to Chris_S seems like an acceptable compromise, though I don't know how tight memory is for targets with 32 or 64mb and whether there is a downside to going any higher.
Not sure what a 'chunking display' means, unless it means chopping longer comment fields up into substrings? I don't understand the details of how tag reading works in Rockbox, but would that still not need a longer string capacity to get access to the full comment field in the first place?
When you say you dislike 'extend tags for comments', do you mean 'extending tags for comments' or 'extended tags for comments', and do you mean 'within rockbox' or just that you dislike podcasters using comment tags in the way they seem to do?
-
I mean extended tags like having Cmt0 Cmt1 Cmt2
I feel like that is a burden all the way around when it should just work
The other issue I have with just increasing the buffer is that its static and gone forever when it could be either managed static or totally allocated from the start
..its not a longterm solution