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.

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

Conversation

mykola-mokhnach
Copy link
Contributor

@mykola-mokhnach
Copy link
Contributor Author

@kolyakrasnik FYI

}
args.push(
'-vcodec', this.opts.videoType,
'-y', this.videoPath
Copy link
Member

@KazuCocoa KazuCocoa Nov 21, 2018

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..

@mykola-mokhnach mykola-mokhnach merged commit 714cc44 into appium:master Nov 21, 2018
@mykola-mokhnach mykola-mokhnach deleted the video_format branch November 23, 2018 20:38
khanayan123 pushed a commit to khanayan123/appium-xcuitest-driver that referenced this pull request May 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants