V-jinmgong/Emulation: Add implementation and evaluation scripts for RL-BWE emulation on AlphaRTC platform #3
Conversation
| assert "video" in data | ||
| assert type(data["video"]) == float | ||
| assert data["video"] >= 0 and data["video"] <= 100 | ||
| assert "audio" in data | ||
| assert type(data["audio"]) == float | ||
| assert data["audio"] >= 0 and data["audio"] <= 100 | ||
| assert "network" in data | ||
| assert type(data["network"]) == float | ||
| assert data["network"] >= 0 and data["network"] <= 100 |
There was a problem hiding this comment.
How about using a function to do that:
def validate_metric(data, key):
assert key in data, f"Missing key: {key}"
assert isinstance(data[key], float), f"Value for {key} is not a float"
assert 0 <= data[key] <= 100, f"Value for {key} is out of range: {data[key]}"
validate_metric(data, "video")
validate_metric(data, "audio")
validate_metric(data, "network")
By the way, it is generated by copilot. :-)
There was a problem hiding this comment.
you can select the whole class and ask copilot to review it and give you some suggestions to improve it, and it will generate code which you can choose to replace yours or not based on the code quality.
metrics/utils/calc_perception.py
Outdated
| return avg_lpips | ||
|
|
||
| if __name__ == '__main__': | ||
| original_frame_dir = '/home/onl/onl-emu/metrics/tests/data/test_frames/original' |
There was a problem hiding this comment.
use relative path instead of the abstract path otherwise the code will not be executed if people loade code at other places.
metrics/utils/preprocess.py
Outdated
| cut_frame_path = os.path.join(cut_folder, f'cut_{frame}') | ||
|
|
||
| # Crop the frame number area | ||
| crop_value = '70x30+910+1030' # Adjust this value based on your video resolution |
There was a problem hiding this comment.
if we have changed the video resolution, how can we adjust it? Can we change it into a function and generate its value by uthe resolution getting from the input frame automatically?
| if __name__ == "__main__": | ||
| parser = init_audio_argparse() | ||
| args = parser.parse_args() | ||
| if args.scenario and args.ground_service: |
There was a problem hiding this comment.
just want to know where the service is. Can you provide its URL and introduction in some comments?
| if src_video_info.format_name not in self.eval_method.support_type: | ||
| fo_new_src_video = self.change_video_type(src_video_info, self.eval_method.support_type_abbreviation[0]) | ||
| src_video_info = VideoInfo(fo_new_src_video.name) | ||
| if dst_video_info.format_name not in self.eval_method.support_type: | ||
| fo_new_dst_video = self.change_video_type(dst_video_info, self.eval_method.support_type_abbreviation[0]) | ||
| dst_video_info = VideoInfo(fo_new_dst_video.name) |
There was a problem hiding this comment.
since the only difference of these two blocks are source video and destination video, so I guess you can create a function and pass src/dst video as its parameter. You can keep this for now but please rethink the code to see it we can make it better. :-)
compress_logs.py
Outdated
| import argparse | ||
| import os | ||
|
|
||
| def compress_logs(log_file, patterns, output_path): |
There was a problem hiding this comment.
this name is confusing, I mean, this is not the real compressing, right? How about a better name? :-)
| trace_patterns = config_data.get("uplink", {}).get("trace_pattern", []) | ||
|
|
||
| # Loop through the configuration patterns | ||
| while True: |
There was a problem hiding this comment.
do you really need this "while True", and why do you need it?
…er version of video evaluation metrics
|
Sorry, the test and tc-setup files are not written by me, so I may not make appropriate modifications. Except for those files, I have made corrections to the other parts based on comments. And I have add some new files. |
| @@ -366,7 +366,7 @@ align_ocr() { | |||
| # crop out the frame number and check if it is a number | |||
| left=`expr $WIDTH / 2 - 50` | |||
| top=`expr $HEIGHT - 50` | |||
| crop_value=70x30+$left+$top | |||
| crop_value=150x60+$left+$top | |||
There was a problem hiding this comment.
so 150 and 60 are fixed value instead of configurable value?
There was a problem hiding this comment.
OK, there are more problems here, such as "left=expr $WIDTH / 2 - 60", do we check if the left value or top value will be a positive number? 😁
There was a problem hiding this comment.
Your concern is reasonable, but this script was not originally written by me, so I did not modify the parameter interface inside. In the frame-number marking and frame alignment scripts I wrote myself (i.e.preprocess_video.sh & calc_scores.py), the position and size of the frame number can be used as parameter input, so users need to ensure that the input is valid. I will create a simple instructional document called 'readme' to explain the usage precautions for each file.
|
I have added a readme file. If there is anything else that needs to be explained, please feel free to comment here |
No description provided.