OLD | NEW |
---|---|
(Empty) | |
1 #!/usr/bin/env python | |
2 # Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. | |
3 # | |
4 # Use of this source code is governed by a BSD-style license | |
5 # that can be found in the LICENSE file in the root of the source | |
6 # tree. An additional intellectual property rights grant can be found | |
7 # in the file PATENTS. All contributing project authors may | |
8 # be found in the AUTHORS file in the root of the source tree. | |
9 | |
10 import argparse | |
kjellander_webrtc
2017/01/19 13:55:34
Please add a module docstring that briefly describ
mandermo
2017/01/19 16:46:42
Have added a comment now. Anything other format or
| |
11 import os | |
12 import shutil | |
13 import subprocess | |
14 import sys | |
15 import tempfile | |
16 | |
17 SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__)) | |
18 | |
19 WEBRTC_DIR = os.path.normpath(os.path.join(SCRIPT_DIR, '../../')) | |
kjellander_webrtc
2017/01/19 13:55:34
Call this SRC_DIR to avoid confusion (there's a we
kjellander_webrtc
2017/01/19 13:55:34
The way to write this is:
os.path.normpath(os.path
mandermo
2017/01/19 16:46:42
Done.
mandermo
2017/01/19 16:46:42
Done.
| |
20 | |
21 def _ParseArgs(): | |
22 parser = argparse.ArgumentParser(description='Start loopback video analysis') | |
23 parser.add_argument('--source_dir', | |
kjellander_webrtc
2017/01/19 13:55:34
This flag doesn't seem to be needed if we just use
mandermo
2017/01/19 16:46:42
This is an optional argument. It was useful when I
kjellander_webrtc
2017/01/20 08:18:56
As it's now defaulting to the SRC_DIR, it's fine t
mandermo
2017/01/23 09:09:30
Done. Added "." to the end of the help texts also.
| |
24 default=WEBRTC_DIR, help='The path to the WebRTC source directory') | |
25 parser.add_argument('build_dir_android', | |
26 help='The path to the build directory for Android') | |
27 parser.add_argument('--build_dir_x86', | |
28 help='The path to the build directory for building locally') | |
29 parser.add_argument('--temp_dir', | |
30 help='A temporary directory to put the output') | |
31 | |
32 args = parser.parse_args() | |
kjellander_webrtc
2017/01/19 13:55:34
Please add parser.error() messages to error out in
mandermo
2017/01/19 16:46:42
Why should they all be mandatory? Only build_dir_a
| |
33 return args | |
34 | |
kjellander_webrtc
2017/01/19 13:55:35
+1 blank line before top-level functions.
mandermo
2017/01/19 16:46:42
Done.
| |
35 def main(): | |
36 args = _ParseArgs() | |
37 | |
38 source_dir = args.source_dir | |
39 build_dir_android = args.build_dir_android | |
40 build_dir_x86 = args.build_dir_x86 | |
41 temp_dir = args.temp_dir | |
42 if not temp_dir: | |
43 temp_dir = tempfile.mkdtemp() | |
44 else: | |
45 if not os.path.exists(temp_dir): | |
46 os.makedirs(temp_dir) | |
47 | |
48 if not build_dir_x86: | |
49 build_dir_x86 = os.path.join(temp_dir, 'LocalBuild') | |
50 subprocess.check_call(['gn', 'gen', build_dir_x86]) | |
51 subprocess.check_call(['ninja', '-C', build_dir_x86, 'frame_analyzer']) | |
52 | |
53 # Download ffmpeg and zxing. | |
54 download_script = os.path.join(source_dir, | |
kjellander_webrtc
2017/01/19 13:55:35
Please introduce a constant at the top of the file
mandermo
2017/01/19 16:46:42
Have introduced a local variable toolchain_dir. If
kjellander_webrtc
2017/01/20 08:18:56
This is fine.
| |
55 'tools-webrtc/video_quality_toolchain/download.py') | |
kjellander_webrtc
2017/01/19 13:55:35
Avoid all / and just use multiple string arguments
mandermo
2017/01/19 16:46:42
Done.
| |
56 subprocess.check_call([download_script]) | |
57 | |
58 # Run the Espresso code. | |
59 espresso_target = os.path.join(build_dir_android, | |
60 'bin/run_AppRTCMobileTestStubbedVideoIO') | |
61 subprocess.check_call([espresso_target]) | |
62 | |
63 # Pull the output video. | |
64 test_video = os.path.join(temp_dir, 'test_video.y4m') | |
65 subprocess.check_call(['adb', 'pull', '/sdcard/output.y4m', test_video]) | |
kjellander_webrtc
2017/01/19 13:55:35
Please pass the filename (output.y4m) as a flag, w
mandermo
2017/01/19 16:46:42
As a flag to the script? This path is already hard
kjellander_webrtc
2017/01/20 08:18:56
Argh, you're right. I forgot about that. Hopefully
| |
66 | |
67 test_video_yuv = os.path.join(temp_dir, 'test_video.yuv') | |
68 | |
69 ffmpeg_path = os.path.join(source_dir, | |
70 'tools-webrtc/video_quality_toolchain/linux/ffmpeg') | |
71 | |
72 def convert_video(input_video, output_video): | |
73 subprocess.check_call([ffmpeg_path, '-y', '-i', input_video, output_video]) | |
74 | |
75 # Convert the test video. | |
kjellander_webrtc
2017/01/19 13:55:35
drop this comment, it doesn't add any value.
mandermo
2017/01/19 16:46:42
Done.
| |
76 convert_video(test_video, test_video_yuv) | |
77 | |
78 reference_video = os.path.join(source_dir, | |
79 'resources/reference_video_640x360_30fps.y4m') | |
kjellander_webrtc
2017/01/19 13:55:35
Please pass this string as a flag instead of hardc
mandermo
2017/01/19 16:46:42
As flag to this script?
Can be good with flexibil
kjellander_webrtc
2017/01/20 08:18:56
You're right, my bad.
| |
80 | |
81 reference_video_yuv = os.path.join(temp_dir, | |
82 'reference_video_640x360_30fps.yuv') | |
83 | |
84 # Convert the reference video. | |
kjellander_webrtc
2017/01/19 13:55:35
This comment doesn't tell anything the code isn't.
mandermo
2017/01/19 16:46:42
Done.
| |
85 convert_video(reference_video, reference_video_yuv) | |
86 | |
87 # Run compare script. | |
88 compare_script = os.path.join(source_dir, 'webrtc/tools/compare_videos.py') | |
89 zxing_path = os.path.join(source_dir, | |
90 'tools-webrtc/video_quality_toolchain/linux/zxing') | |
91 | |
92 # The frame_analyzer binary should be built for local computer and not for | |
93 # Android | |
94 frame_analyzer = os.path.join(build_dir_x86, 'frame_analyzer') | |
95 | |
96 frame_width = 640 | |
97 frame_height = 360 | |
98 | |
99 stats_file_ref = os.path.join(temp_dir, 'stats_ref.txt') | |
100 stats_file_test = os.path.join(temp_dir, 'stats_test.txt') | |
101 | |
102 subprocess.check_call([ | |
103 compare_script, '--ref_video', reference_video_yuv, | |
104 '--test_video', test_video_yuv, '--yuv_frame_width', str(frame_width), | |
105 '--yuv_frame_height', str(frame_height), | |
106 '--stats_file_ref', stats_file_ref, | |
107 '--stats_file_test', stats_file_test, '--frame_analyzer', frame_analyzer, | |
108 '--ffmpeg_path', ffmpeg_path, '--zxing_path', zxing_path]) | |
109 | |
110 shutil.rmtree(temp_dir) | |
111 | |
kjellander_webrtc
2017/01/19 13:55:34
+1 blank line before top-level functions and state
mandermo
2017/01/19 16:46:42
Done.
| |
112 if __name__ == '__main__': | |
113 sys.exit(main()) | |
114 | |
OLD | NEW |