fix: Adjust ffmpeg CLI args for performance #19983

BlueWinds · 2022-01-31T17:58:53Z

User facing changelog

Speeds up video recording on low-resourced machines (such as most CI docker containers).

Additional details

ffmpeg is now limited to a single thread on machines with 2 or fewer CPUs, reducing resource contention with the main cypress project. Also adjusted ffmpeg probes to reduce buffering, especially impactful on short videos.

How has the user experience changed?

No change. Video output remains unchanged in quality and filesize.

For Cypress' driver integration tests, this appears to save about 20 seconds for each container, from

to

Exact time saved depends on branch, test, load, and the phase of the moon, but seems to be generally improved.

PR Tasks

[N/A] Have tests been added/updated? Existing tests cover video recording functionality.
[N/A] Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
[N/A] Has a PR for user-facing changes been opened in cypress-documentation?
[N/A] Have API changes been updated in the type definitions?
[N/A] Have new configuration options been added to the cypress.schema.json?

cypress-bot · 2022-01-31T17:58:55Z

Thanks for taking the time to open a PR!

Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

cypress · 2022-01-31T18:09:10Z

Test summary

4355 • 0 • 51 • 0 • 0

Run details


Project	cypress
Status	Passed
Commit	`0ba6c8c`
Started	Jan 31, 2022 10:28 PM
Ended	Jan 31, 2022 10:38 PM
Duration	09:56 💡
OS	Linux Debian - 10.10
Browser	Chrome 97

View run in Cypress Dashboard ➡️

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

jennifer-shehane · 2022-01-31T19:33:30Z

