Skip to content

Conversation

@illwieckz
Copy link
Member

  1. Apply revert patch by @bmorel for c64b504 and 870df5b from:

I modified the patch to fix some merge issues with external_deps/build.sh for historical purpose (this commit can still build the broken Theora codec on Linux).

  1. Remove broken and unsafe OGM (Theora) support again.

  2. Remove broken cinematic mode and command again.

The videoMap feature is working again, currently only with legacy RoQ format.

Here is mxl-school map on viewpos 699 1228 304 -58 -7:

unvanquished_2023-11-06_030404_000

Fixes #936:

See also:

Morel Bérenger and others added 2 commits November 6, 2023 02:11
…other things)

- Revert c64b504

>  Drop support for cinematic mode and videoMap

- Revert 870df5b

> NUKE the RoQ video decoder

Also fixes theora building as external_deps (tested only on Linux).

Co-authored-by: Thomas Debesse <dev@illwieckz.net>
@slipher
Copy link
Member

slipher commented Nov 6, 2023

I was opening the dpk of mxl-school to see the roq file and noticed that the license states "This map [...] may NOT be modified IN ANY WAY". So that DPK version, which adds various files, renames files, etc., would seem to be in violation. The original can't be loaded because it uses an underscore in the map name. Is there any legally available test data?

Looking over the code a bit, I see various buffer overflow problems (e.g. reading 65536+8 from a buffer of size 65536, I think I saw a writing one too but forget where). Please don't merge until I have confirmed that there are no obvious security holes.

@illwieckz
Copy link
Member Author

illwieckz commented Nov 6, 2023

So that DPK version, which adds various files, renames files, etc., would seem to be in violation.

Is printing a docx modifies the text in the docx? The format is totally different though people would say the work is not modified, the story would be carried without modification.

Anyway, here is a test map:

https://dl.illwieckz.net/b/unvanquished/test-assets/map-test-video_2.1-20231106-091111-e20b56a.dpk

This uses the Tremulous splash, but downscaled to 256×256 because it looks like RoQ video as map texture should not be larger, and stripped from source because map video can't play sound.

I downscaled it and stripped sound this way (ffmpeg knows how to convert to RoQ, including both video and sound if there is):

ffmpeg -i splash.roq -an -vf scale="256:256" tremulous-splash-256.roq

@slipher
Copy link
Member

slipher commented Nov 6, 2023

Is printing a docx modifies the text in the docx? The format is totally different though people would say the work is not modified, the story would be carried without modification.

Yes, printing would involve creating a modified digital version of the document. The Creative Commons ND licenses explicitly allow "technical modifications" so that a work can be used in different formats. (Stuff you do in private and don't distribute seems to usually fall under fair use exceptions though, so the printing thing would not be infringement.)

I think a better analogy with the mxl-school DPK (converted pak format and added minimaps among other stuff) would be converting the docx to a PDF, adding a table of contents, and distributing it to other people.

@sweet235
Copy link
Contributor

sweet235 commented Nov 6, 2023

I did not change that map in any way.

@slipher
Copy link
Member

slipher commented Nov 6, 2023

I did not change that map in any way.

OK, someone else did though. It has minimaps so it's clearly been modified for Unvanquished.

@illwieckz
Copy link
Member Author

illwieckz commented Nov 6, 2023

Those topics have multiple levels. We don't consider a book being modified if the wording is not modified, some page layout can be considered part of the work too, but every print of the book will not be considered a modification, despite you can be 100% sure the paper will not be the same. A book author will complain his work was modified if that's not the story he wrote. This is a moral right, for example : « I don't want to be attributed for a work that is not mine ». Anyway this is totally off topic.

This PR is about keeping compatibility with legacy formats, this is not related to how people bring legacy formats (we even have legacy pk3 support if needed), and we can produce test assets if needed. If a technical discussion brings an off-topic discussion (like here to discuss how to deal with legacy assets, etc), we better do this kind of talk in forums .

@slipher
Copy link
Member

slipher commented Nov 6, 2023

Anyway, here is a test map:

https://dl.illwieckz.net/b/unvanquished/test-assets/map-test-video_2.1-20231106-091111-e20b56a.dpk

Thanks, it works. The mxl-school repacking is now a moot point for me, as long as it doesn't use any features not used in the test map.

@illwieckz
Copy link
Member Author

Anyway, here is a test map:
https://dl.illwieckz.net/b/unvanquished/test-assets/map-test-video_2.1-20231106-091111-e20b56a.dpk

Thanks, it works. The mxl-school repacking is now a moot point for me, as long as it doesn't use any features not used in the test map.

What the test map doesn't do is to apply some filter to the video (school map does it). I can extend the test map to add some more extended uses case, but at least this is good enough to check that the video plays, and we can extend the test map later.

@slipher
Copy link
Member

slipher commented Nov 7, 2023

Hmm something missing from the test map is two different videos playing at once. There is a lot of global variable abuse so that would be important to test.

@illwieckz
Copy link
Member Author

Here is an updated map with two videoMap surface, the one on the right having intentional flickering effect (so we can verify the effect is applied):

https://dl.illwieckz.net/b/unvanquished/test-assets/map-test-video_2.1-20231107-010139-59e1280.dpk

Right now it plays the same RoQ file in both surfaces, we may want to try to play two RoQ files. Actually the movie used for the RoQ file in mxm-school is Nosferatu from 1922 and is in public domain so maybe we can reuse that file.

@illwieckz
Copy link
Member Author

Here is an updated map with two different RoQ files played at the same time:

https://dl.illwieckz.net/b/unvanquished/test-assets/map-test-video_2.1-20231107-023827-9a75c8e.dpk

There is indeed a bug when the two RoQ movies are not the same, right now the engine can't display both of them at the same time. If only one move is on screen, it works.

The map is made in a way it's easy to have two movies on screen at the same time, or only one.

I used excerpts from some public domain movies:

  • on the left: Excerpt from Admiral Ushakov (1953), battleship scene, color.
  • on the right: Excerpt from The Kid (1921 original cut), regrets scene, black & white, with some flicker effect applied on it.

@ghost
Copy link

ghost commented Nov 7, 2023

I did not change that map in any way.

OK, someone else did though. It has minimaps so it's clearly been modified for Unvanquished.

I wonder who, then. Or there's a disagreement about what is "changing".
Adding a minimap? This could be done by engine when a map gets downloaded (and perhaps it should do this, would make map packages easier to do, and would allow for minimap improvements to happen without needing a new package)...
Moving files around? Not sure that'd be considered a modification neither...

I'm pretty sure that people would not argue against repacking of their map if the map's bsp, textures, models, and other "script files" are not modified (for example to modify entities or shaders for a different game). In fact, without unzipping a map, one can not know the licence, so it seems rather logic to me that it only applies to the files in the archive, instead of to the archive itself.
Anyway: I'm a lawyer as much as I consider the importance of "did someone changed that map?", which is: not at all.

@slipher
Copy link
Member

slipher commented Nov 7, 2023

I'm working on cleaning this up. Luckily there is a lot of code that can be deleted. For example the smootheddouble, half, and samplesPerPixel options can never be set (this is true in ioq3 as well), so the code gated behind them can be deleted.

@slipher
Copy link
Member

slipher commented Nov 8, 2023

On the slipher/roq branch you can now play two videos at once. There's still a lot of work to be done though.

MSVC on Appveyor reports those errors:

C:\projects\daemon\src\engine\client\cl_cin.cpp(1102,85): error C2220: the following warning is treated as an error [C:\projects\daemon\build\ttyclient-objects.vcxproj]
C:\projects\daemon\src\engine\client\cl_cin.cpp(1102,85): warning C4389: '==': signed/unsigned mismatch [C:\projects\daemon\build\ttyclient-objects.vcxproj]
C:\projects\daemon\src\engine\client\cl_cin.cpp(1102,136): warning C4389: '==': signed/unsigned mismatch [C:\projects\daemon\build\ttyclient-objects.vcxproj]
@illwieckz
Copy link
Member Author

Thanks! This looks very good, that's awesome !

Do you want a test map with a RoQ video larger than 256×256? Previously it didn't worked but if that's a bug (and not a design limitation), it would be nice to have it fixed.

I assume the Frame_yuv_to_rgb2 function to be reusable in the future if we do a WebM/VP9 implementation. To make it easy to revert this function if we need it one day I redid my branch with that code being removed in a single commit that can be reverted without bringing back other unwanted things.
I made it in a way your branch should be easily rebased on mine (except maybe some easy to fix conflict in your remove unused stuff and garbage comments commit but maybe not and it should be straightforward).

@slipher
Copy link
Member

slipher commented Nov 9, 2023

Do you want a test map with a RoQ video larger than 256×256? Previously it didn't worked but if that's a bug (and not a design limitation), it would be nice to have it fixed.

I wonder if it worked in q3. If the mxl-school one (designed for q3) was exactly this size that kind of suggests no... there is a ton of unreachable dead code in the ioq3 video code so the fact that there is code for something doesn't mean it ever worked. If it doesn't work in ioq3 I don't want to spend more than like 15 minutes on that.

@slipher
Copy link
Member

slipher commented Nov 9, 2023

I assume the Frame_yuv_to_rgb2 function to be reusable in the future if we do a WebM/VP9 implementation. To make it easy to revert this function if we need it one day I redid my branch with that code being removed in a single commit that can be reverted without bringing back other unwanted things.

I don't really care about this since I think the ioq3 version is a better reference anyway. I plan to add a comment linking to that in case any removed feature turns out to be needed. Our version is mostly the same except bit-rotted and with minor API migrations.

@illwieckz
Copy link
Member Author

I wonder if it worked in q3.

Documentation I read said it didn't.

@illwieckz
Copy link
Member Author

Must be 256x256 to be played correctly in-level by vanilla Q3/OA. If the RoQ has to be used inside menus (full screen) instead of inside a map, it should be safe up to 512x512.
-- https://openarena.fandom.com/wiki/RoQ

@sweet235
Copy link
Contributor

sweet235 commented Nov 9, 2023

I do not agree with some opinions @slipher has expressed here. But I do agree with their opinion that it might be more future-proof to not support RoQ videos, and maybe support some other video codec in the future.

@cu-kai
Copy link
Contributor

cu-kai commented Nov 9, 2023

Some other video codec would also be fine, any RoQ videos could be converted for the purposes of porting legacy maps to Unvanquished.

@illwieckz
Copy link
Member Author

As said in chat, while I believe it would be good to have a newer and state-of-the-art codec, the same way we have WebP and CRN for images, I believe it is good to keep compatibility with the legacy video format of the engine, the same way we keep TGA image support.

The format may be lossless, the code may be rusty and may have some limitations (no more than 256×256 size, etc.) but it actually works and proves the videoMap mechanism works. It means we have a reference implementation we can compare to when implementing newer codecs. It means that if we implement another codec one day, we would be able to check if a regression when using videoMap would come from the codec integration or from the videoMap implementation itself, since we would have two codecs to test. And in fine, this means we can use videoMap feature right now, before any other codec is implemented.

@slipher
Copy link
Member

slipher commented Nov 10, 2023

The nice thing about adding a real video library would be that besides the goofy video on a wall thing, we could use it for the demo video recording. It's annoying to have to make a 1 minute long AVI weighing a gigabyte and then transcode it.

@illwieckz
Copy link
Member Author

Yes but one problem I want us to avoid reproducing is trying to climb too high steps in one go when we can reach lower steps first and end with “nothing” because we look for “better good”:

  • Getting videoMap working with RoQ is a lower step.
  • Once done, cleaning-up RoQ code is a lower step.

After than, we can try to add a new codec, like webm/vp9 or av1.

This next step would be implementable either for videoMap, either for recording, but only one of them at first, then we would implement it for the other task.

In fact, I would not be surprised if implementing a new codec would be easier to be done as an alternative to AVI recording than as an alternative to RoQ videoMap, and I would be totally fine if we go on the recording side first. This for multiple reasons:

  • next-gen codec recording:
    • maybe tweaking recording is easier to do than videoMap,
    • recording issues doesn't affect all players, only the ones recording,
    • tweaking recording doesn't bring any security issues from randomly downloaded content from the internet,
    • AVI is not really what expects people who want to record a game, this is not considered “good enough”.
  • next-gen codec videoMap:
    • RoQ is already “good enough” for videoMap to provide kind of “GIF like” low resolution animations for some surfaces.

@slipher
Copy link
Member

slipher commented Nov 11, 2023

A pretty bad shortcoming that we haven't discussed yet is the way that the video stops playing when you're not looking at and resumes where you left off when you look again. It would be better if the video's time advanced consistently regardless of whether is rendered, which would allow synchronizing the video with other map elements such as movers. The crappy behavior is obligated by the ROQ format because it doesn't have keyframes. If you wanted to keep the timing consistent you'd have to either potentially decode the whole video during one frame, or always continue decoding even when the video is not being seen. A decent video format has keyframes which allow jumping to any time point with only a little processing. It would be unfortunate if we gave the stopping and starting behavior to other video formats in the name of consistency. Thoughts?

@DolceTriade
Copy link
Contributor

Given that there are converters for roq to any other media format, I think we should encourage conversion to something modern.

No q3 map can be played on Unvanquished as is anyways right?

@illwieckz illwieckz mentioned this pull request Nov 12, 2023
@illwieckz
Copy link
Member Author

illwieckz commented Nov 12, 2023

RoQ shortcomings compared to what other codecs can do deserve a dedicated thread:

TL;DR:

  • It's fine to implement with another codec a feature that RoQ can't do.
  • There is no request to port back to RoQ features other codecs can do.
  • The behavior of videoMap RoQ in Dæmon is not expected to be less or more than what Quake 3 / Tremulous did, with same shortcomings.

@slipher
Copy link
Member

slipher commented Nov 13, 2023

After digging into the code, I understand that a video frame is loaded just like any other texture image, so there shouldn't be any difficulty in handling images of different sizes, just need to change 256, 256 to the actual dimensions. (Earlier I saw a complicated RE_StretchRaw function but it turned out to be dead code.) So I'm willing to try videos with dimension 512, smaller than 256, or non-square - it should only take a minute.

@illwieckz
Copy link
Member Author

illwieckz commented Nov 13, 2023

Hmm, I tried to encode a 640×360 video but ffmpeg aborted with:

[roqvideo @ 0x7f03f8001d40] Dimensions must be divisible by 16

@illwieckz
Copy link
Member Author

@slipher here is an updated map:

A 128×128 video on the left (Asteroid animation from Public Domain Archive), a 1280×720 video on the left (excerpt from Big Buck Bunny, CC By 2.5).

@cu-kai
Copy link
Contributor

cu-kai commented Nov 13, 2023

Given that there are converters for roq to any other media format, I think we should encourage conversion to something modern.

No q3 map can be played on Unvanquished as is anyways right?

mxl-school is a perfect example of "Q3 map loaded in Daemon Engine"

@DolceTriade
Copy link
Contributor

I mean you always need to do additional work like repackaging, crunching, creating layouts, etc...

@slipher
Copy link
Member

slipher commented Nov 17, 2023

Superseded by #966.

@slipher slipher closed this Nov 17, 2023
@illwieckz illwieckz deleted the illwieckz/roq branch November 17, 2023 16:30
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.

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

6 participants