#8396 closed patch
smush_player performance patch
| Reported by: | SF/ole00 | Owned by: | |
|---|---|---|---|
| Priority: | normal | Component: | Engine: SCUMM |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
Hello,
I have an update for the scumm smush_player and chunk class. The old smush player was fairly excelent on fast computers wheere seek / and transfer times are very short. On the other hand if smush player was not fed properly right from the start (mostly happens on platforms where there is no read-ahead file caching, and on platforms where data transfer rate is slow - flash disks etc.) then the playback performace suffer. Another glitch concerned the time-compression on the start of the movie (first 2 or 3 seconds of the movie were missing - or played back at once with stacked voice streams) is also solved.
This fix/update adds read-ahead caching to the chunk class (I have created CachedFileChunk from the FileChunk) and slightly modifies the smush_player.
I'm sorry that the source is attached here, but I have no other easier way how to contribute.
Hope it will be usefull - if not, never mind :).
BTW. Why the chunk classes are passed as 'instance copy' (Chunk &b) in the smush/saud methods and not as the pointers (Chunk *b)? Maybe I'm wrong but this way the system creates instance copies of the chunks each time it is passed to another method which might be imho quite memory/time consuming...
Ole
The source code is attached bellow....
Ticket imported from: #1115053. Ticket imported from: patches/501.
Attachments (1)
Change History (7)
by , 19 years ago
| Attachment: | smush-speed.txt added |
|---|
comment:1 by , 19 years ago
Better learn some C++ ;-). The "&" operator here indicates a "reference", which is, to simplify the matter a bit, kind of a "hidden" pointer. So no copy is performed, and no performance lost.
comment:2 by , 19 years ago
Some remarks:
* Please submit patches in the "patch" format. Any CVS program should be able to generate such a patch for you, using its "diff" command. You don't tell us what OS you are using, so I won't go into details.
* You added a DW_LE_BE macro. Don't. Use SWAP_BYTES_32 or bypass getDword and call read() directly.
comment:3 by , 19 years ago
I'm sorry for the source format, but Im' still working with the old 0.7.0 version and due to the testing/researching of the code functionality I have added many tracing printfs which would make a total diff-mess... BTW. I found the bug in the submited CachedFileChunk that shows up when the main (parent) chunk is deleted earlier than it's child subchunk (occurs in the Insane repositions). That way the system frees the memory of the chunk and lately allocates the memory space to the new chunk, but the sub chunk still holds memory pointer now pointing to the new chunk (and during destruction sets the wrong cache data...). This behaviour is harmless for FileChunk, because theese are not sharing/updating the cache data.... Anyway I fixed it by applying id's for parent-child so child would update cache positions for true-parent chunk only. About the os I'm using the win32 os and working on the ps2 port which has no real os system. Generally most of the scummvm engines suppose that there is an underlying os that is caching the data files. That leads to some performance slowdowns (calling read of size 1 byte is not rare occasion :-) that I'm dealing right now. I plan to create some simple cache solution and put it nearby the file.cpp.
comment:4 by , 19 years ago
Well, w/o a patch made against current CVS, this patch won't make it even to the "testing" stage, so unless you provide one, I am afraid it has little chance of being accepted :-/
Regarding file caching: yeah, the File class is written around the assumption that the OS or the backend author provides some kind of sane implementation of fopen/fread/fclose. If that's a serious problem, we can think about making the File class customizable by backends. But this should be done in a clean and transparent fashion, well designed, and not as an after thought add-on cruft thingy... :-)
comment:5 by , 19 years ago
| Status: | new → closed |
|---|
comment:6 by , 5 years ago
| Component: | → Engine: SCUMM |
|---|
smush/chunk performance update source code