Opened 18 years ago

Closed 18 years ago

Last modified 19 months ago

#8024 closed patch

new smush player

Reported by: SF/xtrochu Owned by:
Priority: normal Component: Engine: SCUMM
Keywords: Cc:


This patch contains a whole new smush player.

It is based on the smush player source code that I made available a few weeks ago.

The archive contains a diff file and a complete directory. Only the makefile.common has been modified, so only ports compiling with make will be affected.

The new smush player is disabled by default. there is a #define USE_OLD_SMUSH_PLAYER in script_v2.cpp that can be removed to try the new one. Notice that I only tested under Mingw/Win32 with GCC 3.1, so I don't know if it works under other ports (I suppose it will work also under linux too). There may be a few endian or memory alignment issues, but I've taken extreme care to prevent this to happen.

The new smush player offers a far better support for Full Throttle (stereo sounds, subtitles) and the at least the same support for The Dig. Memory requirement should also be a little less, but don't expect better performance.


Ticket imported from: #598939. Ticket imported from: patches/129.

Attachments (3)

smush.tar.bz2 (30.3 KB ) - added by SF/xtrochu 18 years ago.
weird.png (103.1 KB ) - added by fingolfin 18 years ago.
Weird The Dig screenshot
weird2.png (145.4 KB ) - added by fingolfin 18 years ago.
Weird The Dig screenshot #2

Download all attachments as: .zip

Change History (17)

by SF/xtrochu, 18 years ago

Attachment: smush.tar.bz2 added

comment:1 by fingolfin, 18 years ago

Argh! These source files have mixed line endings! Please, fix the files to have either only Windows or Unix style file endings (Unix ones prefered). I really don't feel like going through all the files and fixing them myself (though I guess a script could be written to do it).

comment:2 by fingolfin, 18 years ago

Some more things you might want to correct: * it's "Chunk" not "Chunck" * do *never* use names that start with double underscores (__foo), those are reserved for the compiler and the system. * this code is not following the ScummVM style conventions ( documentation.php?view=conventions) at all. Of course I realized that you wrote that code independtly originally, but I mention it anyway just in case :-) * there is a lot of #if 0 code, why not just remove that? * why is "min" and "max" used instead of "MIN" and "MAX" I wonder?

Please don't get me wrong, my comments are meant as constructive criticism, and not to diminish the hard work you put into this,

comment:3 by aquadran, 18 years ago

patch comited

comment:4 by aquadran, 18 years ago

Status: newclosed

comment:5 by fingolfin, 18 years ago

Status: closednew

comment:6 by fingolfin, 18 years ago

But only to scummvm, not scummvm-new

comment:7 by fingolfin, 18 years ago

OK so it's now in scummvm-new, too, and I also fixed the build system in scummvm-new (not in the old module, though). However:

* it still says "chunck" everywhere! Please rename this to "chunk", which is a proper english word unlike "chunck"

* it definitly has endian issues as it crashes on my big endian system. Hence for now it should *not* be the default.

* we need Xavier's permission when/if we have to change our license to something non-GPL. Xavier? Otherwise, we'll have to rip this out again.

comment:8 by fingolfin, 18 years ago

OK, thanks for all the work you do on this, aquadran :-) More complaints: the headers still use double underscores for some macros, which is not good (names starting with double underscores are reserverd by the system).

I am working on the endianess issues now. I got it working, sort off, but the decoding is only partly incorrect so far (i.e. looks odd). If I can't figure it out I'll attach a screenshot of The Dig, maybe one of you knows what's going on.

BTW I am getting lots of these (in The Dig): WARNING: Bundle: Compressed sound -1 invalid ()! WARNING: Decompression of bundle sound failed! Is that normal? Or maybe another endian issue?

comment:9 by fingolfin, 18 years ago

Oh another problem: I often can't abort smush sequences with neither ESC nor mouse clicks, this should be fixed.

by fingolfin, 18 years ago

Attachment: weird.png added

Weird The Dig screenshot

comment:10 by fingolfin, 18 years ago

I commited some endian fixes, I hope they don't break little endian systems.

I still get this (in The Dig) WARNING: unable to read text information for "/ Volumes/Data/Spiele_II/LEC_games/DIG/video/ sq1.san"! And also the bundle error (see below).

The decoded graphics look weird, attached two screen shots.

by fingolfin, 18 years ago

Attachment: weird2.png added

Weird The Dig screenshot #2

comment:11 by aquadran, 18 years ago

Bugs in loading text information and gfx errors, should be endian problem. About bundle music. When this happens ? All of time ? If yes this should be music.

comment:12 by SF/xtrochu, 18 years ago

chunk, right... sorry about that... My english was better at school, but even if I try to keep on training and learning, the lack of use of it can cause... problems like that.

There is indeed quite a lot of dead/unused code in it, as there was in the original decoder. This is the way I code, but that should not appear in commited code. I'll work on remove them. I was away all this week end, and wanted to submit the patch before it so that people could try it and find bugs. So there are indeed a few issues left. This is also why it is disabled by default.

The problem with Bundle music is most probably not related to the smush decoder, but may be an issue with a previous patch that I submitted in sound.cpp (between lines 52 and 55)

For text loading, I only search lower case filenames. That may b e the problem.

The weird screenshot look like an optimisation problem I had with the original code, but I thought I had fixed it. Compile decoder37.cpp without optimisation to check this. (It may also be endian issues).

For license issues, we already discussed of this with Ender and I leave all rights to this code to the scummvm project or it's maintainers.

Now, I would love to have a bigendian computer at home to check endianness, but I hope those bugs will be removed soon.

comment:13 by aquadran, 18 years ago

Status: newclosed

comment:14 by digitall, 19 months ago

Component: Engine: SCUMM
Note: See TracTickets for help on using tickets.