Opened 17 years ago

Closed 17 years ago

Last modified 11 months ago

#8024 closed patch

new smush player

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

Description

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.

Xavier

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

Attachments (3)

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

Download all attachments as: .zip

Change History (17)

by SF/xtrochu, 17 years ago

Attachment: smush.tar.bz2 added

comment:1 by fingolfin, 17 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, 17 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 (http://scummvm.sourceforge.net/
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, 17 years ago

patch comited

comment:4 by aquadran, 17 years ago

Status: newclosed

comment:5 by fingolfin, 17 years ago

Status: closednew

comment:6 by fingolfin, 17 years ago

But only to scummvm, not scummvm-new

comment:7 by fingolfin, 17 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, 17 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, 17 years ago

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

by fingolfin, 17 years ago

Attachment: weird.png added

Weird The Dig screenshot

comment:10 by fingolfin, 17 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, 17 years ago

Attachment: weird2.png added

Weird The Dig screenshot #2

comment:11 by aquadran, 17 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, 17 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, 17 years ago

Status: newclosed

comment:14 by digitall, 11 months ago

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