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

Fix timing of AVI files dumped on Linux #1345

Merged
merged 1 commit into from Oct 28, 2014

Conversation

sgadrat
Copy link

@sgadrat sgadrat commented Oct 20, 2014

Scenario

  • Uses a Linux build of Dolphin-Emu.
  • Emulate a game that runs at 30 FPS instead of the targetted 60 FPS.
  • Start dumping frames.
  • Stop dumping frames after some times.
  • Play the dumped AVI file with a media player.

Expected result

  • The video is played at 30 FPS

Actual result

  • The video is played at 60 FPS, making it accelerated.

The patch

The timing information is set on s_scaled_frame->pts, giving precise timing information to the encoder. Frames arriving too early (less than one tick after the previous frame) are droped.

The setting of packet's timestamps and flags is done after the call to avcodec_encode_video2()
as this function resets these fields according to its documentation.

@@ -335,6 +336,8 @@ static int s_height;
static int s_size;
static u64 s_last_frame;
bool b_start_dumping = false;
static Common::Timer s_timer;
static int64_t s_last_pts;

This comment was marked as off-topic.

@RisingFog
Copy link
Member

Does this handle save states and dumping in the middle of emulation?

@sgadrat
Copy link
Author

sgadrat commented Oct 20, 2014

Dumping in the middle of emulation works.

About save states. I tried this scenario :

  • Start emulation
  • Start frame dumping
  • Save the state
  • Wait a little then load the saved state
  • Stop frame dumping

The resulting AVI contains the video with as output by the emulator. We notice only the time at wich we loaded the state, as de game returns to the past. It seems to be the normal behaviour to me. Do you expect somthing else ?

@RisingFog
Copy link
Member

That's the expected output. How about games like Crazy Taxi and Super
Monkey Ball? Do those games provide a proper output with regular dumping?

On Mon, Oct 20, 2014 at 9:19 AM, sgadrat notifications@github.com wrote:

Dumping in the middle of emulation works.

About save states. I tried this scenario :

  • Start emulation
  • Start frame dumping
  • Save the state
  • Wait a little then load the saved state
  • Stop frame dumping

The resulting AVI contains the video with as output by the emulator. We
notice only the time at wich we loaded the state, as de game returns to the
past. It seems to be the normal behaviour to me. Do you expect somthing
else ?


Reply to this email directly or view it on GitHub
#1345 (comment).

@sgadrat
Copy link
Author

sgadrat commented Oct 20, 2014

I will test with these games as soon as I get my hands on it. Before testing I can only say that the patch should be resilient irregular framerates. The point of the patch is to timestamp frames to be encoded by watching the wallclock when a frame is output, making it resilient to framerate variance being due to slowdowns in emulation or naturally irregular.

Note that I considered that these games provide irregular framerate by reading your comment, but I may be wrong. Is there any specific trap I should considere when testing these games ?

@JMC47
Copy link
Contributor

JMC47 commented Oct 20, 2014

The Spyro: Enter the Dragonfly game likes to jump between 20, 30 and 60 fps. That game is scary for video dumping.

@RisingFog
Copy link
Member

Those games have a splash screen at the beginning where it's actually just
a single frame which then is held there by a VI interrupt (same with now
loading, etc. for crazy taxi)

On Mon, Oct 20, 2014 at 9:54 AM, sgadrat notifications@github.com wrote:

I will test with these games as soon as I get my hands on it. Before
testing I can only say that the patch should be resilient irregular
framerates. The point of the patch is to timestamp frames to be encoded by
watching the wallclock when a frame is output, making it resilient to
framerate variance being due to slowdowns in emulation or naturally
irregular.

Note that I considered that this games provide irregular framerate by
reading your comment, but I may be wrong. Is there any specific trap I
should considere when testing these games ?


Reply to this email directly or view it on GitHub
#1345 (comment).

@sgadrat
Copy link
Author

sgadrat commented Oct 20, 2014

Tested crazy taxi and spyro.

About crazy taxi, the "now loading" lonely frame is held on screen by the player an amount of time that seems correct. Tested with VLC and ffplay.

About Spyro, I had some slowdowns during kinematics in the emulator itself so it is repercuted in the video file. But the consistent ingame framerate changing is unnoticeable on the video file.

@@ -311,6 +311,7 @@ bool AVIDump::SetVideoFormat()
#include "Common/FileUtil.h"
#include "Common/StringUtil.h"
#include "Common/Logging/Log.h"
#include "Common/Timer.h"

This comment was marked as off-topic.

@lioncash
Copy link
Member

Can you squash two change commits back into the original one?

@RisingFog
Copy link
Member

@dolphin-emu-bot rebuild

@RisingFog
Copy link
Member

EDIT: nvm, read below

@RachelBryk
Copy link
Member

Nope, this is totally wrong behavior. Frames need to be dumped based on emulated time, not host time.

@RisingFog
Copy link
Member

Ah didn't notice that. Thanks for pointing that out. If you could base it
off coretiming tick counts that would be a lot better
On Oct 21, 2014 6:32 PM, "RachelBryk" notifications@github.com wrote:

Nope, this is totally wrong behavior. Frames need to be dumped based on
emulated time, not host time.


Reply to this email directly or view it on GitHub
#1345 (comment).

@sgadrat
Copy link
Author

sgadrat commented Oct 22, 2014

Here is the patch for dumping frames based on emulated time. It is indeed a better idea than using host system as the video should be smooth event when the emulator slow down.

I have to some questions about the patch itself. I reuse s_last_frame that where effectively unused while apparently there for this kkind of synchronization. Is it a good idea ? And s_last_pts could be removed and recomputed for each frame. Do you prefere with the s_last_pts variable or recompute it ?

@sgadrat
Copy link
Author

sgadrat commented Oct 22, 2014

Oops did not saw that "AVIDump::DoState()" was shared between Linux and Windows.

@sgadrat
Copy link
Author

sgadrat commented Oct 22, 2014

Hmmm with this patch, the video playback is accelerated.

@RisingFog
Copy link
Member

@sgadrat in what way?

else
s_scaled_frame->pts = 0;
s_scaled_frame->pts += (delta * s_stream->codec->time_base.den) / SystemTimers::GetTicksPerSecond();
s_last_frame += delta;

This comment was marked as off-topic.

This comment was marked as off-topic.

@sgadrat
Copy link
Author

sgadrat commented Oct 22, 2014

@RisingFog It was due to the use of a little delta converted in 1/60th of a second then basing the next computing on the imprecise value. It is fixed int the last patch.

int error = avcodec_encode_video2(s_stream->codec, &pkt, s_scaled_frame, &got_packet);
int got_packet = 0;
int error = 0;
u64 delta = CoreTiming::GetTicks() - s_last_frame;

This comment was marked as off-topic.

@RisingFog
Copy link
Member

I noticed some audio sync issues with some Wii/VC titles (similar to the issues I ran into initially with frame dumping). Hopefully those changes I suggested should be able to fix those issues.

Also once the changes are tested and committed, combine all the commits into a single commit.

The timing information is set on s_scaled_frame->pts, giving precise
timing information to the encoder. Frames arriving too early (less than
one tick after the previous frame) are droped. The setting of packet's
timestamps and flags is done after the call to avcodec_encode_video2()
as this function resets these fields according to its documentation.
@sgadrat
Copy link
Author

sgadrat commented Oct 23, 2014

I blindly fixed the points you commented as I am currently unable to test this Audio/Video sync issue. Is the reproduction scenario something like that ?

  • Check "Dump Frames" and "Dump audio" from "Movie" menu
  • Start the emulation of a Wii title [1]
  • Play the game making noises with obvious timing (like shot with a gun)
  • Stop the emulation
  • Merge the audio and video file
  • Play the resulting file watching for A/V synchronization

[1] Which games in particular may be good candidates ?

@RisingFog
Copy link
Member

Yes. Good examples include Super Mario Galaxy (1 & 2) and Doc Louis Punch
Out.

On Thu, Oct 23, 2014 at 5:43 PM, Sylvain Gadrat notifications@github.com
wrote:

I blindly fixed the points you commented as I am currently unable to test
this Audio/Video sync issue. Is the reproduction scenario something like
that ?

  • Check "Dump Frames" and "Dump audio" from "Movie" menu
  • Start the emulation of a Wii title [1]
  • Play the game making noises with obvious timing (like shot with a
    gun)
  • Stop the emulation
  • Merge the audio and video file
  • Play the resulting file watching for A/V synchronization

[1] Which games in particular may be good candidates ?


Reply to this email directly or view it on GitHub
#1345 (comment).

@RisingFog
Copy link
Member

@dolphin-emu-bot rebuild

@RisingFog
Copy link
Member

lgtm

skidau added a commit that referenced this pull request Oct 28, 2014
Fix timing of AVI files dumped on Linux
@skidau skidau merged commit b13ba06 into dolphin-emu:master Oct 28, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
6 participants