packages/server/lib/video_capture.ts

  return new Bluebird((resolve, reject) => {
    debug('processing video from %s to %s video compression %o',
      name, cname, videoCompression)

    const command = ffmpeg()
+    .addOptions([


This could use some more documentation. It may be a lot of cover in a code comment, but it would be nice to not have an engineer re-learn every single flag a year from now when they look at this code. What would you suggest in terms of documentation for this?

I've added a bunch of comments to all these, will push the changes in just a sec.

😍 😍 😍

packages/server/lib/video_capture.ts

tgriesser · 2022-02-01T00:28:06Z

Do we have a set of benchmarks logged/measured anywhere of before/after impact of the different flags, and what the environments/situations where they're perceived to cause the most impact?

BlueWinds · 2022-02-01T18:57:55Z

Do we have a set of benchmarks logged/measured anywhere of before/after impact of the different flags, and what the environments/situations where they're perceived to cause the most impact?

We do not have anything like that at the moment. The short version is "it seems faster, tests run a little bit faster in CI compared to a couple of develop branch builds I checked" is both all I have.

The longer version is that those sorts of benchmarks start to run into the sorts of things we've discussed a couple of times, where getting good baselines is hard - you have to have extremely consistent environments, or lots of data, and even then analysis is difficult. This is the sort of answer I'd like to be able to pull out of honeycomb in the future, but we're not there yet.

flotwig

Nice. The artifacts from the system tests on this look good too.

* develop: chore: fix cypress npm package artifact upload path (#20023) chore(driver): move cy.within logic into it's own file (#20036) chore: update automerge workflows (#19982) fix(selectFile): use target window's File/DataTransfer classes (#20003) chore: Update Chrome (stable) to 98.0.4758.80 and Chrome (beta) to 98.0.4758.80 (#19995) fix: Adjust ffmpeg CLI args for performance (#19983) build: allow unified to run cypress on Apple Silicon (arm64) (backport #19067 to 9.x) (#19968)

* 10.0-release: chore: make error more informative about migration to Cypress 10 (#20007) fix: remove shelljs in example build script, correct file renames fix: last step in the ct setup goes back to start (#20030) fix: typescript default plugin file (#20046) chore: fix cypress npm package artifact upload path (#20023) chore(driver): move cy.within logic into it's own file (#20036) chore: update automerge workflows (#19982) fix(selectFile): use target window's File/DataTransfer classes (#20003) chore: Update Chrome (stable) to 98.0.4758.80 and Chrome (beta) to 98.0.4758.80 (#19995) fix: Adjust ffmpeg CLI args for performance (#19983) build: allow unified to run cypress on Apple Silicon (arm64) (backport #19067 to 9.x) (#19968) release 9.4.1 [skip ci] fix: trigger 9.4.1 build release 9.4.0 [skip ci]

* chore: rename errors.js -> errors.ts * refactor: type safety on errors * refactor: add err_template for consistent error formatting * fix a few system tests * fix tests; update snapshots * Fix types * normalize snapshot - remove chalk ansi colors * more unit test fixes * more system test fixes * circleci build * backtick always in stdout, fix error formatting and failing snapshots * refactor: create @packages/errors * fix import * fix import * fixing build / tests * remove extraneous file * move warnIfExplicitCiBuildId * fix build / tests * Fix * error, type fixes, documentation, standardize child process error serialization * fix import * build errors on install * wrote specs generating visual images of all errors * remove unused dep * sanitize stack traces * add image diffing - if base images don't exist, create them - if base images don't match and local, overwrite them, if in CI throw - if base images are stale and local, delete them, if in CI throw * remove Courier New + MesloLGS NF font * type fixes, remove Bluebird, general cleanup * TS Cleanup * skip typecheck on tests for now * yarn.lock * fix @types/chai version * fix yarn.lock * Different version of mocha types so it isnt patched * errors spec snapshot * CI fix * fixes * store snapshot images in circle artifacts * dont change artifact destination prefix * use Courier Prime * antialias the text * decrease pixelmatch threshold, fail in CI only when changed pixels > 100 * increase timeout * overflow: hidden, remove new Promise, add debug logging Co-Authored-By: Tim Griesser <tgriesser@gmail.com> * run unit tests w/ concurrency=1 * unique window per file * disable app hardware acceleration + use in process gpu + single process * do not do image diffing - conditionally convert html to images - store html snapshots - do not store images in git * store snapshot html * Merge branch 'tgriesser/chore/refactor-errors' of https://github.com/cypress-io/cypress into tgriesser/chore/refactor-errors * remove concurrency * fix assertion * fixing ci * Link in readme * pass the browsers to listItems * fix: build @packages/errors in CI, defer import to prevent errors locally * Merge branch 'develop' into tgriesser/chore/refactor-errors * develop: chore: fix cypress npm package artifact upload path (#20023) chore(driver): move cy.within logic into it's own file (#20036) chore: update automerge workflows (#19982) fix(selectFile): use target window's File/DataTransfer classes (#20003) chore: Update Chrome (stable) to 98.0.4758.80 and Chrome (beta) to 98.0.4758.80 (#19995) fix: Adjust ffmpeg CLI args for performance (#19983) build: allow unified to run cypress on Apple Silicon (arm64) (backport #19067 to 9.x) (#19968) * fix run-if-ci.sh * remove dead code * Mark the .html files as generated in gitattributes * fix running single error case, slice out more of the brittle stack * remove additional brittle stack line * firefox web security error * nest inside of describe * reformat and redesign errors * more error cleanup and standardization * additional formatting of errors, code cleanup, refactoring * update ansi colors to match terminal colors * cleanup remaining loose ends, update several errors, compact excess formatters * fix types * additional formatting, remove TODO's, ensure no [object Object] in output * add test for 412 server response on invalid schema * update unknown dashboard error on creating run * use fs.access instead of fs.stat for perf * added PLUGINS_FILE_NOT_FOUND error - separated out from PLUGINS_FILE_ERROR - add system tests for both cases - update snapshots - remove stack trace from PLUGINS_FILE_NOT_FOUND fka PLUGINS_FILE_ERROR * add plugins system test around plugins synchronously throwing on require * remove forcing process.cwd() to be packages/server, update affected code - this was a long needed hangover from very old code that was doing unnecessary things due to respawning electron from node and handling various entrypoints into booting cypress - this also makes the root yarn dev and dev-debug work correctly because options.cwd is set correctly now * perf: lazy load chalk * remove excessive line since the file exists but is invalid * fix types * add system test when plugins function throws a synchronous error * create new PLUGINS_INVALID_EVENT_ERROR, wire it up correctly for error template use - properly pass error instance from child to ensure proper user stack frames - move error display code into the right place * only show a single stack trace, either originalError or internal cypressError * push error html snapshots * fix tests, types * fix test * fix failing tests * fix tests * fixes lots of broken tests * more test fixes * fixes more tests * fix type checking * wip: consistent handling of interpolated values * feat: fixing up errors, added simple error comparison tool * wrapping up error formatting * Fixes for unit tests * fix PLUGINS_VALIDATION_ERROR * fix fs.readdir bug, show rows even if there's only markdown formatting [SKIP CI] * when in base-list, show full width of errors * Fix type errors * added searching and filtering for files based on error name * fix: system tests * updated NO_SPECS_FOUND error to properly join searched folder + pattern - join patterns off of process.cwd, not projectRoot - highlight original specPattern in yellow, baseDir in blue - add tests * fixes failing tests * fix test * preserve original spec pattern, display relative to projectRoot for terminal banner * make the nodeVersion path display in gray, not white * fix tests, pass right variables * fix chrome:canary snapshots * update snapshots * update snapshot * strip newlines caused by "Still waiting to connect to {browser}..." * don't remove the snapshotHtmlFolder in CI, add additional verification snapshots match to error keys symmetrically * update snapshot * update snapshot * update snapshot * update snapshot * update snapshot * update snapshot * update gitignore * fix snapshot * update snapshot html * update logic for parsing the resolve pattern matching, add tests * update snapshots * update snapshot * update snapshot * update snapshot * fix failing test * fix: error_message_spec * fix snapshot * run each variant through an it(...) so multiple failures are received * add newlines to multiline formatters, add fmt.stringify, allow format overrides * stringify invalid return values from plugins * move config validation errors into packages/errors, properly highlight and stringify values * add component testing yarn commands * fix the arrow not showing on details * fix typescript error * fixed lots of poorly written tests that weren't actually testing anything. created settings validation error when given a string validation result. * fixes tests * fixes tests, adds new error template for validating within an array list (for browser validation) * remove dupe line * fix copy for consistency, update snapshots * remove redundant errors, standardize formatting and phrasing * more formatting * remove excess snapshots * prune out excessive error snapshot html files when not in CI * add missing tests, add case for when config validation fails without a fileType * fixes test * update snapshot * update snapshot * update snapshot * sort uniqErrors + errorKeys prior to assertion - assert one by one * add system test for binding to an event with the wrong handler * fixes tests * set more descriptive errors when setting invalid plugin events, or during plugin event validation * remove duplicate PLUGINS_EVENT_ERROR, collapse into existing error * use the same multiline formatting as @packages/errors * standardize verbiage and highlighting for consistency * fix incorrect error path * fixes tests, standardized and condensed more language * Update packages/errors/src/errors.ts Co-authored-by: Ryan Manuel <ryanm@cypress.io> * Update guides/error-handling.md Co-authored-by: Ryan Manuel <ryanm@cypress.io> * Update guides/error-handling.md Co-authored-by: Ryan Manuel <ryanm@cypress.io> * added some final todo's * fix types Co-authored-by: Tim Griesser <tgriesser10@gmail.com> Co-authored-by: Tim Griesser <tgriesser@gmail.com> Co-authored-by: Ryan Manuel <ryanm@cypress.io>

wachunga · 2022-06-15T23:45:39Z

Has this been released? I don't it in the public changelog

BlueWinds · 2022-06-16T19:45:32Z

It went out in 9.5.0 - looks like we did miss it in the changelog at that time, whoops.

Adjust ffmpeg CLI args for performance

e161c9b

BlueWinds changed the title ~~fix: Adjust ffmpeg CLI args for performance~~ chore: Adjust ffmpeg CLI args for performance Jan 31, 2022

BlueWinds changed the title ~~chore: Adjust ffmpeg CLI args for performance~~ fix: Adjust ffmpeg CLI args for performance Jan 31, 2022

Properly use and clean up metadata file

d55dd5f

BlueWinds marked this pull request as ready for review January 31, 2022 19:08

BlueWinds requested a review from a team as a code owner January 31, 2022 19:08

BlueWinds requested review from jennifer-shehane and tgriesser and removed request for a team January 31, 2022 19:08

jennifer-shehane reviewed Jan 31, 2022

View reviewed changes

tgriesser reviewed Jan 31, 2022

View reviewed changes

packages/server/lib/video_capture.ts Outdated Show resolved Hide resolved

BlueWinds added 4 commits January 31, 2022 12:33

Always limit encoder to 1 thread regardless of os.cpu count

d5ffc92

Typo in last commit

b2d21d4

Add comments to ffmpeg args

2dc63d8

Remove threads arg, back to original threading behavior

0ba6c8c

BlueWinds requested review from tgriesser and jennifer-shehane January 31, 2022 22:38

jennifer-shehane requested a review from brian-mann January 31, 2022 22:40

tgriesser approved these changes Feb 1, 2022

View reviewed changes

jennifer-shehane approved these changes Feb 2, 2022

View reviewed changes

flotwig approved these changes Feb 2, 2022

View reviewed changes

flotwig mentioned this pull request Feb 2, 2022

Videos of failing tests only show 1 or 2 frames #20012

Closed

BlueWinds merged commit 2c1ecab into develop Feb 2, 2022

BlueWinds deleted the unify-985-ffmpeg-performance branch February 2, 2022 15:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Adjust ffmpeg CLI args for performance #19983

fix: Adjust ffmpeg CLI args for performance #19983

BlueWinds commented Jan 31, 2022 •

edited

Loading

cypress-bot bot commented Jan 31, 2022

cypress bot commented Jan 31, 2022 •

edited

Loading

jennifer-shehane Jan 31, 2022

BlueWinds Jan 31, 2022

jennifer-shehane Jan 31, 2022

tgriesser commented Feb 1, 2022

BlueWinds commented Feb 1, 2022

flotwig left a comment

wachunga commented Jun 15, 2022

BlueWinds commented Jun 16, 2022 •

edited

Loading

fix: Adjust ffmpeg CLI args for performance #19983

fix: Adjust ffmpeg CLI args for performance #19983

Conversation

BlueWinds commented Jan 31, 2022 • edited Loading

User facing changelog

Additional details

How has the user experience changed?

PR Tasks

cypress-bot bot commented Jan 31, 2022

cypress bot commented Jan 31, 2022 • edited Loading

Test summary

Run details

jennifer-shehane Jan 31, 2022

Choose a reason for hiding this comment

BlueWinds Jan 31, 2022

Choose a reason for hiding this comment

jennifer-shehane Jan 31, 2022

Choose a reason for hiding this comment

tgriesser commented Feb 1, 2022

BlueWinds commented Feb 1, 2022

flotwig left a comment

Choose a reason for hiding this comment

wachunga commented Jun 15, 2022

BlueWinds commented Jun 16, 2022 • edited Loading

BlueWinds commented Jan 31, 2022 •

edited

Loading

cypress bot commented Jan 31, 2022 •

edited

Loading

BlueWinds commented Jun 16, 2022 •

edited

Loading