Skip to content

Conversation

@ccawley2011
Copy link
Member

This is an initial (untested) proof of concept for Trac #14173. Only King's Quest 6 is affected - since the SCI32 code doesn't appear to change modes when loading QuickTime videos, I'm assuming that the Mac version of King's Quest 7 only uses palette-based codecs.

@ccawley2011 ccawley2011 requested a review from sluicebox April 25, 2023 14:09
Copy link
Member

@sev- sev- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small optimization recommendation

@sev-
Copy link
Member

sev- commented May 14, 2023

@ccawley2011 do you have time fir optimizing it?

@mikrosk
Copy link
Contributor

mikrosk commented Nov 6, 2023

Btw, similar issue (running game in 8bpp while video requesting 16bpp) is present with other engines, too. I'm aware of prince and bbvs, I suppose there's no way to pull off a similar trick?

@ccawley2011
Copy link
Member Author

Btw, similar issue (running game in 8bpp while video requesting 16bpp) is present with other engines, too. I'm aware of prince and bbvs, I suppose there's no way to pull off a similar trick?

For QuickTime videos, it's possible to enable dithering for all videos. For AVI videos, it's possible to enable dithering for videos using Cinepak, but there isn't currently a generic fallback for other codecs. It would be nice to have this though, as well as faster code for dithering YUV411 videos for codecs like Indeo 3.

// The only argument is the string for the video
videoDecoder.reset(new Video::QuickTimeDecoder());

// Switch to 16bpp graphics for Cinepak
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why has this code been removed? The title of the PR says "Dither QuickTime videos in paletted screen modes", but with this change, we'll always use a paletted screen mode.

The whole point of this code is to actually switch to 16bpp, since the QT videos in question (the Mac KQ6 videos) are encoded in 16bpp. If we rip this code out, the screen will stay in paletted mode, which means that the video will ALWAYS be dithered.
Why? What's the point of making this video look always dithered (i.e. ugly)?

warning("This video requires >8bpp color to be displayed, but could not switch to RGB color mode");
return NULL_REG;
// Handle the pixel format for Cinepak videos
if (videoDecoder && videoDecoder->getPixelFormat() != screenFormat) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point, the screen will be in 8bpp, but the video will be encoded in 16bpp, so this logic will always dither the video, instead of switching the screen to 16bpp

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The format that the screen is using is dependent on whether or not the "RGB rendering" option is enabled or not. If it's off, the screen will be 8bpp. If it's on, the screen will either be 16bpp or 32bpp depending on which one the backend prefers.

Copy link
Member

@bluegr bluegr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR makes 16bpp videos dithered all the time. There should be a way of using dithering as a fallback, not as a default behavior

@bluegr
Copy link
Member

bluegr commented Nov 7, 2023

Some screenshots from the KQ6 Mac intro, courtesy of @sluicebox:

Before this PR:
image
image
image
image

With this PR:
image
image
image
image

@sluicebox
Copy link
Member

I don't know much about the low-level pixel formats, screen modes, dithering or cinepak. But as my screenshots show, this PR makes the KQ6-Mac movies go from good to bad under default settings, at least on Windows, and that's clear just from five seconds into the opening logo. That leaves me confused.

What's the benefit of this PR? What platforms is it expected to affect? Under what settings?

Those feel like obvious questions to someone familiar with these low level pieces, but that's not me, so I don't know if the intent is to make the movies look this way under default settings or not. Put another way: How does one test this PR?

I must warn everyone that I'm overly invested in this because I put an embarrassing amount of time into fixing playback of these specific KQ6-Mac movies in #3512 , so if the intent really is to make them look worse by default... I think my feedback will be predictable! =)

@ccawley2011
Copy link
Member Author

What's the benefit of this PR? What platforms is it expected to affect? Under what settings?

It's intended to benefit platforms like the RiscPC where 8bpp screen modes are noticeably slower than 16bpp modes, or with the Atari backend where true colour isn't available at all.

Those feel like obvious questions to someone familiar with these low level pieces, but that's not me, so I don't know if the intent is to make the movies look this way under default settings or not. Put another way: How does one test this PR?

As mentioned previously, dithering is currently enabled based on if RGB rendering is enabled or not. It's currently off by default but I wouldn't have major objections to it being turned on by default as long as it has a proper fallback if switching to true colour fails.

I must warn everyone that I'm overly invested in this because I put an embarrassing amount of time into fixing playback of these specific KQ6-Mac movies in #3512 , so if the intent really is to make them look worse by default... I think my feedback will be predictable! =)

Do you have any screenshots of the original executable playing the videos in 8bpp? It's possible I've missed something that the original did when playing videos.

@sluicebox
Copy link
Member

@ccawley2011 Thank you for those details. I can arrange for screenshots from a Mac emulator if necessary but that implies that you're not able to run KQ6 Mac under an emulator to see how it works.

Ping me on discord if you'd like help setting up KQ6 Mac emulation environments.

I'm concerned that we risk talking past each other, because you're speaking of interesting environments and settings, meanwhile I'm saying that this ruins the movie in default configurations on our most popular environments. Given that, I feel like the ball is in your court. =)

@bluegr
Copy link
Member

bluegr commented Nov 20, 2023

The issue here is that on our most used systems, dithering kicks in, where it shouldn't. We could do the following:

  • Upon launch, check if we have a configuration key to use RGB mode
  • Check if no such configuration key exists, and the system supports RGB mode
  • If the above two statements are true, throw a warning and either 1) override the option to use RGB for KQ6 Mac to true or 2) switch to 16bpp for the intro video and then back to palette mode (whichever is better, I'd prefer number 2 in this case)
  • The user can then opt out of the RGB mode (if we choose to go with solution 1 above)

@ccawley2011 @sluicebox thoughts? Opinions?

@bluegr
Copy link
Member

bluegr commented Oct 31, 2024

Closing in favor of #6211

@bluegr bluegr closed this Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants