mickelson / attract

A graphical front-end for command line emulators that hides the underlying operating system and is intended to be controlled with a joystick or gamepad.
http://attractmode.org
GNU General Public License v3.0
397 stars 115 forks source link

Fixed audio out of sync when playing videos #549

Open oomek opened 5 years ago

oomek commented 5 years ago

Also in certain conditions, especially on slower hardware, when there is nothing to update on the screen and only videos are playing the framerate decreases significantly. Fixed by forcing screen update when at least one video is playing.

eulogy37 commented 5 years ago

I didn’t want to wait for this to be merged into master, so I cloned the latest release to my RPi 3B and manually made the edits to media.cpp and (at least for me) the desync issue persists. I’m using an older image, so I’m going to try one more time after I update everything else to current versions (ffmpeg etc) and ill give it another go.

oomek commented 5 years ago

I've checked it on the PC and the audio is now properly synced. Maybe RPI is just too slow to decode 640x480 at 60fps

eulogy37 commented 5 years ago

Attract Mode 2.4.0-10 (built by checking out commit 793c2f8) on the same hardware and image does not have the desync issue. Higher res and FPS can be choppy because it’s RPi, but no desync.

eulogy37 commented 5 years ago

I did go through the exercise of updating all packages and FFmpeg to their latest versions last night, as I said I would try above, and the issue persists. Is there anything I can provide that would be helpful?

jhdgghost commented 5 years ago

Only confirm that in a rpi3b+ retropie 4.4 i have the same desync issue. I test the branch fix-av-sync with commit 02609a7 and the issue continues. Test Attract Mode 2.4.0-10 (built by checking out commit 793c2f8) as eulogy37 comments and no desync. Also i detect that some videos reloops before finish surely due audio delay. Also available if there is something i can test to help.

oomek commented 5 years ago

Could you share the example video that's not syncing with audio please?

jhdgghost commented 5 years ago

This video shows clear desync in 2.5.1 but no in 2.40

https://drive.google.com/open?id=1nMHGIJcxTspnaICjeK5-l-TECMOjnM2x

oomek commented 5 years ago

I meant the source mp4 file

Mah0uPh0x commented 5 years ago

It looks likes you can download the mp4 file from that google drive link. The download button is on the top right, on my end.

oomek commented 5 years ago

Thanks. This is a tricky one. The container shows

MPEG4 Video (H264) 1280x720 239.998fps 870kbps [V: h264 main L3.1, yuv420p, 1280x720, 870 kb/s]

The stream shows:

Frame rate                     : 30.000 FPS
Minimum frame rate             : 26.316 FPS
Maximum frame rate             : 34.884 FPS

I'll see what I can do, but can't promise any timeframe as I'm trying to solve those issues another way. I'm working now on adding a native OpenMAX decoder which according to my tests is able to play 720p60 streams with just 2% CPU usage on the Raspberry PI.

jhdgghost commented 5 years ago

Hi oomek. I see you add new changes in the fix-av-sync branch. That should fix de desync issue? If you want i can test to check if the problem is fixed. What is the command to clone the fix-av-sync with the latest commits to rpi?

oomek commented 5 years ago

I've just rebased the commit. This will not fix AV synchronisation issues on the RPI when there is not enough CPU power to handle the decoding. Current implementation takes over 100% of the cpu for 720p streams. The new decoder I'm working on should fix it. According to my tests the CPU usage went down to < 8%. Unfortunately it's taking longer than I anticipated to implement it.

jhdgghost commented 5 years ago

Ok. Thanks for the update and your work.

meowthed commented 5 years ago

any updates on this?

oomek commented 5 years ago

My fork has it merged so you can try if you have means to build it yourself.

mickelson commented 4 years ago

Hi is there still an issue with this? is it confirmed to fix the desync?

oomek commented 4 years ago

It does fix the desync, but I'm not a fan of that fixed hardcoded 8ms offset. We should rely on the refresh rate of the monitor which SFML does not expose. This feature became stalled on the sfml forum so I was wondering if we could implement it ourself. It would require an independent implementation for each platform. Having a refresh rate available from within the squirrel script would also be very beneficial. Shall we go for it?

oomek commented 4 years ago

Also part of the code is irrelevant as forced update is covered by the power saving commit. We should keep this opened until we figure out what to do with the refresh rate getter.

mickelson commented 4 years ago

It does fix the desync, but I'm not a fan of that fixed hardcoded 8ms offset. We should rely on the refresh rate of the monitor which SFML does not expose. This feature became stalled on the sfml forum so I was wondering if we could implement it ourself. It would require an independent implementation for each platform. Having a refresh rate available from within the squirrel script would also be very beneficial. Shall we go for it?

I'm not a fan of implementing ourselves, it would have to be implemented for all the various systems that are supported. That seems like a lot of work to address something that is imperceptible to most people (well me at least) if we are talking about timeframes in the ~8ms range

oomek commented 4 years ago

This delay should be around half the frame time, so for 144hz displays it's way too much. I really wish we could implement the refresh getter it would finally solve the frame based animations as some people prefer it over rtc based ones which are offen stuttering. for pi and normal linux it's dead easy, for windows also, Not sure about other platforms. Besides, FPS Monitor assumes 60Hz and it will show wrong results for other refresh rates. I can't fix it until I get the refresh rate of the monitor.

There is one potential route of solving it without implementing platform specific code though. It would require around 1/3 sec delay on AM start. We could just count first 20 frames and determine the refresh rate based on the frame time ( skipping the first 3 frames due to the frame queue filling which would give close to 0 results ) What do you think about that approach?

oomek commented 4 years ago

I will finally be able to push forward this PR as soon as the new refresh rate detection is merged. Needs one line change still.