Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make video codec and scale configurable #833

Merged
merged 2 commits into from
Nov 21, 2018
Merged

Make video codec and scale configurable #833

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8793bdb
Make video codec and scale configurable
Nov 20, 2018
1440849
Update docs
Nov 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 22 additions & 11 deletions lib/commands/recordscreen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const DEFAULT_RECORDING_TIME_SEC = 60 * 3;
const DEFAULT_MJPEG_SERVER_PORT = 9100;
const DEFAULT_FPS = 10;
const DEFAULT_QUALITY = 'medium';
const DEFAULT_VCODEC = 'mjpeg';
const MP4_EXT = '.mp4';
const FFMPEG_BINARY = 'ffmpeg';
const ffmpegLogger = logger.getLogger(FFMPEG_BINARY);
Expand Down Expand Up @@ -49,9 +50,14 @@ class ScreenRecorder {
const args = [
'-f', 'mjpeg',
'-i', `http://localhost:${localPort}`,
'-vcodec', 'mjpeg',
'-y', this.videoPath,
];
if (this.opts.videoScale) {
args.push('-vf', `scale=${this.opts.videoScale}`);
}
args.push(
'-vcodec', this.opts.videoType,
'-y', this.videoPath
Copy link
Member

@KazuCocoa KazuCocoa Nov 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make changeable the video path to an arbitrary extension like .avi?
Since:

$ ffmpeg -f mjpeg -r 10 -i http://localhost:9100 -vcodec mjpeg -y sample.mp4

=> Can get broken video. We cannot play the video even we change the extension of sample.mp4 to .avi

$ ffmpeg -f mjpeg -r 10 -i http://localhost:9100 -vcodec mjpeg -y sample.avi

=> Can get playable video

$ ffmpeg -f mjpeg -r 10 -i http://localhost:9100 -vcodec mjpeg -f mjpeg -y sample

=> Can get broken video, but we cannot play the video even we add .avi to sample file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying not to over engineer it. If the file is returned as base-64 encoded then the user is free to set any extension. And even if one sets mp4 instead of avi then video players are still able to play it properly, since they only care about the header information.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understood.

It was because when I ran https://github.com/appium/ruby_lib_core/pull/164/files#diff-a09385fd21af58063384116d47f91d91R27 ,

      def test_start_recording_screen
        to_path = 'recorded_file_ios.avi'
        @@driver.start_recording_screen video_type: 'mjpeg'
        sleep 3 # second
        @@driver.stop_and_save_recording_screen to_path
      end

I was not able to play recorded_file_ios.avi by QuickTime. The templorary file in /var/folders/y6/524wp8fx0xj5q1rf6fktjrb00000gn/T/20181021-40254-ug067c.gg8l/appium_3febaf.mp4 which created in Appium server side also was not.

The stop_and_save_recording_screen is simply store decoded base64 data into a file as below.

            def stop_and_save_recording_screen(file_path)
              base64data = execute(:stop_recording_screen, {}, {})
              File.open(file_path, 'wb') { |f| f << Base64.decode64(base64data) }

I was able to play the result like below under this PR.

      def test_start_recording_screen
        to_path = 'recorded_file_ios.mp4'
        @@driver.start_recording_screen video_type: 'libx264'
        sleep 3 # second
        @@driver.stop_and_save_recording_screen to_path
      end

By changing this.videoPath to use .avi format instead of .mp4 in my local, I could get playable video, recorded_file_ios.avi, with the former scenario. (ffmpeg changes headers following exported file, I guess since we can get an error like Unable to find a suitable output format for 'sample' when we do not set output file properly.) So, I read options like -f if we could get a file which does not depend on the extention of file output.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quicktime is not very smart. I'd rather use VLC or Mplayer ;)
But anyway, you see it is possible to change the extension locally in your case, so this should not be an issue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was able to play it by VLC.
ah..

);
this.mainProcess = new SubProcess(FFMPEG_BINARY, args);
this.mainProcess.on('output', (stdout, stderr) => {
if (stderr && !stderr.includes('frame=')) {
Expand Down Expand Up @@ -142,20 +148,23 @@ class ScreenRecorder {
* @property {?string} remotePath - The path to the remote location, where the resulting video should be uploaded.
* The following protocols are supported: http/https, ftp.
* Null or empty string value (the default setting) means the content of resulting
* file should be encoded as Base64 and passed as the endpount response value.
* file should be encoded as Base64 and passed as the endpoint response value.
* An exception will be thrown if the generated media file is too big to
* fit into the available process memory.
* This option only has an effect if there is screen recording process in progreess
* This option only has an effect if there is screen recording process in progress
* and `forceRestart` parameter is not set to `true`.
* @property {?string} user - The name of the user for the remote authentication. Only works if `remotePath` is provided.
* @property {?string} pass - The password for the remote authentication. Only works if `remotePath` is provided.
* @property {?string} method - The http multipart upload method name. The 'PUT' one is used by default.
* Only works if `remotePath` is provided.
* @property {?string} videoType - The format of the screen capture to be recorded.
* Available formats: "h264", "mp4" or "fmp4". Default is "mp4".
* @property {?string} videoType - The video codec type used for encoding of the be recorded screen capture.
* Execute `ffmpeg -codecs` in the terminal to see the list of supported video codecs.
* 'mjpeg' by default.
* @property {?string|number} videoQuality - The video encoding quality (low, medium, high, photo - defaults to medium).
* @property {?string|number} videoFps - The Frames Per Second rate of the recorded video. Change this value if the resulting video
* is too slow or too fast. Defaults to 10.
* @property {?string} videoScale - The scaling value to apply. Read https://trac.ffmpeg.org/wiki/Scaling for possible values.
* No scale is applied by default.
* @property {?boolean} forceRestart - Whether to try to catch and upload/return the currently running screen recording
* (`false`, the default setting) or ignore the result of it and start a new recording
* immediately.
Expand All @@ -164,8 +173,8 @@ class ScreenRecorder {
*/

/**
* Record the display of devices running iOS Simulator since Xcode 8.3 or real devices since iOS 8
* (ios-minicap utility is required: https://github.com/openstf/ios-minicap).
* Record the display of devices running iOS Simulator since Xcode 9 or real devices since iOS 11
* (ffmpeg utility is required: 'brew install ffmpeg').
* It records screen activity to a MPEG-4 file. Audio is not recorded with the video file.
* If screen recording has been already started then the command will stop it forcefully and start a new one.
* The previously recorded video file will be deleted.
Expand All @@ -177,11 +186,11 @@ class ScreenRecorder {
*/
commands.startRecordingScreen = async function (options = {}) {
const {
// TODO: Apply video type to ffmpeg command line
// videoType,
videoType = DEFAULT_VCODEC,
timeLimit = DEFAULT_RECORDING_TIME_SEC,
videoQuality = DEFAULT_QUALITY,
videoFps = DEFAULT_FPS,
videoScale,
forceRestart,
} = options;

Expand All @@ -200,6 +209,8 @@ commands.startRecordingScreen = async function (options = {}) {
const screenRecorder = new ScreenRecorder(this.opts.device.udid, videoPath, {
remotePort: this.opts.mjpegServerPort || DEFAULT_MJPEG_SERVER_PORT,
usePortForwarding: this.isRealDevice(),
videoType,
videoScale,
});
if (!await screenRecorder.interrupt(true)) {
log.errorAndThrow('Unable to stop screen recording process');
Expand Down Expand Up @@ -264,7 +275,7 @@ commands.startRecordingScreen = async function (options = {}) {
* @property {?string} remotePath - The path to the remote location, where the resulting video should be uploaded.
* The following protocols are supported: http/https, ftp.
* Null or empty string value (the default setting) means the content of resulting
* file should be encoded as Base64 and passed as the endpount response value.
* file should be encoded as Base64 and passed as the endpoint response value.
* An exception will be thrown if the generated media file is too big to
* fit into the available process memory.
* @property {?string} user - The name of the user for the remote authentication. Only works if `remotePath` is provided.
Expand Down