Add decode_image Op - #4222

Can one of the admins verify this patch?

The _image_type() function is placeholder logic that needs to go into a separate TensorFlow operation. The code currently pushed works if pre-loaded bytes are sent to the Op, but will crash and burn if the output from another op is passed in (say, from tf.read_file())

I'm assuming that since this is still a WIP we should hold off on reviewing and testing it. Let me know if that isn't the case.

@danmane That's correct. The current code works if you load the bytes of a file ahead of time, but would throw an error if you passed in code from a function like tf.read_file.

The current game plan is to write a tf.substr Op and then utilize it in this function. Here's that conversation for context.

Thanks!

Any updates on this?

@rohan100jain Waiting for feedback on #4338: I can close this in the meantime if you'd prefer.

@rohan100jain @danmane - tf.substr has been merged, and I've update this Op to utilize it. There are a few things left to do, but the main functionality of the op is here. Left to do:

• More/better documentation
• More tests (should we test with small jpeg/png/gif files in addition to hard-coding in bytes?)
• Tests for gifs

Thanks @samjabrahams ! Ping us when you think this is ready for a final review.

@vrv ready for review! Couple of judgement calls:

• Included GIF support. decode_gif returns a 4-D tensor as opposed to 3-D for decode_jpeg and decode_png due to the extra "number-of-frames" dimension. I'm also not sure if decode_gif is supposed to be exposed publicly yet (currently not in the API).
• Did not add actual image files to be read from disk and tested- stuck with hard-coded byte strings.

@girving let me know if you have time to look at this. I worry that the variable shape output of this convenience op will make it inconvenient :), but maybe it'll be alright. Also, it might make sense to think about how to pass the options to decode_image in a better way -- otherwise the op's arguments are going to keep increasing to account for every image option that is added.

The different shaped output definitely seems a bit clunky. Couple options (which may or may not be any less clunky):

1. Return tf.squeeze(tf.decode_gif(...)). Since the decode_gif option always has three color channels, this will return a 3-D tensor for single-framed gifs (assuming the image is larger than 1x1) and 4-D tensors otherwise. If decode_gif ever gets an option for number of color channels, this will have to be modified.
2. Reshape the png and jpeg results to be 4-D. This keeps it consistent, but means that this Op requires a different workflow to the standard decode_png and decode_jpeg
3. Remove the decode_gif portion
4. Leave it as is and have the end user figure it out.

For the growing number of parameters: I could use **kwargs and either let the user look up the options, or just leave the options out of the function signature but include them in the documentation.

I can look at it, but could we squash it before the initial review?

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

Jenkins, test this please.

@mrry: The newly added decode_image tests are failing to find the test files on Windows. Do you know why that would happen on Windows but not on other platforms?

Looks like it's a path issue: tensorflow/core/lib/gif/testdata/scan.gif : The system cannot find the path specified. Curse you backslashes! I'll wrap the files with os.path.join and see if that works.

Edit: it probably isn't this, but I'll keep in the os.path as a best-practices thing (unless thats no-bono)

I assume it's because the working directory isn't what the test expects. (It looks like the similar code image_ops_test.py isn't running as part of the Windows build because it's in python/ops rather than python/kernel_tests.)

As a quick workaround, you might try adding WORKING_DIRECTORY ${tensorflow_source_dir} to this line and see if it fixes things.

@girving @mrry I pushed the change to tf_tests.cmake so we can see if that does the trick.

Jenkins, test this please.

May need to rerun- the GPU test had an aneurysm when trying to git fetch

Jenkins, test this please.

@zffchen78 The Linux GPU build has twice failed during git fetch. Do you know what might be going wrong?

The fact that it returns different tensors shapes based on the file format is not the 'usual' tensoflow way.
Can we just return the first frame in case of a gif image instead? If the user really wants the whole gif sequence then they can use the specialized version. This function will be very useful when you have mixed images formats in your data folder.

@Mistobaan What fixed shape do the other image decode functions return?

Right now decode_jpeg and decode_png return tensors with rank 3, while decode_gif returns one with rank 4, as it can decode multiple frames from a gif image.

Jenkins, test this please.

@Mistobaan you can always tf.squeeze the output of this Op of you know that you have gifs that only contain a single frame. Personally, I don't really know what the "TensorFlowian" way to do it would be, but it seems wrong to discard image data (by automatically only keeping the first frame of multi-frame gifs). That said, I wasn't sure I should even include gif support when I started this Op due to the rank differences. Here are my thoughts for options:

1. Leave it as is. Let users figure out how they want to handle gifs
2. Don't allow multi-frame gifs. Throw an assertion error if the number of frames is greater than 1, and squeeze the first dimension out
3. Leave it as is, and create a contrib layer that does extra work to ensure that the output ranks match for jpeg, png, and gif

I'd prefer to either leave it as is or automatically squeeze the first dimension of the gif if it's a single frame (unfortunately that requires another tf.cond). Silently discarding data by picking the first frame is not a good idea.

If we want to add the optional squeeze to gif I think it can be done in a second pass. I'm going to merge for now.