Reimplementing RoQ video format and make sure video integration in maps work

[illwieckz]

RoQ is a legacy format that was standard in idTech3 and the default one for its kind, like TGA or WAV for image and sounds, not supporting it is a regression.

We also need to make sure features like videoMap works, RoQ can be the default codec to use when debugging or implementing videoMap feature, especially since there are game assets out there using RoQ videos.

[illwieckz]

See:

• https://github.com/Unvanquished/Unvanquished/issues/2798

[slipher]

RoQ can be the default codec to use when debugging or implementing videoMap feature

Huh? Why not one that is supported by viewers in use today and has compression?

[illwieckz]

Because we have a reference implementation that works (Quake 3, Tremulous, etc.).

[ghost]

Huh? Why not one that is supported by viewers in use today and has compression?

If this is an excuse to remove things, then you should remove CRN support, as it's supported by nothing outside of unvanquished and perhaps unity. Tools that allow to load several image formats do not know about crn, and such (like FreeImage lib) tools would be valuable to clean the codebase.

Obviously, I don't think removing crn support would be an improvement.

[illwieckz]

One thing I have in mind and I want to care about is how it is easy for quake3 mods to port to Dæmon. Having stock quake3 formats being supported means porting efforts only have to focus on code porting first. It also means that if something doesn't work as expected, the regression is in code, without doubt.

[ghost]

I'm ok to spend some time to reintegrate this, if that time won't be a waste as it often is with unvanquished. Since this is also about the engine, I simply can not just ignore what you do here as a mod can't undo engine things (not without pain for users).

I gave a quick look at various sources, and it seems ROQ support won't be hard to maintain, and is about ~2KLoC.

[illwieckz]

And it's like we will never have to update it. All the changes we may do to it is fixing warnings and some c++ purity or things like that…

[ghost]

if you want commits to fix warnings, I can send you a bunch of patches doing that, but you should 1st start to look at the patches to get rid of the cvar memleaks before I do more efforts. The project lost my trust for real, I was not playing games. Fixing this is going to require some work, if it's intended. Until then, I'll limit my contributions to patches that I don't have to maintain, because I'm a person who share knowledge, and bug reports, sometimes with full analysis, but not always.

[illwieckz]

@bmorel do you have a branch that would bring back RoQ video support?

[illwieckz]

Actually an argument in favor of keeping support legacy formats is that we don't have the manpower to make sure we can 100% reimplement the feature with a non-legacy format. We need to guarantee that something works, even if old and rusty.

For example, if we implement a new video format (let's say Webm as an example) but not RoQ, the mappers would then face this problem:

• for video they would have to convert RoQ to WebM because “legacy format is dropped“
• for doors they would have to convert models to md3 because “legacy format is the only format having implemented all the features“.

When someone creates a map or ports a map, he should know what to do to be safe, with a “single principle”. For example if the “single principle” is “legacy formats always work like before“, then the mapper has a solid safe baseline without any surprise and can be sure everything he knew worked will work. Then, the mapper knows “he can try new shiny things”, but he is the one who takes the risk, and he is the one evaluating if it's worth it.

Not doing that means that on every format the support is a flipping coin from mapper point of view:

• Model? Should I prefer old or new format to be safe? Let's try something and get the wrong side of the coin!
• Vidéo? Should I prefer old or new format to be safe? Let's try something and get the wrong side of the coin!

If we don't have a “single principle” that coin flipping game will turn out this way for the mapper:

I tried video it didn't worked, they said I had to use the “new format” to be safe, OK. Then I tried to do a door and I used the “new format” to be safe and then it didn't worked and they said I had to use the “old format” to be safe, I'm crying.

Keeping support for legacy formats looks the less-costly “single principle”: we know the cost of keeping legacy stuff is maintenance, while the cost of promising to fully re-implement features with other formats is totally unclear and bets on our own skills (which is itself a flipping coin).

[ghost]

@bmorel do you have a branch that would bring back RoQ video support?

If I had it, you would know already.

As for the stuff about legacy, let's be clear: daemon-engine is based on an engine which never supported a lot of basic features, and this is the core problem. XReal really sucked. I still have work to do on my mod to reduce dependencies with daemon's API (notably glm, but also getting sane and actually useful trace wrapper) but when done I will definitely investigate the idea of working with another engine.

Daemon lacks manpower to a very alarming point, no release despite a major memory leak being fixed (but CBSE submodule was merged into unvanquished's git history, and more moves to use CBSE more despite knowing it's a bad implementation were made!) and no attention on this which provides patches that fix memleaks, which are necessary to run the game under ASAN, all this makes me thinking the real problem is not manpower but the priorities set. Of course, nobody is paid to work on this, so nobody have to receive instructions, but then this pretends that "Do you fix bugs before writing new code? Let's say yes," which feels like a lie to me. Since this received no attention, I can not trust a patch that implements RoQ would do. Is @slipher's PR to merge the fix I described regarding the shader changes merged yet? Probably not. I've seen plenty of examples of PRs rotting, approved or not, while other were forced to merge despite being untested and unreviewed, so that's not the blocking point, let's be honest.

Bike-shedding, refactoring for the sake of it, increasing the hard-coding of things which leads to duplicated code (yes it does) and defending trolls definitely have a higher priority than bugfixes or refactorings that lead toward reduced maintenance cost.

I nowadays consider daemon-engine as one of the bigger weak points of my mod. My mod which is independent from unvanquished's codebase, with the common ancestor being around 300 commits ago. Yes, almost 300, and I have not yet merged all of old betterai features in it. Those commits are mostly refactoring going toward less duplicated code, more isolated code for faster compilation, more configurability, and reduce code maintenance cost (glm is one example, but I also have some efforts to get rid of the undebuggable CBSE. I mean it, just try to run gdb on this mille-feuille crap and tell me again it does not increase maintenance cost! Or perhaps you guys are smart enough to write bug-free code, but considering the regression in AI code I've seen on @cu-kai bots, I doubt it. And yes, I know which commit is responsible for that, which is why I didn't cherry picked it).

Another big blocking point is the lack of support for modern model formats. This is caused by the fact that each model is implemented manually, instead of relying on a library that would do the work (I name it: assimp). There is no stable toolchain to work with iqm or md5 and even less for md3. Those are supported by assimp, though. Yes, skeletons and animations as well, but more importantly, many more are, which are supported out of the box by blender, the most important open source 3D tool. I could also talk about the use of crn, which are only supported by unvanquished and perhaps by unity. Apparently, they're loaded into netradiant as if they were a pack of DDS, which according to crunch's documentation is the poorer way in terms of performances (it apparently supports 2). If the images are just encoded as zipped DDS, then just zip DDS, it will make support by, say, trenchbroom, easier.

Last problematic point is that unvanquished both have inconsistent file naming, and steals tremulous's names, which creates texturing problems in maps which use misc-model, the most obvious being the previous iteration of one of @cu-kai's maps, but this problems appear in other maps as well, even if more discrete as usually, it's human weapons which are baked into geometry.

All this can be summarised as: the unvanquished project lacks a goal and thus efforts are wasted in many often conflicting directions. And before it lacked a goal, it lacked a realistic goal: no, "we are not trem so let's change everything and loose content compatiblity" is not such a realistic goal.

This is something I've said many, many times on IRC, but was always ignored or worst. If you guys finally realize the problem, well, good for you. My intent is clear since a while, and since a shorter while, it is also clear that working alone is more efficient than working with you. My changes cost me less time, and the time I save by not arguing about my changes can be spent in funnier or more useful occupations.

[illwieckz]

and no attention on this which provides patches that fix memleaks

One cannot publish code in the least reviewable way and at the same time complain the code is not reviewed. 🤷‍♀️️

I actually made the effort to download the zip when you opened the issue, which was already an extra effort that was totally added for free and that I would have never done if this was a PR. Then I seen this was a set of patch, I started to think about the best way to review it, and then I gave up because at some point I already spent the free time I had, before actually doing any review.

[ghost]

One cannot publish code in the least reviewable way and at the same time complain the code is not reviewed

It's not like the reviews got helpful to me in the past, except to receive endless nitpicks and give me shitload of git conflicts between my branches.

Also, one can not ask me to maintain GH PRs neither, nor to implement RoQ nor anything. I can also play that game of "you can't ask me to do things".

[slipher]

Is @slipher's PR to merge the fix I described regarding the shader changes merged yet?

Yes

@bmorel What is the common ancestor commit of your (gamelogic) fork with upstream? I started an effort to look for interesting commits to merge here and it would be helpful to be able to work with the changes as a git branch, in addition to as patches.

One cannot publish code in the least reviewable way and at the same time complain the code is not reviewed. 🤷‍♀️️

I actually made the effort to download the zip when you opened the issue, which was already an extra effort that was totally added for free and that I would have never done if this was a PR. Then I seen this was a set of patch, I started to think about the best way to review it, and then I gave up because at some point I already spent the free time I had, before actually doing any review.

I agree that publishing a small number of commits in a zip is a rather annoying workflow to deal with. Unlike the mod dpk's patch set which has hundreds of commits, so it's actually worthwhile to wrangle them :)

[illwieckz]

@slipher maybe https://codeberg.org/bmorel/Unvanquished/commits/branch/pre-release is the branch (0266-CFG-sane-default-config-for-bots being renamed to AI: sane default config for bots and then later things are newer.

[ghost]

What is the common ancestor commit of your (gamelogic) fork with upstream?

aa0df65ca110b69e073acb167d57886de3dbc398 as indicated on my mod's changelog entry. It's also available on codeberg, the "pre-release" branch, that I have to admit does not have a stable history as I try to make it's commits regression-free. Each commits. I also take a great care of having them all compiling and running individually. Lot of work, but less than trying to send PRs.

Note that git have a rather useful tool for that: git merge-base. Also note that since my patches are generated by git format-patch rebuilding my branches is doable easily with git am, which is on purpose.

I agree that publishing a small number of commits in a zip is a rather annoying workflow to deal with.

This is a direct consequence of the fact I no longer want to talk about my code changes with you all. Sending patches means: "there's an issue. Here is one fix. Apply it the way you want." compared to a PR which means "there's this issue and it's fix I'd like to discuss with you.". Very different, because since you require your tastes to be applied, at least this way, you apply them, not me.

[illwieckz]

Yes but that puts more work on other's shoulders, which increases the original problem of a lack of manpower. I can fully understand why and how doing like this can abstract this lack of manpower on one side, and then how it can be seen as preferable for one side, at least on short term, since that's the principle of outsourcing. The problem of outsourcing in volunteering context is that since there is no transaction the cost is outsourced with the work. I remind my will is to reduce the level of expectations people put on others, it means I want to reduce that level of expectation from you and then the related weight on you, but I also want the same for everyone. I'm always in favor of making contributions easier to merge than making them harder to review to begin with. It's great if @slipher wants to take the extra work of merging an external tree, but as I said, my only spot for dealing with an external tree is already taken.

[ghost]

which increases the original problem of a lack of manpower.

Looks like a joke to me, since there's enough manpower to move things toward CBSE and merge the submodule, while knowing that CBSE is a poor implementation of ECS, but not to open a zip archive that contains a bugfix that allows in turn to run the game with ASAN enabled in the upstream cmake? There is no lack of manpower, only a problem about how it's used.

Now, I had several options:

1. do nothing and complain. Not my stuff.
2. patch locally and go with it. Not much of my stuff neither.
3. patch locally and do a PR. Experience proved to me that this is just giving myself more work with a very low efficiency level, when it comes to unvanquished.
4. patch locally, and share the patch, letting you guys do whatever you want. This is the most efficient way for everyone: this way, you don't have to review and wait for PR's author to do your bidding, you can just change on the fly, and the author does not needs to care about the nitpicks neither.

So, the way I read your reaction is basically you asking others (here, me) to do work (RoQ, maintaining a PR for the leak), but refusing to be asked to do some if they ask (like, releasing a bugfix for the engine that would fix a ~200Megs per map loading leak). Quite unfair.

Yes, you have a life outside of unvanquished. So do I. This applies to everyone. It would also be possible to just read the issue that describes memory leaks that are spotted by ASAN on the very start, ignore the patches, and fix the bug still. There is no need to pick my changes, it's just the easier way as they're already written and I'm simply sharing them, which i have no obligation to do, since the code is not under AGPL.

[ghost]

At least those commits are responsible for the regression:

• c64b504cc3a8e5428ed010c38509751dea1e1b8a
• 870df5bfd029c00fa2301072f23b3090e7bd7989

Reverting them makes the map mxl-school work (on the video at least) but nova is still broken, and actually worst, perhaps because of the missing portal alphablend feature, which exists in ioquake3. Reverting those commits and making sure mxl-school works took me ~2H, while not knowing daemon's codebase.

Here's the full diff, not as .patch as I don't consider this ready (since: nova is worst):

Edit by illwieckz: the 3600 lines patch is now attached: revert-roq.patch.txt

I'm putting this here as reference, nothing less, nothing more.

[sweet235]

They have been NUKED for a reason, I bet.

[ghost]

Huh? Why not one that is supported by viewers in use today and has compression?

mpv supports RoQ without any problem.

[illwieckz]

Even The TI-Nspire calculator has a RoQ player. 😄️

@bmorel, I prefer you attach patches to your message than to paste them if it's not short. Even if not ready (just says it's not ready).

[ghost]

As said: this is just for future reference, I don't believe this to be clean. Also, GH refuses .patch files IIRC (even if saying otherwise) and I didn't wanted to spend more time on that.

[illwieckz]

A not-clean patch can be attached, it's fine, just say it's not clean. GitHub accepts .txt, this is good enough.

There is no rule like “a patch should be clean”, exactly like there is no rule and never was any rule saying “only perturbed and illwieckz can approve a PR, so let's never approve them and never merge them and complain they never get merged”.

We definitely have to destroy those mental locks that are just wasting our efforts and making things difficult to anyone for no reason, see this thread.

[illwieckz]

I updated your comment to attach the patch instead, there is no way I'll let this thread being polluted by 3600 lines of patch.

[illwieckz]

Some comments by @slipher from a PR from 2020:

• https://github.com/DaemonEngine/Daemon/pull/391

This PR was removing OGM support because Theora was vulnerable and unmaintained (Google is going to remove Theora from Google Chrome for the same reasons), making the Video feature without any codec, leading to the removal of the Video feature itself:

I don't understand, OGM is the only supported format. I'm not going to add support for a new one when there isn't any demand for the feature. It harms development to keep dead code in the build; much better to delete it and then revert when a need arises.

Thanks. If you find a map with videoMap or something, I'd be happy to help recreate such functionality.

So it looks like we have at least 2 maps making use of video.

I recommend using the RoQ codec first because:

• This is the legacy format and it is considered granted by mappers, like TGA or WAV.
• We actually have maps with RoQ files.
• The RoQ code exists and implementing is is basically a revert.

[illwieckz]

I noticed something interesting,

The map mxl-school has a RoQ file: video/guess256.RoQ

But the map nova doesn't, instead it seems to define the video frames in shaders…

models/mapobjects/monitor/display_roq
{
    {   
        map $lightmap
        rgbGen identity
    }
    {
        map models/mapobjects/monitor/display_0.jpg
        rgbGen identity
    }

}
models/mapobjects/monitor/display_roq_01
{
    {
        map models/mapobjects/monitor/matth.jpg
    }
    {
        map models/mapobjects/monitor/matth_b.jpg
        blendfunc GL_ONE GL_ONE
        rgbGen wave sin .8 0.05 0 10
    }
}
models/mapobjects/monitor/display_roq_03
{
    {   
        map $lightmap
        rgbGen identity
    }
    {
        map models/mapobjects/monitor/display_b.jpg
        rgbGen identity
    }
    {
        map models/mapobjects/monitor/display_b.jpg
        rgbGen identity
        rgbGen wave sawtooth .8 0.4 0 20
        blendFunc add
    }
    {
        map models/mapobjects/monitor/load.jpg
        rgbGen identity
        tcMod rotate 100 100
        blendFunc add
    }
}
models/mapobjects/monitor/display_roq_04
{
    {
        map models/mapobjects/monitor/display_b.jpg
        rgbGen identity
    }
    {
        map models/mapobjects/monitor/display_b.jpg
        rgbGen identity
        rgbGen wave sawtooth .8 0.4 0 20
        blendFunc add
    }
    {
        map models/mapobjects/monitor/text.jpg
        rgbGen identity
        tcMod scale 1 0.45
        tcMod scroll 0 0.030
        blendFunc add
    }
}
models/mapobjects/monitor/display_roq_02
{
    {
        map models/mapobjects/monitor/2016.jpg
    }
    {
        map models/mapobjects/monitor/2016_b.jpg
        blendfunc GL_ONE GL_ONE
        rgbGen wave sin .8 0.05 0 10
    }
}

The models/mapobjects/monitor/monitor_nobase_roq.md3 model has reference to models/mapobjects/monitor/display_roq shader.

So it looks like we had two ways to do videos? One by feeding a RoQ file, another one by feeding separate frames as materials?

[illwieckz]

Nova has those entities:

{
"target" "intro_shader3"
"wait" "18"
"targetname" "intro_delay_01"
"origin" "9128.000000 2752.000000 -42.000000"
"classname" "target_delay"
}
{
"targetname" "intro_shader3"
"targetShaderName" "models/mapobjects/monitor/display_roq"
"targetShaderNewName" "models/mapobjects/monitor/display_roq_03"
"origin" "9130.000000 2689.000000 -44.000000"
"classname" "target_relay"
}
{
"target" "intro_shader0"
"wait" "5"
"targetname" "intro_delay_01"
"origin" "9044.000000 2752.000000 -42.000000"
"classname" "target_delay"
}
{
"targetname" "intro_shader0"
"targetShaderName" "models/mapobjects/monitor/display_roq"
"targetShaderNewName" "models/mapobjects/monitor/display_roq_01"
"origin" "9037.000000 2689.000000 -44.000000"
"classname" "target_relay"
}
{
"target" "intro_shader2"
"wait" "15"
"targetname" "intro_delay_01"
"origin" "9098.000000 2752.000000 -42.000000"
"classname" "target_delay"
}
{
"targetname" "intro_shader2"
"targetShaderName" "models/mapobjects/monitor/display_roq"
"targetShaderNewName" "models/mapobjects/monitor/display_roq"
"origin" "9100.000000 2689.000000 -44.000000"
"classname" "target_relay"
}
{
"target" "intro_shader"
"wait" "10"
"targetname" "intro_delay_01"
"origin" "9072.000000 2752.000000 -42.000000"
"classname" "target_delay"
}
{
"targetname" "intro_shader"
"targetShaderName" "models/mapobjects/monitor/display_roq"
"targetShaderNewName" "models/mapobjects/monitor/display_roq_02"
"origin" "9070.000000 2689.000000 -44.000000"
"classname" "target_relay"
}
{
"target" "intro_shader1"
"wait" "20"
"targetname" "intro_delay_01"
"origin" "9157.000000 2751.000000 -42.000000"
"classname" "target_delay"
}
{
"targetname" "intro_shader1"
"targetShaderName" "models/mapobjects/monitor/display_roq"
"targetShaderNewName" "models/mapobjects/monitor/display_roq_04"
"origin" "9159.000000 2689.000000 -44.000000"
"classname" "target_relay"
}

That doesn't look like a video to me (not like a RoQ file, I mean), but @bmorel said reverting the video code removal brought back the “video” in game.

[slipher]

That doesn't look like a video to me (not like a RoQ file, I mean), but @bmorel said reverting the video code removal brought back the “video” in game.

Where did he say that? To the contrary, I see this:

Reverting them makes the map mxl-school work (on the video at least) but nova is still broken, and actually worst, perhaps because of the missing portal alphablend feature, which exists in ioquake3.

If nova has started to work, that's probably because a fix to restore shader replacement (targetShaderName/targetShaderNewName) was merged into master a few days ago.

[illwieckz]

Ah, maybe I was confused by those talks happening at the same time then.

[illwieckz]

I was probably confused by that exact sentence, where it is said dealing with video code has impact on nova screen, and similar ones that talk about video and nova being affected by video code:

2023-10-04 07:32:31 +0200 <> @masmblr also, I thought the 3rd screen of nova was
                          an unfinished video "portal" to the map... my bad. It was a
                          regression in unvanquished as well.

The RoQ removal is considered a regression and a video is not a portal, so believing the video on Nova screen was actually a video file was my first assumption. 🤭️

2023-10-28 23:27:32 +0200 <> illwieckz: so, some serious stuff, because why not:
                          those reverts make mxl-school work, *but* they make nova
                          buggier

Here if the video code has impact on something that looks like a video on screen, my first assumption was that the video would be a video file. 🫣

My bad.

[ghost]

So in short, Nova used to use or a least plan using a RoQ video, but the author changed he's mind and instead managed to use non-video shaders to get the expected result.

The only map I know actually using RoQ is mxl. The bug I was seeing after the reverts in nova was related to unrelated changes (if I may say so) in my own engine codebase, as I was on a buggy branch by accident, related (again unrelated to video though) to some old attempt to silent the shitload of warnings daemon emits at build time (thousands of those, and this, even when only including the libary, because I use fascist-mode in my compilers).

The currently known by me problems with that patch are only that they add more warnings (C-style casts, implicit casts, NULL instead of nullptr, padding (can't be avoided, but could be reduced by a great load in unv/daemon), that kind of stuff).

[slipher]

We also need to make sure features like videoMap works, RoQ can be the default codec to use when debugging or implementing videoMap feature, especially since there are game assets out there using RoQ video

ROQ has no specification, which would seem to make it an unfortunate choice for "the default codec to use when debugging or implementing videoMap". Some of the semantics are pretty wacky.

[slipher]

The mxl-school map has a questionable license, so illwieckz has made a test map to use instead: https://dl.illwieckz.net/b/unvanquished/test-assets/map-test-video_2.1-20231106-091111-e20b56a.dpk