ZVISION: Experimental sound support for the high-resolution ZGI movies (DVD version) #1356

eriktorbjorn · 2018-10-17T15:31:22Z

This has been sitting on my hard drive for quite some time now...

Back before clone2727 left the ScummVM project, he and I were trying to get the sound working in the high-resolution ZGI movies. When he left, there was still major problems with the audio/video sync and the project fell by the wayside. When GOG released the DVD version, this became a lot more relevant so I made an effort to update the code and received permission from clone2727 to release it.

There are some caveats:

It uses liba52 for sound support. I hope this is portable enough for our purposes.
I've only actually tested the three opening movies (i.e. up to and including the "Propaganda on Parade" movie.)
The movies use higher resolution than the game itself, but the window remains the same size so movies may be downscaled. This is far from ideal, and it wasn't the case when work began on this.
It only uses the video timestamps for sync, not the audio ones. This is hopefully good enough. I had a hell of a time getting it to work at all, and ended up having to buffer packets so that they could be handled out of order. Otherwise, it would get out of sync no matter what I did.

There may be other things to consider, but the truth is that I don't have the time nor the knowledge to get it any better than it is now. Hopefully someone else will be able to work out the last kinks. I hope I rebased the code properly. Git is an intelligence test that I keep failing...

This is for an experiment aiming to decode audio for the hi-res videos in Zork: Grand Inquisitor. I don't know if we want to add a dependency on liba52, but the license should allow us to include the code verbatim, if that's preferrable.

At the moment, this produces nothing but misery in the form of Valgrind warnings and horrible noise.

This collects the whole frame before trying to decode it. It's still now working right, but it's way better than it was before.

This is used for some logo animations at the start of the game. I don't know if it's used for anything else, but probably not.

Like most things that make this branch actually work, this comes from clone2727.

This code comes from clone2727's now defunct (?) ac3 branch.

This code comes from clone2727's now defunct (?) ac3 branch, with some minor compile fixes. This represents the latest version of the stalled AC-3 decoder work for Zork: Grand Inquisitor. Note, however, that I have not yet asked for clone2727's permission to use this. I'm just experimenting.

From what I understand, this has something to do with the image being either made up from two or three parts. When it's made from three parts, the frame should be displayed for half again as long as normal. This makes the speed of the Zork: Grand Inquisitor video look about right to me. It's still out of sync, but it doesn't seem to get *more* out of sync as the video progresses.

This is another attempts at improving the audio/video sync in the MPEG-PS decoder. Unfortunately, the audio probably also needs to be synced to its pts timestamps...

I think it makes things easier to read, and I have some ideas that I want to try which should be easier this way...

We weren't doing anything with it anyway. And I'm not sure it's even available in the ZGI videos.

Sometimes (only at the very start of a movie?) there will be a video packet that has a PTS but no frame to display. Save that PTS and use it for the next frame. This doesn't actually improve anything, as far as I can tell, but feels right.

In all my attempts to get the audio and video to sync up in the ZGI videos, there have always been 9-10 frames of video before the audio even starts, even though the audio is timestamped to start before. This attempts to fix that by prioritizing sending audio packets to the decoder in a timely fashion. I do not know if this is the correct way of doing this, and there are still some things that need to be fixed. But pragmatically, it does procude significantly better sync, so...

eriktorbjorn · 2018-10-21T06:51:10Z

One thing I forgot to mention is that there seems to be some sort of interlacing going on in some parts of the "Propaganda on Parade" intro video. It doesn't really have anything to do with this pull request, but since I don't see it if I play the video in a media player it's something that should be looked into, I guess.

I had hoped Wikipedia's articles on "Telecine" or "Three-two pull down" would give me ideas, but nope. No clue what to do about it.

bluegr · 2018-10-21T17:46:05Z

Nice work from everyone involved!
liba52 looks to be portable (they even say so on their website)

Out of curiosity, why can't we change the screen resolution when a video is played, and we resort to downscaling?

angstsmurf · 2018-10-21T18:00:27Z

Wouldn't the best solution be to always run in the highest resolution needed and upscale the non-video parts?

eriktorbjorn · 2018-10-22T04:49:47Z

Out of curiosity, why can't we change the screen resolution when a video is played, and we resort to downscaling?

As I said (though perhaps not clearly enough), when work started on this the backend would change the window size when the videos were playing. But at some point, probably related to that you can now resize the game window at will, even with the SDL renderer and not just the OpenGL renderer, this was changed. There's this comment in backends/graphics/sdl/sdl-graphics.cpp:

// We only update the actual window when flags change (which usually means
// fullscreen mode is entered/exited), when updates are forced so that we
// do not reset the window size whenever a game makes a call to change the
// size or pixel format of the internal game surface (since a user may have
// resized the game window), or when the launcher is visible (since a user
// may change the scaler, which should reset the window size)
if (!_window->getSDLWindow() || _lastFlags != flags || _overlayVisible || _allowWindowSizeReset) {

So it probably has something to do with _allowWindowSizeReset, but I'm not familiar with the inner workings on the backend.

criezy · 2018-10-22T07:10:08Z

It should indeed run at the highest resolution and upscaled some parts rather than downscale others. I am guessing the zvision engine does not call initGraphicsModes when it starts with the list of resolutions it uses. Doing so should allow the backend to choose the best window size.

eriktorbjorn · 2018-10-22T12:24:42Z

Wouldn't the best solution be to always run in the highest resolution
needed and upscale the non-video parts?

Maybe, but I thought the game graphics looked ugly when I resized the window to 800x600 pixels. And you'd still have the problem where subtitles would look smaller during cutscenes, wouldn't you? So personally I would prefer the window to change size as needed during gameplay like it used to, at least as long as the window hasn't been manually resized. But there's probably no one-size-fits-all solution here...

criezy · 2018-10-22T13:08:43Z

The engines that can change resolution during game play should now call initGraphicsModes when starting to indicate all the resolutions that it will use. It is then up to the backend to decide what to do with this information. The SDL backends sets the window size to the highest of the provided resolutions and keeps it at this size all the while the game is running. You can see a discussion in PR #1007 that added the feature. They is indeed no magical behaviour that fits all the cases and all the tastes and we had to settle for what we though was best.

Also at the time the change was made this indeed meant that the parts in lower resolution would be upscaled to fit the bigger window. Since then a stretch mode has been added however, which means that while the window will remain at the higher resolution we can instruct the SDL backend to not scale the display and instead use black borders for the parts at a lower resolution (by select the Center stretch mode in the options).

eriktorbjorn · 2018-10-22T16:21:56Z

Ok, so if I understand correctly commit 9fb8e87 is the correct way of handling the different resolutions. And commit a945825 is hopefully the right way of allowing me to set the stretch mode per game, even when using the launcher dialog.

digitall · 2018-10-22T16:31:57Z

engines/engine.cpp

@@ -210,6 +210,9 @@ void initCommonGFX() {

 		if (gameDomain->contains("filtering"))
 			g_system->setFeatureState(OSystem::kFeatureFilteringMode, ConfMan.getBool("filtering"));
+
+		if (gameDomain->contains("stretch_mode"))
+			g_system->setStretchMode(ConfMan.get("stretch_mode").c_str());


It would be good if this could be merged seperately from the rest of this pull request, since it is a good fix anyway and I think this may be the fix for a problem reported on the forum a few days back:
http://forums.scummvm.org/viewtopic.php?t=14732

Oops! This is an oversight from myself when I added the feature. But it is indeed unrelated to this PR. I will commit that change this evening when I am back home (unless somebody does it before me).

@criezy : Happy for you to commit it later. Though @eriktorbjorn will have to rebase the PR at that point to remove this commit...

criezy · 2018-10-22T17:04:35Z

Commit 9fb8e87 is actually not the correct way to handle the game having multiple resolutions, although it probably works with the SDL backend.

The idea is to do something like this when starting the engine (before the first call to initGraphics):

Graphics::ModeList modes;
modes.push_back(Graphics::Mode(WINDOW_WIDTH, WINDOW_HEIGHT));
if (getGameId() == GID_GRANDINQUISITOR && (getFeatures() & GF_DVD))
	modes.push_back(Graphics::Mode(HIRES_WINDOW_WIDTH, HIRES_WINDOW_HEIGHT));
initGraphicsModes(modes);

Note: since initScreen() is called not only from ZVision::initialize() but also after playing the video in ActionStreamVideo::execute(), it is probably best to call initGraphicsModes() from ZVision::initialize() before the call to initScreen() rather than to do it in initScreen() itself.

criezy · 2018-10-22T22:55:00Z

Good work on this feature!
I finally installed my copy of ZGI from GoG and it was nice to see those high resolution videos with sound :-)

Before merging this you will need to drop the last commit (because I have already pushed it to trunk) and amend the previous one as per my previous comment (to use initGraphicsModes() before calling initScreen() in ZVision::initialize()). Actually I would suggest to also check if we are using the high-res videos or not, so the code would be:

Graphics::ModeList modes;
modes.push_back(Graphics::Mode(WINDOW_WIDTH, WINDOW_HEIGHT));
if (getGameId() == GID_GRANDINQUISITOR && (getFeatures() & GF_DVD) && (!ConfMan.hasKey("mpegmovies") || ConfMan.getBool("mpegmovies")))
        modes.push_back(Graphics::Mode(HIRES_WINDOW_WIDTH, HIRES_WINDOW_HEIGHT));
initGraphicsModes(modes);

I have made the changes locally to test this PR, and I can force push if you want.

eriktorbjorn · 2018-10-23T05:08:54Z

I have made the changes locally to test this PR, and I can force push if you want.

If you can do that, that'd be great because I'm short on time right now. There's also a typo ("procude" instead of "produce") in the commit message for commit b326ff6, if that can be fixed.

By the way, if the hi-res graphics mode is only specified if "mpegmovies" is set, what happens if you enable it after the game has begun?

criezy · 2018-10-23T09:09:54Z

You can enable the high-res movies while the game is running? I missed that. If you do that then the window would remain at its small size (unless resized by the user of playing in fullscreen) and the high-res videos would be downscaled. Then indeed it might be better to not check if they are enabled or not when starting the engine and always add the high-res size to the used graphics modes.

eriktorbjorn · 2018-10-23T10:26:06Z

You can enable the high-res movies while the game is running? I missed that.

I think so. I can't check at this very moment, but can't you enable/disable them from the game's own options dialog?

bluegr · 2018-10-23T16:21:45Z

You can enable the high-res movies while the game is running? I missed that.

I think so. I can't check at this very moment, but can't you enable/disable them from the game's own options dialog?

I've just checked and no, you can't toggle the hi-res movies while the game is running. You can toggle if movie upscaling will occur via line skipping, or via doubling pixels.

eriktorbjorn · 2018-10-23T16:46:16Z

I've just checked and no, you can't toggle the hi-res movies while the game is running. You can
toggle if movie upscaling will occur via line skipping, or via doubling pixels.

My copy of the game has a switch labelled "MPEG" in the lower right corner of the options dialog that switches between the hi-res and lo-res movies.

This allows the backend to show the hi-res videos at full resolution, rather than scaling them down.

criezy

I also checked and I can confirm what @eriktorbjorn wrote. At least with the version from GoG there is the possibility to toggle on/off the use of the high-res videos in the in-game Preferences dialog. I have thus reverted to the code that always adds the high-res size to the graphics modes for the DVD version.

I have forced pushed the commit to properly handle the multiple resolutions. I am now happy with this PR and I think it can be merged. I will leave a few more days for review and if there is no objection I will merge the PR this weekend.

bluegr · 2018-10-24T19:35:45Z

Ah it has been awhile. I remember now: this option needs to be set in the script manager, otherwise it is disabled and not shown in the preferences dialog. I specifically added code to hide it when libmpeg2 has not been included in the build, and it wasn't included in my local build, which is why I didn't see the option :)

The code in question is in zvision.cpp:222:

#ifndef USE_MPEG2
	// libmpeg2 not loaded, disable the MPEG2 movies option
	_scriptManager->setStateValue(StateKey_MPEGMovies, 2);
#endif

eriktorbjorn · 2018-10-28T07:17:22Z

The code in question is in zvision.cpp:222:

#ifndef USE_MPEG2
	// libmpeg2 not loaded, disable the MPEG2 movies option
	_scriptManager->setStateValue(StateKey_MPEGMovies, 2);
#endif

Ah, so I guess this should be "if !defined(USE_MPEG2) || !defined(USE_A52)" now? (I'm not sure if there's any such condition for having the option in ScummVM's engine options though.)

eriktorbjorn · 2018-10-28T11:41:13Z

And perhaps the hi-resolution mode should only be set if ScummVM is compiled with the necessary libraries to play the hi-resolution movies.

bluegr · 2018-10-28T12:22:39Z

@eriktorbjorn: yes, and yes to both of your questions :P

eriktorbjorn · 2018-10-29T05:50:47Z

@eriktorbjorn: yes, and yes to both of your questions :P

Hmm... Well, unfortunately I'm still short on time and probably will be for the rest of the week. I'm uncertain how the aforementioned force push criezy made affects things if I commit to my branch, so I won't do anything at this moment. I see there are a couple of other places where it only checked for USE_MPEG that should probably include USE_A52 as well.

I don't like having such a minor thing hold this up, so if anyone else can fix it I'd be grateful. Again. :-)

