Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VIDEO: Add generic subtitles in SRT format #4186

Merged
merged 18 commits into from Sep 23, 2022
Merged

Conversation

sev-
Copy link
Member

@sev- sev- commented Aug 14, 2022

There is an ongoing trend to add subtitles to video-based cutscenes in games.

This PR adds two classes Video::SRTParser and Video::Subtitles classes that do the following:

  1. Load specified subtitles in SRT format
  2. Plays them in a GUI overlay on top of the video in the specified bounding box
  3. Colors are supported
  4. TTF fonts are supported and they're drawn with outlines

Demonstration on how to use it is in Groovie engine which was implemented in one of the re-releases, and I will add support for those versions eventually.

Comments are welcome.

@ccawley2011
Copy link
Member

ccawley2011 commented Aug 14, 2022

I don't think the overlay is a good fit for displaying subtitles, since backends aren't required to allow updating the game screen while the overlay is opened - in particular, the comments in common/system.h say this:

For 'coolness' we usually want to have an overlay that is blended over the game graphics. On backends that support alpha blending, this is no issue but on other systems this needs some trickery.

Essentially, we fake (alpha) blending on these systems by copying the current game graphics into the overlay buffer when activating the overlay, and then manually compose whatever graphics we want to show in the overlay.
This works because we assume the game to be "paused" whenever an overlay is active.

@athrxx
Copy link
Member

athrxx commented Aug 14, 2022

Displaying subtitles on a CLUT screen without the overlay is a pita, though. It would require to hack up the video player to reserve a text color.

Might be easier to just have a way to force updates of the normal screen.

Or we might just introduce another texture, only to be used for the subtitles. But that would also require backend upgrades...

_loaded = _srtParser.parseFile(fname);
}

void Subtitles::setBBox(const Common::Rect bbox) {
Copy link
Contributor

@mgerhardy mgerhardy Aug 15, 2022

Choose a reason for hiding this comment

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

could get passed by ref


Common::String text;

SRTEntry(uint seq_, uint32 start_, uint32 end_, Common::String text_) {
Copy link
Contributor

@mgerhardy mgerhardy Aug 15, 2022

Choose a reason for hiding this comment

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

text_ could get passed by ref

return true;
}

Common::String SRTParser::getSubtitle(uint32 timestamp) {
Copy link
Contributor

@mgerhardy mgerhardy Aug 15, 2022

Choose a reason for hiding this comment

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

method could be const

@sev-
Copy link
Member Author

sev- commented Aug 15, 2022

I don't think the overlay is a good fit for displaying subtitles since backends aren't required to allow updating the game screen while the overlay is opened - in particular, the comments in common/system.h say this:

Well, in fact, I think we could safely drop that comment. Yes, when we added it 17 years ago, we were thinking about making it simpler for the backend developers. But since that, all the current backends do support displaying both overlays at the same time as they were following the SDL backend implementation.

We do use the GUI overlay, for instance, for the Cloud Sync icon these days.

So yes, it is safe to use it. To make sure it works as expected, I could say add another test in the graphics module.

The advantages of drawing subtitles on the overlay are the higher gfx resolution, often native and no headache with palettes.

@lephilousophe
Copy link
Member

lephilousophe commented Aug 15, 2022

The N64 port seems to have such limitation: either the game is displayed, either the overlay.
It seems that game screen is copied to overlay when it is cleared.

@ccawley2011
Copy link
Member

ccawley2011 commented Aug 15, 2022

The N64 port seems to have such limitation: either the game is displayed, either the overlay.
It seems that game screen is copied to overlay when it is cleared.

This is also the case for SurfaceSdlGraphicsManager as well. Because both of them exclusively use software rendering, changing them to use proper alpha transparency would likely hurt performance on platforms that would need software rendering.

We do use the GUI overlay, for instance, for the Cloud Sync icon these days.

That's rendered using the OSD, not the overlay. This might be a better option here since it's typically implemented using small surfaces which should reduce the performance impact involved with alpha transparency. It also won't affect the mouse coordinate scale when it's active, so can be used for subtitles during gameplay as well as in cutscenes.

@sev-
Copy link
Member Author

sev- commented Sep 23, 2022

@BLooperZ already tried to implement subtitles for Feeble Files based on this.

The main finding is that indeed, even SDL backend does not support transparency, thus, a compromise was implemented: now there is a rectangle drawn around the subtitles, so on SDL backend it is neat and looking nice, while on backend with transparency it is still transparent. We would need to make it optional in the future.

I was looking into exposing Overlay similar to lockScreen(), but number of backends do not keep Overlay internal representation in a Surface, so that did not work well.

The same problem goes with OSD: it is not even implemented everywhere.

Thus, I am merging this as is, but we will need to add more flexibility to the rendering methods. I will do that in-tree eventually.

Thanks everyone for the feedback.

@sev- sev- merged commit 3f611f9 into scummvm:master Sep 23, 2022
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants