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

Make extracted ffmpeg executable #361

Closed
wants to merge 2 commits into from
Closed

Make extracted ffmpeg executable #361

wants to merge 2 commits into from

Conversation

jrb0001
Copy link

@jrb0001 jrb0001 commented Feb 11, 2018

This also makes ./gradlew executable for easier building on linux.

@itdelatrisu
Copy link
Owner

Can you explain / can anyone else confirm? No one has reported this to be an issue and the feature has been out for a long time now.

@jrb0001
Copy link
Author

jrb0001 commented Feb 11, 2018

Opsu failed to play background videos and the log showed stacktraces caused by the ffmpeg file not being executable.

@tpenguinltg
Copy link
Contributor

tpenguinltg commented Feb 12, 2018

Confirmed. I probably haven't noticed because I build using the system FFmpeg and turn videos off.

However, this patch isn't enough to fix the problem. The patch only covers the case when you're running the JAR and natives get extracted into the Natives directory, but not when running using gradle and there's already a build/natives directory present. The maven build (with target/natives) is probably affected, too but I've never been successful in building using maven (it's probably my setup).

Sun Feb 11 18:53:02 EST 2018 ERROR:Cannot run program "/scratch/opsu/./build/natives/ffmpeg64": error=13, Permission denied
java.io.IOException: Cannot run program "/scratch/opsu/./build/natives/ffmpeg64": error=13, Permission denied
        at java.lang.ProcessBuilder.start(ProcessBuilder.java:1048)
        at itdelatrisu.opsu.video.FFmpeg.extractMetadata(FFmpeg.java:97)
        at itdelatrisu.opsu.video.Video.<init>(Video.java:80)
        at itdelatrisu.opsu.states.Game.loadVideo(Game.java:1976)
        at itdelatrisu.opsu.states.Game.enter(Game.java:1654)
        at org.newdawn.slick.state.StateBasedGame.update(StateBasedGame.java:248)
        at org.newdawn.slick.GameContainer.updateAndRender(GameContainer.java:702)
        at itdelatrisu.opsu.Container.gameLoop(Container.java:136)
        at itdelatrisu.opsu.Container.start(Container.java:80)
        at itdelatrisu.opsu.Opsu.main(Opsu.java:213)
Caused by: java.io.IOException: error=13, Permission denied
        at java.lang.UNIXProcess.forkAndExec(Native Method)
        at java.lang.UNIXProcess.<init>(UNIXProcess.java:247)
        at java.lang.ProcessImpl.start(ProcessImpl.java:134)
        at java.lang.ProcessBuilder.start(ProcessBuilder.java:1029)
        ... 9 more

@itdelatrisu
Copy link
Owner

Is this solved by changing getNativeLocation() in FFmpeg.java to this?

private static File getNativeLocation() {
	File customLocation = Options.getFFmpegLocation();
	File f = (customLocation != null && customLocation.isFile()) ? customLocation : FFMPEG_PATH;
	if (!Files.isExecutable(f.toPath())) {
		f.setExecutable(true);
	}
	return f;
}

@tpenguinltg
Copy link
Contributor

@itdelatrisu Yes, it appears so. I'm not so sure about stuffing it in that function as it would result in too many concerns, but I guess it's fine as long as you're okay with the side effect.

@itdelatrisu
Copy link
Owner

Committed in cd1cdf8, thanks!

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.

3 participants