Skip to content
This repository has been archived by the owner on Aug 15, 2019. It is now read-only.

EncodeBase64 and DecodeBase64 ops #1779

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

vabarbosa
Copy link

@vabarbosa vabarbosa commented Jun 6, 2019

the TF implementation of the pix2pix model which fails conversion because of

`Unsupported Ops: DecodeJpeg, EncodePng, DecodeBase64`

the Open NSFW model also fails conversion with some of the same ops (tensorflow/tfjs#433).

i wanted to try to implement some of these ops in TensorFlow.js. starting with this pull request for DecodeBase64 and EncodeBase64.

along with this tfjs-core PR, there is a corresponding PR in tfjs-converter (tensorflow/tfjs-converter#376)


To see the logs from the Cloud Build CI, please join either
our discussion
or announcement mailing list.


This change is Reviewable

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@vabarbosa vabarbosa reopened this Jun 6, 2019
@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@nsthorat
Copy link
Contributor

Can you fix the failing errors: https://console.cloud.google.com/gcr/builds/b214fa07-9b38-4194-909d-f5bf95a89811?project=834911136599

Just curious have you tried this in Node, does it work everywhere? Why aren't you using the built in methods to do base64 conversion?

@nsthorat
Copy link
Contributor

By the way thanks for the PR!

@vabarbosa
Copy link
Author

vabarbosa commented Jun 11, 2019

Can you fix the failing errors: https://console.cloud.google.com/gcr/builds/b214fa07-9b38-4194-909d-f5bf95a89811?project=834911136599

@nsthorat my apologies, the errors should be resolved now.

Just curious have you tried this in Node, does it work everywhere? Why aren't you using the built in methods to do base64 conversion?

i have added it to the Node kernel (tensorflow/tfjs-node#259) and i tried it out.

regarding built in methods are you asking about btoa()/atob()? if so, browsers fail if a character exceeds the range of a 8-bit byte (0x00~0xFF). for example, this would not work:

btoa('add emphasis— with em dash')

because of the em dash/long dash unicode character (i.e., 0x2014).
if you are referring to some other built in methods please let me and i can review to make sure i didn't miss something.

thanks

@nsthorat
Copy link
Contributor

Hi @vabarbosa!

So @dsmilkov and I just did a deep dive and we have the following conclusions:

  • We are actually about to change the internal representation of string tensors to hold onto to the underlying byte array
  • This means that for you, you will just need to take that byte array and generate the base64 encoded version (as well as the reverse). This means just the method arrayBufferToString

We will let you know once string stuff is done!

@vabarbosa
Copy link
Author

thank you! i'll be on the look out for your string tensor updates. and if you have any questions for me in the meantime, feel free to ask.

@nsthorat
Copy link
Contributor

Here is the PR if you want to follow: #1816

- replace `arrayBufferToString` with `decodeString()`
- remove unused `stringToArrayBuffer`
@vabarbosa
Copy link
Author

hi @nsthorat i have pulled in all the latest updates (including string tensor PR #1816)
and i have made my changes accordingly. let me know if you have any questions.
thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants