Skip to content
New issue

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

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

Already on GitHub? # to your account

[Improvement] Add font color to args in long_video_demo #947

Merged

Conversation

rlleshi
Copy link
Contributor

@rlleshi rlleshi commented Jun 20, 2021

Motivation

Make font color in long_video_demo.py controllable by fetching it as args to the script. Otherwise one has to modify the code.

Modification

Small modifications at long_video_demo.py

Use cases (Optional)

Updated docs accordingly

@codecov
Copy link

codecov bot commented Jun 20, 2021

Codecov Report

Merging #947 (727211c) into master (0a6fde1) will increase coverage by 0.01%.
The diff coverage is n/a.

❗ Current head 727211c differs from pull request most recent head 209cd5b. Consider uploading reports for the commit 209cd5b to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #947      +/-   ##
==========================================
+ Coverage   83.69%   83.70%   +0.01%     
==========================================
  Files         132      132              
  Lines        9983     9983              
  Branches     1722     1722              
==========================================
+ Hits         8355     8356       +1     
  Misses       1211     1211              
+ Partials      417      416       -1     
Flag Coverage Δ
unittests 83.70% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
mmaction/datasets/pipelines/augmentations.py 92.98% <0.00%> (+0.11%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0a6fde1...209cd5b. Read the comment docs.

return text_info


def get_results_json(result_queue, text_info, thr, msg, ind, out_json):
def get_results_json(result_queue, text_info, msg, thr, ind, out_json):
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is not good to change the argument position

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 updated accordingly. But why isn't this one good btw?

THICKNESS, LINETYPE)
elif len(text_info):
for location, text in text_info.items():
cv2.putText(frame, text, location, FONTFACE, FONTSCALE, FONTCOLOR,
cv2.putText(fr, text, location, FONTFACE, FONTSCALE, clr,
Copy link
Collaborator

Choose a reason for hiding this comment

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

it is not good to use abbreviation as variable name, which is hard to understand

@dreamerlin dreamerlin requested a review from innerlee June 20, 2021 10:37
@rlleshi rlleshi requested a review from dreamerlin June 20, 2021 13:52
args = parser.parse_args()
return args


def show_results_video(result_queue, text_info, thr, msg, frame, video_writer):
def show_results_video(result_queue, text_info, thr, msg, font_color, frame,
video_writer):
Copy link
Collaborator

Choose a reason for hiding this comment

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

def show_results_video(result_queue, text_info, thr, msg, frame, video_writer, font_color=(255, 255, 255)):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah.. updated (default value is in parse_args, however)

@rlleshi rlleshi requested a review from dreamerlin June 21, 2021 13:02
@innerlee
Copy link
Contributor

I'm okay with the change as long as it can run successfully 😃

@dreamerlin dreamerlin merged commit 830f2a4 into open-mmlab:master Jun 22, 2021
@rlleshi rlleshi deleted the add-color-to-args-long-video-demo branch June 23, 2021 10:18
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants