Chromium video duration fix #10145
Chromium video duration fix #10145
Conversation
Drop excessive screencast frames coming from browser to comply with declared frame rate. Keep dropped frame for future use as the most current one. Fixes microsoft#9611
Round last frame timestamp to multiply of interval calculated from declared frames per second. Fixes microsoft#9611
Account for first screencast frame delay send by browser after start screencast request. Fixes microsoft#9611
| assert(this._process); | ||
| if (this._isStopped) | ||
| return; | ||
| if (timestamp - this._lastFrameTimestamp < 1 / fps) { |
There was a problem hiding this comment.
With the new backpressure mechanism added in #9893 this should not be necessary. If the throttler doesn't give desired effect we should adjust its parameters instead. The difference with this approach is that Chromium won't send new screencast frames until it gets acks for already sent once whereas with the approach in this PR the frames are still generated just to be dropped on the floor on the playwright end.
Can you check if the problem is still reproducible on @next version (which includes the fix mentioned above)?
| for (let i = 0; i < repeatCount; ++i) | ||
| this._frameQueue.push(this._lastFrameBuffer); | ||
| this._lastWritePromise = this._lastWritePromise.then(() => this._sendFrames()); | ||
| this._lastFrameTimestamp += repeatCount / fps; |
There was a problem hiding this comment.
This shouldn't make any difference as the number of frames we feed into ffmpeg stays the same or am I missing something?
| this._lastWritePromise = this._lastWritePromise.then(() => this._sendFrames()); | ||
| this._lastFrameTimestamp += repeatCount / fps; | ||
| } else { | ||
| this._lastFrameTimestamp = timestamp - (monotonicTime() - this._launchTimestamp) / 1000; |
There was a problem hiding this comment.
This will only increase the duration of the resulting video by the time it took to launch screencast, why do we want this?
|
Closing since it looks stale and as part of the triage process! Feel free to re-open / create a new PR if you are still interested to work on it. |
3 participants
Videos recorded with playwright using chromium engine are longer than what could be expected by measuring execution time. It is caused by more than one problem:
Fixes #9611