Buginator changed status from new to assigned

Buginator changed owner from `` to Buginator

florian@... commented

Movie (sound) plays but I just see a white square.

This is because my OpenGL does not support GL_ARB_texture_non_power_of_two which would be required to for showing a 320x240 texture.

I'll attach a patched ogg.c (extremely shit quick and dirty solution) to get it working...

Buginator commented

Thanks for testing Florian.

Here is a update for non NPOT cards... hope it works. :)
(unsure if I should keep both codepaths or not, but for testing, I am forcing GL_TEXTURE_RECTANGLE_ARB for everyone.)

If I may ask, what kind of hardware do you got anyway? I assume intel 945 chipset or something like that?

Buginator commented

I forgot to say, that hitting the main menu ** 'view intro**' button is the best thing to use to see the videos so far. Unless you want to play SP game a bit.

florian@... commented

Actually I've got a mobile ATI, this is what lspci says:
ATI Technologies Inc RV350 [Mobility Radeon 9600 M10]

I think GL_ARB_texture_non_power_of_two is purely a software issue, not a hardware issue and I am using the free software ATI driver. If I feel motivated enough I may try and look into the mesa drivers and DRI at some point to find out.

Buginator uploaded file FMV5c.patch (54.6 KiB)

Buginator commented

Updated once again to trunk, and this time, added FMVs to the intel screen.

For now, after you download the sequences, rename the Cam1/, Cam2/ Cam3/ folders to cam1, cam2, cam3.

Please report back any issues you may have.

(Plan is to have everything in squences in lowercase when patch is finally done. Some scripts will need to be change, to be lowercase, and have the correct .ogg extension as well.)

pabs3@... commented

Works nicely here (Debian GNU/Linux, Intel GPU). Only issue is that the resolution of the FMVs is fairly low. I'd suggest that in the long term, the FMVs will need to be replaced by something higher resolution.

pabs3@... commented

Some other things:

When switching windows, pausing the video isn't instant, takes about a second for the sound to stop.

Probably should strip Eidos and any other trademarks from the FMVs, unless the relicensing stuff gave you permission to use them.

pabs3@... commented

One other thing: it would be nice if warzone checked for the FMVs and skipped them and or remove relevant menu items when not present; some folks aren't going to want to download 160Mb of video to play warzone.

The intro video item should probably go to the top of the menu.

pabs3@... commented

One more thing; when an intel item automatically starts, afterwards it drops into the intel screen, I think it might be better to return to the game.

dmx commented

Seems to break on Xcode, but perhaps its something to do with the Theora dependency.

Not sure how to get the theora dependency working.

Buginator uploaded file FMV5d.patch (53.5 KiB)

Giel uploaded file improve-FMV5c.patch (26.6 KiB)

Improvements against attachment:FMV5c.patch

Giel uploaded file improve-FMV5d.patch (0.9 KiB)

Improvements against attachment:FMV5d.patch

Giel uploaded file FMV5e.patch (46.0 KiB)

The result of applying attachment:improve-FMV5c.patch to attachment:FMV5c.patch and attachment:improve-FMV5d.patch to attachment:FMV5d.patch

Giel uploaded file improve-FMV5e.patch (37.0 KiB)

Improvements against attachment:FMV5e.patch

Giel uploaded file FMV5f.patch (44.3 KiB)

The result of applying attachment:improve-FMV5e.patch to attachment:FMV5e.patch

Giel commented

Change log attachment:improve-FMV5c.patch:

• Remove commented out printf statements

• Remove whitespace from the end of lines

• Use iV_DrawTextF instead of snprintf into some temporary buffer combined with a call to iV_DrawText

• Remove code that was commented out at insertion time already

• Remove whitespace from the end of lines

• Use decent header guards that have a smaller chance of conflicting with system headers

• Use WZ_OS_WIN instead of WIN32 to detect a Windows system

• Use WZ_OS_UNIX instead of the absence of WIN32 to detect a Unix system

• Separate functions by a single empty line and nothing else

• Restore some removed empty lines

• Remove unused and relatively empty functions added by the FMV patch

• Use ssprintf instead of snprintf

• Use ssprintf's return value to determine whether string truncation has occurred rather than manually recreating the condition with strlen

• Make sure to use Theora's preprocessor flags when compiling lib/sequence

• Reorder the source lists alphabetically in lib/sequence/Makefile.am

• Const correctness of seq_PlayOgg

• Don't assume that the extension is held in the last 3 characters

• Use strrchr to strip the current extension

• Use sstrcat to add the new extension (".ogg")

• Rename rather stupidly named variable "kludge" (whoever used this

• deserves some slapping) to "realFilename"

• Reorder MSVC project file to reduce diff-size

• Some style fixes

• Remove unused macro RPL_FRAME_TIME

• Remove commented out variable time_started

Change log attachment:improve-FMV5d.patch:

• Separate functions by exactly one empty line
• Proper indenting and style of if-condition

Change log attachment:improve-FMV5e.patch:

• Get rid of the Windows specific timer code in favour of using gettimeofday() provided by [6062] and [6063]

• Use astyle to format the newly included sources

• Manually reformat the changed parts of existing sources

• Remove Eidos copyright, because Eidos has absolutely zero to do with code we write from scratch

• Set our copyright to 2008 (not 2005-2007) for new sources

• Fix a bug where we would increment the frame counter even accross different movie sequences

Buginator commented

The sequences have all been converted now, and are available on GNA.

Do we want to convert everything in that archive to lower case? Right now, only the directories Cam1, Cam2, and Cam3 need to be converted to cam1, cam2, and cam3, and it will play all the videos fine.

We do need to rename all the *.rpl to *.ogg though, and get rid of that hack in the codebase.

This change adds a Cut scene (FMV) size menu option.
(*defaults to fullscreen)

Choices are fullscreen or native.

For native, it just centers it on the screen.

We still need mac testing though.

Buginator uploaded file FMV5g.patch (23.0 KiB)

adds FMV size option in menu

Buginator commented

revert FMV5g.patch, I didn't see the old stuff before.
Sorry about that.

This one has 3 options, Fullscreen, 1x and 2x.

Buginator uploaded file FMV5h.patch (53.3 KiB)

Giel uploaded file improve-FMV5f.patch (1.4 KiB)

Improvements against attachment:FMV5f.patch

Giel uploaded file improve-FMV5h.patch (3.5 KiB)

Improvements against attachment:FMV5h.patch

Giel uploaded file FMV5i.patch (50.7 KiB)

The result of applying attachment:improve-FMV5f.patch to attachment:FMV5f.patch and attachment:improve-FMV5h.patch to attachment:FMV5h.patch

Giel commented

Changelog attachment:improve-FMV5f.patch:

• Put some comments on a line of their own

• Get rid of the addition to win32/Warzone2100.sln as it's not required and installation specific

Changelog attachment:improve-FMV5h.patch:

• Style fixes: use whitespace (in addition to commas) as separation

• Move FMV_FULLSCREEN and the "default" case to the bottom of the switch statements

• ASSERT that the default case for switch(war_GetFMVmode()) never happens (as it'd be a programmer error)

• Initialize variables x and y on declaration

Giel commented

NOTE: Some things that still need fixing

• Try to get rid of the huge amount of global variables in ogg.c (e.g. a more object oriented approach might be in order, by stuffing variables in a struct)
• Remove the "Ogg" prefixes from the functions, as it doesn't matter for client code what the container is

EvilGuru changed milestone from 2.1 to 2.2

Giel commented

Changelog attachment:improve-FMV6b.patch:

• Rename ogg.[ch] to sequence.[ch]

• Rename the sequence.h function interfaces to the previous sequence.h format

• Use proper indenting for "else if"s by keeping the "if" on the same line as the "else"

• Rename rint() to nearbyint() because the behaviour it implements is that of the latter (i.e. it doesn't raise the "inexact" exception)

• Only define nearbyint() when we're compiling without a C99 compliant compiler

• Add a Doxygen comment to nearbyint()

• Define nearbyint() statically

• Use SDL_Delay instead of platform dependent sleep code (that is wrong for POSIX systems)

• Lets not #include headers we don't need

• Mark all global variables as static

• Remove unused global variables

• Mark all functions that aren't used outside of sequence.c as static

• Remove unnecessary forward declarations of functions

• Get rid of "int a" in nearbyint()

• Don't print the datadir in the error message produced by seq_Play when PhysicsFS fails to open the given file

• Move nearbyint to lib/framework/math-help.h

• Use the function audio_Disabled() instead of the global variable openal_initialized to determine whether we should or shouldn't use audio

• Don't add the VCWebDeploymentTool to the MSVC project

• Replace usage of "BOOL" with "bool" and "FALSE" by "false"

Giel uploaded file improve-FMV6c.patch (17.9 KiB)

Improvements against attachment:FMV6c.patch

Giel uploaded file FMV6d.patch (53.8 KiB)

The result of applying attachment:improve-FMV6c.patch to attachment:FMV6c.patch

Giel commented

Changelog attachment:improve-FMV6c.patch:

• Recover needlessly removed whitespace

• Don't link in libtheora.lib (MSVC) from an absolute path

• Use an uppercase X in all instances of 1X

• Recover a deleted "break;" statement

• Remove several unused variables

• Initialize several variables at the point of declaration

• Proper indentation of several curly braces

• Remove some useless whitespace and recover somet other whitespace

• Get rid of seq_StartBufVideo which is basically a copy-paste of seq_StartFullScreenVideo

• Don't ASSERT that "audioName" is shorter than MAX_STR_LENGTH as that size doesn't matter

• Remove rather useless functions report_colorspace and dump_comments (they do nothing to aid in decoding)

• Reorder the public API functions of [browser:trunk/lib/sequence/sequence.c sequence.c] (i.e. the non-static ones) such that they appear in the same order as in its header ([browser:trunk/lib/sequence/sequence.h sequence.h])

Giel uploaded file improve-FMV6d.patch (3.3 KiB)

Improvements against attachment:FMV6d.patch

Giel uploaded file FMV6e.patch (163.2 KiB)

The result of applying attachment:improve-FMV6d.patch to attachment:FMV6d.patch

Giel commented

Changelog attachment:improve-FMV6d.patch:

• Change all references to *.rpl filename to lowercase references to *.ogg filenames

• Remove the code from seq_Play that changed the extension to .ogg

• Mention the added video playback support in the changelog

Giel commented

The Theora/Vorbis decoding code still needs a lot of clean up, but that can happen in the repository.

I consider attachment:FMV6e.patch ready for committing.

Buginator commented

Fix seq_StartFullScreenVideo() to handle intel vids correctly, since seq_StartBufVideo() was removed.

Other than that, looks good on this end.

Note, I didn't have time to test on linux.

If all works well for you, feel free to commit patch 6f.

sidenote, the bool ForceRes maybe better named as something else.
While we do Force a Resolution, we also use it to skip sub title text & loop control.

the miniFMV6f is just the changes I made since patch 6e, and FMV6f is everything including 6e.

Buginator uploaded file FMV6f.patch (159.8 KiB)

everything in FMV6e + my fixes for intel screen

Buginator uploaded file miniFMV6f.patch (10.5 KiB)

just the changes since FMV6e

Giel uploaded file improve-FMV6f.patch (12.6 KiB)

Improvements against attachment:FMV6f.patch

Giel uploaded file FMV6g.patch (167.2 KiB)

The result of applying attachment:improve-FMV6f.patch to attachment:FMV6f.patch

Giel commented

Changelog attachment:improve-FMV6f.patch:

• Join the "else" and accompanying "if" on the same line and fix indenting of the following block

• Get rid of variable "state" which is used as some sort of "delayed return"

• Change variables "frame" and "HOLD" to enums and use enumerants instead of magic numbers

• Because seq_RenderVideoToBuffer's ForceRes parameter is true always remove it

• Change seq_StartFullScreenVideo's boolean ForceRes parameter into a more descriptive enum and rename to "resolution"

• Split out the video resolution change code from seq_StartFullScreenVideo into a new function: seq_SetUserResolution

• Remove the OGG_ prefix from the functions that had it

• Add a note to do_timer_wait that their license isn't compatible with ours (i.e. it's non-free software)

• Add a FIXME to do_timer_wait saying it needs replacement to solve the license issue

• Add the required BSD license notice to sequence.c (which is derived from a Theora tutorial)

NOTE: We have a licensing issue with function do_timer_wait

The issue is documented in the code, but for clarity's sake lets quote it:

#!c
/**
 * Syncs the time if required by sleeping.
 *
 * @NOTE This function is taken from
 *       http://svn.icculus.org/d2x/trunk/main/mveplay.c?[1]=495&[2]=498&pathrev=498
 *       and modified to use our own timer implementation (timer.c) for
 *       measurement and SDL for sleeping (SDL_Delay). However, d2x's license
 *       is non-free and not compatible with the GPL.
 *
 * FIXME: Replace this function to solve the above license issue.
 */
static void do_timer_wait(void)

Buginator commented

Replying to [comment:28 Giel]:

• Remove the OGG_ prefix from the functions that had it
• Add a note to do_timer_wait that their license isn't compatible with ours (i.e. it's non-free software)
• Add a FIXME to do_timer_wait saying it needs replacement to solve the license issue

A few things, I removed the do_timer_wait(), with no adverse side effects.
I tried this on my main machine, and my slower linux one, and it never hit the sleep call (set a breakpoint for verification). Which means that timer_expire's value is small. (It was a negative value all the times I checked) which means that videobuf_time is very fast (very small value) per frame. I think we would need the sleep only on really slow machines, something that can't decode a 320x240 frame very fast.

If we opt to leave in the sleep, I did find that in liboggplay's glut-player.c example, ( http://trac.annodex.net/browser/liboggplay/trunk/src/examples/glut-player.c?rev=3685#L242
) has code that is close to what we do, and their nanosleep(). (which is Sleep() on windows.)

I then inlined the code, so it looks like:

	if (videobuf_ready)
	{
		int tv, ts;

		getRelativeTime();		// make sure timer_expire has valid value
		tv = Timer_getElapsedMicroSecs();

		if (tv <= timer_expire)
		{
			ts = timer_expire - tv;
			SDL_Delay(ts/1000);
		}

		timer_expire += ((videobuf_time - getRelativeTime()) * 1000000.0);
	}

There is no change from before, I never hit the sleep call...

I say nuke it, and if we have issues, then we can come back to this, and do the workaround.

Giel commented

I can confirm that that code never gets executed. I should have thought of that earlier actually as I've already examined that piece of code for this:

• Use SDL_Delay instead of platform dependent sleep code (that is wrong for POSIX systems)

Was a problem with using sleep(3) and passing it milli-seconds on non-Windows systems. Even though sleep(3) has seconds-resolution.

Thus I suggest the same as you do: just dump the do_timer_wait function entirely...

If no one objects I think we should commit this tomorrow.

Giel uploaded file improve-FMV6g.patch (1.2 KiB)

Improvements against attachment:FMV6g.patch

Giel uploaded file FMV7.patch (166.4 KiB)

The result of applying attachment:improve-FMV6g.patch to attachment:FMV6g.patch

Giel changed status from assigned to accepted

Giel changed owner from Buginator to Giel

Giel changed status from accepted to closed

Giel changed resolution from `` to fixed

Giel commented

Applied in [6119].

DevUrandom changed component from other to Engine: Movie

DevUrandom changed blocking which not transferred by tractive

DevUrandom changed blockedby which not transferred by tractive

Buginator removed milestone (was 2.2)

Buginator commented

Milestone 2.2 deleted

Per Inge Mathisen <per.mathisen@...> commented

In Warzone2100/warzone2100@af90044:

#CommitTicketReference repository="" revision="af90044b38ba60ce68a258c0b9e1bbe269157aa7"
Merge pull request #64 from ckorn/linker_error

Reorder libs to fix linker error