Skip to content

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.

3 participants