Synchronize video to audio when drop frames is enabled
ClosedPublic

Authored by jounip on Oct 9 2018, 9:54 AM.

Details

Test Plan

Tested on Linux and Windows with a file containing some very large frames. Without the patch, the audio would stutter or crackle as the frames were encountered. This approach seems to successfully remove issue.

Diff Detail

Repository
R37 Krita
Lint
Lint Skipped
Unit
Unit Tests Skipped
jounip created this revision.Oct 9 2018, 9:54 AM
Restricted Application added a project: Krita. · View Herald TranscriptOct 9 2018, 9:54 AM
jounip requested review of this revision.Oct 9 2018, 9:54 AM
rempt added inline comments.Oct 9 2018, 9:58 AM
libs/ui/canvas/kis_animation_player.cpp
465

When pushing, this needs to have a debug area or go.

jounip updated this revision to Diff 43277.Oct 10 2018, 9:45 AM
jounip edited the test plan for this revision. (Show Details)

Fixed the issues Dmitry found.

jounip marked an inline comment as done.Oct 10 2018, 9:46 AM
dkazakov requested changes to this revision.Oct 10 2018, 8:57 PM

Hi, @jounip!

There is still one small regression: when changing the playback speed or fps right during playback using mouse wheel there is a weird hiccup in the playback. Basically, it stops playing for a couple of seconds. Without the patch it works smoothly, just changes the speed of the clip smoothly :(

This revision now requires changes to proceed.Oct 10 2018, 8:57 PM
jounip updated this revision to Diff 43383.Oct 11 2018, 12:37 PM

Fixed a time-origin mismatch between using a timer vs. audio playback for timing. After this change I can no longer reproduce the regression.

jounip updated this revision to Diff 43406.Oct 11 2018, 4:52 PM

Re-upload of the diff. Previous patch file apparently did not include all the intended changes.

dkazakov requested changes to this revision.Oct 12 2018, 8:55 AM

Hi, @jounip!

There is still the same bug in the patch. I found at least one reason for that: you changed the meaning of 'normalizedPlaybackTime'. Previously it was normalized by the "expected frame update period" and now your implementation normalizes it by the total animation time. And the problem is that your new implementation doesn't take "playback speed" into account (previously, the speed was already taken int account in the update period). Therefore every change of the playback speed drives the code crazy.

So the current regressions:

  1. When changing playback speed slider while playback, the cursor jumps like crazy. Most of the time it tries to jump over the end of the selection and is returned to the beginning by the wrapping algorithm.
  1. When changing FPS slider, the playback stops for some time. If you change the slider to 1 or 2 FPS, then the playback basically stops completely.
  1. There is also a weird thing. If I add an audio channel to the image, GStreamer fails (with some internal assert in the console) and the playback becomes really slow. Without the patch, GStreamer also fails, but the playback is not delayed in any way. I think it might also be considered as a regression :(
This revision now requires changes to proceed.Oct 12 2018, 8:55 AM
jounip updated this revision to Diff 43459.Oct 12 2018, 1:09 PM

Changed to using frames as the fundamental unit instead of two different types of milliseconds. The logic is now hopefully clearer and more correct.

Adjusting FPS and playback speed seem to work as expected now.

jounip updated this revision to Diff 43462.Oct 12 2018, 1:45 PM

Remove an extraneous rounding.

1diff --git a/libs/ui/canvas/kis_animation_player.cpp b/libs/ui/canvas/kis_animation_player.cpp
2index 6be2cb4..0411f4f 100644
3--- a/libs/ui/canvas/kis_animation_player.cpp
4+++ b/libs/ui/canvas/kis_animation_player.cpp
5@@ -63,7 +63,7 @@ public:
6 dropFramesMode(true),
7 nextFrameExpectedTime(0),
8 expectedInterval(0),
9- expectedFrame(0),
10+ currentFrame(0),
11 lastTimerInterval(0),
12 lastPaintedFrame(0),
13 playbackStatisticsCompressor(1000, KisSignalCompressor::FIRST_INACTIVE),
14@@ -98,7 +98,7 @@ public:
15 QElapsedTimer playbackTime;
16 int nextFrameExpectedTime;
17 int expectedInterval;
18- int expectedFrame;
19+ int currentFrame;
20 int lastTimerInterval;
21 int lastPaintedFrame;
22
23@@ -321,10 +321,10 @@ void KisAnimationPlayer::slotUpdatePlaybackTimer()
24
25 const int fps = animation->framerate();
26
27- m_d->initialFrame = animation->currentUITime();
28+ m_d->initialFrame = isPlaying() ? m_d->currentFrame : animation->currentUITime();
29 m_d->firstFrame = playBackRange.start();
30 m_d->lastFrame = playBackRange.end();
31- m_d->expectedFrame = qBound(m_d->firstFrame, m_d->expectedFrame, m_d->lastFrame);
32+ m_d->currentFrame = qBound(m_d->firstFrame, m_d->currentFrame, m_d->lastFrame);
33
34
35 m_d->expectedInterval = m_d->framesToWalltime(1, fps);
36@@ -332,13 +332,18 @@ void KisAnimationPlayer::slotUpdatePlaybackTimer()
37
38 if (m_d->syncedAudio) {
39 m_d->syncedAudio->setSpeed(m_d->playbackSpeed);
40-
41- const qint64 expectedAudioTime = m_d->framesToMSec(m_d->expectedFrame, fps);
42+ qDebug() << "update audio speed";
43+ const qint64 expectedAudioTime = m_d->framesToMSec(m_d->currentFrame, fps);
44 if (qAbs(m_d->syncedAudio->position() - expectedAudioTime) > m_d->framesToMSec(1.5, fps)) {
45 m_d->syncedAudio->syncWithVideo(expectedAudioTime);
46+ qDebug() << "reset audio position";
47 }
48 }
49
50+ qDebug() << ppVar(fps)
51+ << ppVar(m_d->currentFrame)
52+ << ppVar(m_d->expectedInterval);
53+
54 m_d->timer->start(m_d->expectedInterval);
55
56 if (m_d->playbackTime.isValid()) {
57@@ -399,7 +404,7 @@ void KisAnimationPlayer::play()
58
59 m_d->playing = true;
60
61- m_d->expectedFrame = animation->currentUITime();
62+ m_d->currentFrame = animation->currentUITime();
63 slotUpdatePlaybackTimer();
64 m_d->lastPaintedFrame = -1;
65
66@@ -407,7 +412,7 @@ void KisAnimationPlayer::play()
67
68 if (m_d->syncedAudio) {
69 KisImageAnimationInterface *animationInterface = m_d->canvas->image()->animationInterface();
70- m_d->syncedAudio->play(m_d->framesToMSec(m_d->expectedFrame, animationInterface->framerate()));
71+ m_d->syncedAudio->play(m_d->framesToMSec(m_d->currentFrame, animationInterface->framerate()));
72 }
73
74 emit sigPlaybackStarted();
75@@ -474,24 +479,29 @@ void KisAnimationPlayer::uploadFrame(int frame, bool forceSyncAudio)
76 const bool syncToAudio = !forceSyncAudio && m_d->dropFramesMode && m_d->syncedAudio && m_d->syncedAudio->isPlaying();
77
78 if (frame < 0) {
79- const qreal currentTimeInFrames = syncToAudio ?
80+ qreal currentTimeInFrames = syncToAudio ?
81 m_d->msecToFrames(m_d->syncedAudio->position(), fps) :
82 m_d->playbackTimeInFrames(fps);
83
84+ const int previousFrame = m_d->currentFrame;
85+ const int expectedFrame = m_d->incFrame(previousFrame, 1);
86+ const int currentFrame = qFloor(currentTimeInFrames);
87+
88 // qDebug() << ppVar(framesDiff)
89 // << ppVar(m_d->expectedFrame)
90 // << ppVar(framesDiffNorm)
91 // << ppVar(m_d->lastTimerInterval);
92
93 if (m_d->dropFramesMode) {
94- const int currentFrame = qFloor(currentTimeInFrames);
95- const int framesToDrop = qMax(0, currentFrame - m_d->expectedFrame);
96- frame = m_d->incFrame(m_d->expectedFrame, framesToDrop);
97+ const int framesToDrop = qMax(0, currentFrame - expectedFrame);
98+ frame = m_d->incFrame(expectedFrame, framesToDrop);
99+ currentTimeInFrames += frame - currentFrame;
100+ qDebug() << "droppping " << framesToDrop;
101 } else {
102- frame = m_d->expectedFrame;
103+ frame = expectedFrame;
104 }
105
106- m_d->expectedFrame = m_d->incFrame(frame, 1);
107+ const int nextFrame = m_d->incFrame(frame, 1);
108
109 if (!m_d->dropFramesMode) {
110 const qint64 currentTime = m_d->playbackTime.elapsed();
111@@ -499,17 +509,23 @@ void KisAnimationPlayer::uploadFrame(int frame, bool forceSyncAudio)
112
113 m_d->nextFrameExpectedTime = currentTime + m_d->expectedInterval;
114 m_d->lastTimerInterval = qMax(0.0, m_d->lastTimerInterval - 0.5 * framesDiff);
115- } else if (m_d->expectedFrame >= frame) {
116- const int timeToNextFrame = m_d->framesToWalltime(m_d->expectedFrame - currentTimeInFrames, fps);
117+ } else if (nextFrame >= frame) {
118+ const int timeToNextFrame = m_d->framesToWalltime(nextFrame - currentTimeInFrames, fps);
119 m_d->lastTimerInterval = qMax(0, timeToNextFrame);
120 } else {
121 // Animation restarting
122 forceSyncAudio = true;
123 m_d->lastTimerInterval = m_d->expectedInterval;
124 }
125-
126- m_d->timer->start(m_d->lastTimerInterval);
127
128+ qDebug() << ppVar(currentTimeInFrames)
129+ << ppVar(m_d->currentFrame)
130+ << ppVar(fps)
131+ << ppVar(m_d->playbackSpeed)
132+ << ppVar(m_d->lastTimerInterval);
133+
134+ m_d->currentFrame = frame;
135+ m_d->timer->start(m_d->lastTimerInterval);
136 m_d->playbackStatisticsCompressor.start();
137 }
138

jounip updated this revision to Diff 43484.Oct 12 2018, 5:08 PM

Fix delays after adjusting FPS during playback

jounip updated this revision to Diff 43545.Oct 13 2018, 4:49 PM

Fix erratic jumps into middle of the playback range when reaching the end

jounip updated this revision to Diff 43550.Oct 13 2018, 6:12 PM

Simplified frame dropping logic

dkazakov accepted this revision.Oct 13 2018, 8:06 PM

Looks correct. There is still one small regression: the frame selection is reset after stopping the playback if one changed speed or FPS durng the playback at least once. But this problem can be fixed later on.

libs/ui/canvas/kis_animation_player.cpp
156

Check if we really need a precise timer, they are expensive

This revision is now accepted and ready to land.Oct 13 2018, 8:06 PM
This revision was automatically updated to reflect the committed changes.
jounip marked an inline comment as done.Oct 16 2018, 10:51 AM

Setting the timer precise does not seem to be necessary. I also fixed returning to the correct frame when stopping playback.