fix(screencast):Unstable video frame rate due to accumulated timestam… #35777
Conversation
Test results for "tests 1"17 failed 9 flaky39065 passed, 809 skipped Merge workflow run. |
@microsoft-github-policy-service agree [company="Tencent"] |
@microsoft-github-policy-service agree company="Tencent" |
|
|
||
| const expectedFrames = Math.round(fps * (timestamp - this._firstFrameTimestamp)); | ||
|
|
||
| const repeatCount = Math.max(0, expectedFrames - this._framesWritten); |
There was a problem hiding this comment.
It should be max(1,.. to not miss the frame.
| this._isStopped = true; | ||
|
|
||
| if (this._firstFrameTimestamp === null || !this._lastFrameBuffer) { | ||
| if (this._gracefullyClose) |
There was a problem hiding this comment.
The callback should never be null at this point. We didn't have this check before, why do you need it now?
|
|
||
| const expectedFrames = Math.round(fps * (finalTimestamp - this._firstFrameTimestamp)); | ||
|
|
||
| const repeatCount = Math.max(0, expectedFrames - this._framesWritten); |
There was a problem hiding this comment.
Please don't lose frames, max(1,...
| } | ||
|
|
||
| if (repeatCount > 0) |
There was a problem hiding this comment.
This should stay under the if above, as that's where the queue is populated.
| const repeatCount = Math.max(0, expectedFrames - this._framesWritten); | ||
|
|
||
| for (let i = 0; i < repeatCount; ++i) | ||
| this._frameQueue.push(this._lastFrameBuffer); |
There was a problem hiding this comment.
This entire enqueue logic is repeated twice and should be in a shared method.
…p (#35776)