criezy · 2018-10-31T21:52:08Z

I have pushed a commit to check on USE_A52 everywhere where it was checking USE_MPEG and also added that change when initializing the graphics mode.

This seems to work well. The only place where the option is still visible when either libmpeg2 or liba52 is not used is in the engine options. This one is more problematic as the flag is set in the config file when adding the game to ScummVM.

…bled

criezy · 2018-10-31T22:35:06Z

I forgot the engine options in the config are updated when starting the engine. So this is not as bad as I though and I have added a commit to remove the mpeg engine option from the detection table when libmpeg2 or liba52 is not used.

This is not perfect as we now get the following behaviour:

Add game with version of ScummVM that does not support playing the mpeg movies
The option is not visible in the game edit dialog.
Now upgrade to a ScummVM version that supports playing the mpeg movies
The option is still not visible in the game edit dialog.
Start the game, then return to launcher.
The option is now visible in the game edit dialog.

But I think this is better than always having the option visible.

Torbjörn Andersson added 17 commits October 17, 2018 17:13

CONFIGURE: Add detection of liba52

97e5264

This is for an experiment aiming to decode audio for the hi-res videos in Zork: Grand Inquisitor. I don't know if we want to add a dependency on liba52, but the license should allow us to include the code verbatim, if that's preferrable.

VIDEO: Use liba52 to decode audio. This is still laughably broken

b3aec1a

At the moment, this produces nothing but misery in the form of Valgrind warnings and horrible noise.

ZVISION: Enable hi-res videos for testing purposes

2796cca

VIDEO: Committed fixes from clone2727

63f5b1f

This collects the whole frame before trying to decode it. It's still now working right, but it's way better than it was before.

VIDEO: After figuring out sample rate, rewind the AC-3 stream

43a5cce

ZVISION: Play hi-res videos, even if they have no low-res version

06df74b

This is used for some logo animations at the start of the game. I don't know if it's used for anything else, but probably not.

VIDEO: Skip DVD code. The sound plays, but audio/video sync is off

9d36382

Like most things that make this branch actually work, this comes from clone2727.

VIDEO: Some minor cleanups to the AC-3 decoder

9e04d95

AUDIO: Add a separate AC-3 decoder

840dff2

This code comes from clone2727's now defunct (?) ac3 branch.

VIDEO: Synchronize video to pts timestamps

04b7e93

This is another attempts at improving the audio/video sync in the MPEG-PS decoder. Unfortunately, the audio probably also needs to be synced to its pts timestamps...

VIDEO: Make the video pts sync easier to understand

9cba3c2

VIDEO: Moved MPEGStream creation out of readNextPacket()

5ff0017

I think it makes things easier to read, and I have some ideas that I want to try which should be easier this way...

VIDEO: Removed _psmESType

ffa0315

We weren't doing anything with it anyway. And I'm not sure it's even available in the ZGI videos.

VIDEO: Use all video PTS's

2d9e928

Sometimes (only at the very start of a movie?) there will be a video packet that has a PTS but no frame to display. Save that PTS and use it for the next frame. This doesn't actually improve anything, as far as I can tell, but feels right.

digitall reviewed Oct 22, 2018

View reviewed changes

ZVISION: Set 800x600 mode for the DVD version of ZGI

e0a2541

This allows the backend to show the hi-res videos at full resolution, rather than scaling them down.

criezy force-pushed the ac3 branch from a945825 to e0a2541 Compare October 23, 2018 19:51

criezy approved these changes Oct 23, 2018

View reviewed changes

ZVISION: Disable MPEG movies when liba52 is not enabled

3483d4a

ZVISION: Only add mpeg video engine options when mpeg and a52 are ena…

a4018e5

…bled

criezy merged commit e4ff192 into scummvm:master Nov 4, 2018

eriktorbjorn deleted the ac3 branch February 8, 2022 09:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ZVISION: Experimental sound support for the high-resolution ZGI movies (DVD version) #1356

ZVISION: Experimental sound support for the high-resolution ZGI movies (DVD version) #1356

eriktorbjorn commented Oct 17, 2018

eriktorbjorn commented Oct 21, 2018

bluegr commented Oct 21, 2018

angstsmurf commented Oct 21, 2018

eriktorbjorn commented Oct 22, 2018

criezy commented Oct 22, 2018

eriktorbjorn commented Oct 22, 2018

criezy commented Oct 22, 2018

eriktorbjorn commented Oct 22, 2018

digitall Oct 22, 2018

criezy Oct 22, 2018

digitall Oct 22, 2018

criezy commented Oct 22, 2018 •

edited

criezy commented Oct 22, 2018

eriktorbjorn commented Oct 23, 2018

criezy commented Oct 23, 2018

eriktorbjorn commented Oct 23, 2018

bluegr commented Oct 23, 2018 •

edited

eriktorbjorn commented Oct 23, 2018

criezy left a comment

bluegr commented Oct 24, 2018

eriktorbjorn commented Oct 28, 2018

eriktorbjorn commented Oct 28, 2018

bluegr commented Oct 28, 2018

eriktorbjorn commented Oct 29, 2018 •

edited

criezy commented Oct 31, 2018

criezy commented Oct 31, 2018

ZVISION: Experimental sound support for the high-resolution ZGI movies (DVD version) #1356

ZVISION: Experimental sound support for the high-resolution ZGI movies (DVD version) #1356

Conversation

eriktorbjorn commented Oct 17, 2018

eriktorbjorn commented Oct 21, 2018

bluegr commented Oct 21, 2018

angstsmurf commented Oct 21, 2018

eriktorbjorn commented Oct 22, 2018

criezy commented Oct 22, 2018

eriktorbjorn commented Oct 22, 2018

criezy commented Oct 22, 2018

eriktorbjorn commented Oct 22, 2018

digitall Oct 22, 2018

Choose a reason for hiding this comment

criezy Oct 22, 2018

Choose a reason for hiding this comment

digitall Oct 22, 2018

Choose a reason for hiding this comment

criezy commented Oct 22, 2018 • edited

criezy commented Oct 22, 2018

eriktorbjorn commented Oct 23, 2018

criezy commented Oct 23, 2018

eriktorbjorn commented Oct 23, 2018

bluegr commented Oct 23, 2018 • edited

eriktorbjorn commented Oct 23, 2018

criezy left a comment

Choose a reason for hiding this comment

bluegr commented Oct 24, 2018

eriktorbjorn commented Oct 28, 2018

eriktorbjorn commented Oct 28, 2018

bluegr commented Oct 28, 2018

eriktorbjorn commented Oct 29, 2018 • edited

criezy commented Oct 31, 2018

criezy commented Oct 31, 2018

criezy commented Oct 22, 2018 •

edited

bluegr commented Oct 23, 2018 •

edited

eriktorbjorn commented Oct 29, 2018 •

edited