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

Rewrite the fns_candy_style_transfer example so that it doesn't depends on libpng on Windows #163

Merged
merged 22 commits into from
Oct 29, 2022

Conversation

snnn
Copy link
Member

@snnn snnn commented Oct 25, 2022

  1. Rewrite the fns_candy_style_transfer example so that it doesn't depends on libpng on Windows
  2. Fix a problem in pipeline's yaml file: {{parameters.ortversion}} was not evaluated because I forgot to put a $ sign in front of it.

if(WIN32)
target_sources(fns_candy_style_transfer PRIVATE image_file_wic.cc)
else()
target_sources(fns_candy_style_transfer PRIVATE image_file_libpng.c)
Copy link
Contributor

Choose a reason for hiding this comment

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

my understanding is that libpng things (PNG_LIBRARIES, PNG_INCLUDE_DIRS, PNG_LIBDIR) aren't used on windows. perhaps it would be clearer to put them in this block?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, You are right.

done

apt-get update
apt-get install -y cmake gcc g++ libpng-dev libjpeg-turbo8-dev curl
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this is not really part of "downloading ORT", maybe it could go in a separate script or this one could be renamed

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated.

FAIL_FAST_IF_FAILED(decoder->GetFrameCount(&count));
if (count != 1) {
printf("The image has multiple frames, I don't know which to choose.\n");
abort();
Copy link
Contributor

Choose a reason for hiding this comment

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

why not return -1?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! I updated the code.

@snnn snnn requested a review from edgchen1 October 28, 2022 21:04
@snnn snnn merged commit 09f9380 into main Oct 29, 2022
@snnn snnn deleted the snnn/wic branch October 29, 2022 00:05
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.

2 participants