Uh oh!
There was an error while loading. Please reload this page.
Allow to override ffmpeg path and options - #41007
Conversation
Éloi Rivard (azmeuk)
commented
May 26, 2026
@microsoft-github-policy-service agree |
d3cf0da to
7cad89aCompare7cad89a to
7a34702Compare
Pavel Feldman (pavelfeldman)
left a comment
There was a problem hiding this comment.
I'd rather make it lower level and allow passing ffmpeg properties that would override our defaults.
Éloi Rivard (azmeuk)
commented
May 27, 2026
Thanks for the feedback. To make sure I'm building what you have in mind: would |
I'd try to mirror what we have for browser launching: https://playwright.dev/docs/api/class-browsertype#browser-type-launch-option-ignore-default-args |
Pavel Feldman (pavelfeldman)
commented
May 27, 2026
That works for me. |
Pavel Feldman (pavelfeldman)
commented
May 27, 2026
Browsers arsgs are generally used for additional args, which is not override-friendly. I'd use object notation as proposed - ignore-default-args is overall horrible. |
7a34702 to
70c48feCompareUpdated. So there are two options:
|
70c48fe to
e2a84afComparePavel Feldman (pavelfeldman)
commented
May 28, 2026
I love the custom `ffmpeg path option. We discussed it at the API review meeting and are thinking that we want to give you full control over the options (both input and output, even though we produce the input), for simplicity. So we are now thinking that { ffmpegExecutable: string, ffmpegOptions: string, fps: number, outputExtension: string } should be sufficient for you to produce any content desired. Sorry for going back and forth on this one. Wdyt? |
Éloi Rivard (azmeuk)
commented
May 28, 2026
Sounds good, thanks for the feedback. Give me a few days to work this out. |
e2a84af to
4cc4f95CompareÉloi Rivard (azmeuk)
commented
May 29, 2026
Done, and tested in the dowstream project: tushuhei/sphinxcontrib-screenshot#118 |
Test results for "MCP"7207 passed, 1113 skipped Merge workflow run. |
Test results for "tests 1"5 flaky44002 passed, 870 skipped Merge workflow run. |
We gave this another review as part of our release preparation, and decided against shipping this. The proposed public API around the ffmpeg input is very restrictive and makes it hard for Playwright to change its implementation, and we'd rather not lock this in. The Screencast API we shipped in Playwright 1.59 allows implementing this in userland, and after consideration this is what we recommend for your usecase. Essentially you can take our userland ffmpeg recordingimportjpegjsfrom'jpeg-js';importchild_processfrom'child_process';constfps=25;functionmonotonicTime(): number{returnMath.floor(performance.now()*1000)/1000;}classFfmpegVideoRecorder{private_size: {width: number,height: number};private_process: child_process.ChildProcess;private_lastWritePromise: Promise<void>=Promise.resolve();private_firstFrameTimestamp: number=0;private_lastFrame: {timestamp: number,frameNumber: number,buffer: Buffer}|null=null;private_lastWriteNodeTime: number=0;private_frameQueue: Buffer[]=[];private_isStopped=false;constructor(ffmpegPath: string,size: {width: number,height: number},outputFile: string){if(!outputFile.endsWith('.webm'))thrownewError('File must have .webm extension');this._size=size;constw=this._size.width;consth=this._size.height;constargs=`-loglevel error -f image2pipe -avioflags direct -fpsprobesize 0 -probesize 32 -analyzeduration 0 -c:v mjpeg -i pipe:0 -y -an -r ${fps} -c:v vp8 -qmin 0 -qmax 50 -crf 8 -deadline realtime -speed 8 -b:v 1M -threads 1 -vf pad=${w}:${h}:0:0:gray,crop=${w}:${h}:0:0 ${outputFile}`.split(' ');this._process=child_process.spawn(ffmpegPath,args,{stdio: 'pipe'});this._process.stdin!.on('finish',()=>{console.log('ffmpeg finished input.');});this._process.stdin!.on('error',()=>{console.log('ffmpeg error.');});}writeFrame(frame: Buffer,timestamp: number){this._writeFrame(frame,timestamp);}private_writeFrame(frame: Buffer,timestamp: number){if(this._isStopped)return;if(!this._firstFrameTimestamp)this._firstFrameTimestamp=timestamp;constframeNumber=Math.floor((timestamp-this._firstFrameTimestamp)*fps);if(this._lastFrame){constrepeatCount=frameNumber-this._lastFrame.frameNumber;for(leti=0;i<repeatCount;++i)this._frameQueue.push(this._lastFrame.buffer);this._lastWritePromise=this._lastWritePromise.then(()=>this._sendFrames());}this._lastFrame={buffer: frame, timestamp, frameNumber };this._lastWriteNodeTime=monotonicTime();}privateasync_sendFrames(){while(this._frameQueue.length)awaitthis._sendFrame(this._frameQueue.shift()!);}privateasync_sendFrame(frame: Buffer){returnnewPromise(f=>this._process!.stdin!.write(frame,f)).then(error=>{if(error)console.error('ffmpeg failed to write frame',error);});}async_stop(){// Only report the error on stop. This allows to make the constructor synchronous.if(this._isStopped)return;if(!this._lastFrame){// ffmpeg only creates a file upon some non-empty inputthis._writeFrame(createWhiteImage(this._size.width,this._size.height),monotonicTime());}// Pad with at least 1s of the last frame in the end for convenience.// This also ensures non-empty videos with 1 frame.constaddTime=Math.max((monotonicTime()-this._lastWriteNodeTime)/1000,1);this._writeFrame(Buffer.from([]),this._lastFrame!.timestamp+addTime);this._isStopped=true;try{awaitthis._lastWritePromise;this._process?.kill('SIGINT');}catch(e){console.error(e);}}}functioncreateWhiteImage(width: number,height: number): Buffer{constdata=Buffer.alloc(width*height*4,255);returnjpegjs.encode({ data, width, height },80).data;}constrecorder=newFfmpegVideoRecorder(outputFile,{width: 800,height: 600},'output.webm');page.screencast.start({onFrame: frame=>{// playwright we should probably expose frameSwapWillTime publically here.recorder.writeFrame(frame.data,Date.now()/1000);}});This can probably be simplified some more. As noted at the bottom, we should probably expose frameswap time in our public API to make this solid, i'll see if we can get this released in 1.60. Thanks for taking the time to work on this Pull request, and sorry that we have to reject it after going through multiple rounds! |
Oops. I misclicked, I did not want to open the PR here yet, just on my own fork to trigger the CI. I'll update the description soon with all the details.Sorry for that noise ☝️
I am a contributor of sphinxcontrib-screenshot, which is an extension that takes screenshots and screencasts for integration in Python sphinx documentations. I use it to automatically generate videos and demonstrate reactive behaviors in other apps documentations.
The current static quality settings generate some glitches in the videos, and too much blur. The end quality is not fitting professional documentation, so I opened this PR to add quality customization parameters that are passed to ffmpeg. I evoked this in #17217
This PR adds an optional field to
recordVideoto control ffmpeg encoding quality:mode: 'crf'— constant rate factor (constant visual quality, variable file size).valueis an integer between 0 (lossless) and 63 (worst).mode: 'bitrate'— target bitrate (variable visual quality, predictable file size).valueis in bits per second.When
qualityis omitted, the ffmpeg command line stays identical to today. The tests just check that the command run. I am not really sure how to check deterministically which quality parameters were passed from the output video.Let me know if the quality tweaking seems right but the implementation feel wrong, and I will update the PR.
Regards
related to to #22257