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

Reorder channel layer for smooth native RGB - BGR #20

Closed
LukeWood opened this issue Dec 8, 2021 · 26 comments
Closed

Reorder channel layer for smooth native RGB - BGR #20

LukeWood opened this issue Dec 8, 2021 · 26 comments

Comments

@LukeWood
Copy link
Contributor

LukeWood commented Dec 8, 2021

This is a commonly performed operation in image processing making it a good fit for keras-cv. While this is not a super complicated operation (it's really just a lambda into a tf.gather) the resulting code is more readable. Let's include this in keras-cv.

Originally discussed here: keras-team/keras#15705

Re the design of the layer signature...

The current proposal is to implement a layer that looks like this:

rgb2bgr_layer = tf.keras.layers.ReorderChannel(order=[2, 1, 0], axis=-1)

Perhaps we actually may want to use a einsum inspired syntax, such as:

rgb2bgr_layer = tf.keras.layers.ReorderChannel('rgb->bgr', axis=-1)

This would be really readable to anyone stumbling upon a new codebase, and generalizes quite well to any number of channels.

Please feel free to comment below with any additional thoughts

@LukeWood LukeWood changed the title Reorder channel layer for smooth native RGB - BGR Proposal: Reorder channel layer for smooth native RGB - BGR Dec 8, 2021
@bhack
Copy link
Contributor

bhack commented Dec 8, 2021

This seems only about reordering but more in general we have many colorspace conversions in:

https://github.com/tensorflow/io/blob/v0.20.0/tensorflow_io/python/experimental/color_ops.py#L20

@piEsposito
Copy link

I see. The idea here is to enable BGR-RGB conversion on the model serving bundle, so we don't have to manipulate images "by hand" when serving it in any scenario.

@bhack
Copy link
Contributor

bhack commented Dec 10, 2021

I understand that this is at Keras layer level implementation but about lowlevel reusable components Remember that we had already another color space overlapping other then with Tensorflow IO SIG also with TF Graphics:

tensorflow/graphics#107
https://www.tensorflow.org/graphics/api_docs/python/tfg/image/color_space

@piEsposito
Copy link

piEsposito commented Dec 10, 2021 via email

@bhack
Copy link
Contributor

bhack commented Dec 10, 2021

I see. In that case, that’s just me trying to export and deploy my models
without rechanneling my images out of the model.

I understand and it could be useful here. The only problem that I want to highlight is that I prefer to not going again to have a proliferation/duplication of low level ops/API in every SIG.

@piEsposito
Copy link

I agree with you. What do you think would be the best solution for that? I think that we should have a keras layer for that, but avoiding redundancy of implementations :D.

@LukeWood
Copy link
Contributor Author

I'd be fond of the API:

rgb2bgr_layer = keras_cv.layers.ReorderChannel('rgb->bgr', axis=-1)

I think we could include this if someone wanted to contribute it.

@LukeWood
Copy link
Contributor Author

LukeWood commented Jan 25, 2022

If the parsing of the einsum notation is too verbose/unreadable, we can just have it work like a transpose op where you pass the channel ordering in a list.

@ariG23498
Copy link
Contributor

Hey folks,

The RGB-BGR operation is very easily possible with the tf.keras.layers.Permute. Would we want to wrap with with the einsum notation as suggested by @LukeWood?

@LukeWood
Copy link
Contributor Author

@ariG23498, could you post an example of RGB->BGR w/ permute? Thanks! I believe Francois thought this layer would be useful, maybe we can just have some examples showing how to use permute though if it’s possible.

@ariG23498
Copy link
Contributor

Hey @LukeWood
I was wrong with my thoughts. Permute is a useful layer for converting between channel_first and channel_last. I apologize for my comment earlier.

The layer that I am thinking of should be structured like this.

rgb_bgr_layer = keras.layers.Lambda(function=lambda x: x[..., -1::-1])

WDYT?

@bhack
Copy link
Contributor

bhack commented Jan 30, 2022

@bhack
Copy link
Contributor

bhack commented Jan 30, 2022

If we don't want to depend on ecosystem we could just try to copy and adopt these ops chain in layers or in our private API.

@LukeWood
Copy link
Contributor Author

No, we don't want to rely on the ecosystem. We want first party keras extensions to cover common use cases.

@bhack
Copy link
Contributor

bhack commented Jan 30, 2022

It is what I have suggested. I think copy these in private api in a colorspace utils file it could be useful.
E.g. If in another layer you need to do something more than color conversion or in visualizzation it could be still useful to have as an API other then only embedded as a layer componet.

@LukeWood
Copy link
Contributor Author

I don't think we need to copy these utils as I think we want to do something distinct. This is a generic ReorderChannels layer. It will take an arbitrary ordering and return a new one. The linked function only does RGB->BGR.

@bhack
Copy link
Contributor

bhack commented Jan 30, 2022

I think that they do more as If you see the whole file they are covering also many color space conversions that It is more than just channel reordering (unstack, stack):

rgb2bgr_layer = tf.keras.layers.ReorderChannel('rgb->bgr', axis=-1)

But probably we are not interested to have other colorspace transformations.

@innat
Copy link
Contributor

innat commented Jan 31, 2022

Regarding the Proposal: Reorder channel layer for smooth native RGB - BGR,

Feels like a mutual issue, keras-team/keras-io#467

@LukeWood
Copy link
Contributor Author

Instead of Einsum notation, let's just use:

channel_reorder = ReorderChannels('rgb', 'bgr')

This is simpler to parse.

@innat
Copy link
Contributor

innat commented Jan 31, 2022

@LukeWood @bhack
If it's only about rgb-bgr transformation, why if we just include tf.image.rgb_to_bgr function - like we have tf.image.rgb_to_grayscale, tf.image.yuv_to_rgb?

@LukeWood
Copy link
Contributor Author

LukeWood commented Jan 31, 2022

It's an arbitrary transformation regarding the ordering of channels.

If we want to include RGBtoGrayscale that would probably need to be it's own layer, same with yuv_to_rgb. These are more complex transformations than a simple permutation shift. Feel free to open an issue for RGBtoGrayscale layer. Not so sure about yuv_to_rgb, but if the community shows need for it we can include it.

@LukeWood
Copy link
Contributor Author

Ramesh will most likely take this task.

@piEsposito
Copy link

I think reorder channels would have a more general purpose, so we could build that and on top of that the rgb to bgr to rgb transformations.

@innat
Copy link
Contributor

innat commented Feb 19, 2022

Yes, it should be general-purpose. It should support the basic color transformations listed here https://github.com/tensorflow/io/blob/master/tensorflow_io/python/experimental/color_ops.py

(shouldn't be limited to rgb/bgr, IMO)

channel_reorder = ReorderChannels(format='rgb_to_bgr')( tensor )
channel_reorder = ReorderChannels(format='rgb_to_rgba')( tensor )
channel_reorder = ReorderChannels(format='bgr_to_rgb')( tensor )
channel_reorder = ReorderChannels(format='rgb_to_ycbcr')( tensor )
...

@LukeWood
Copy link
Contributor Author

sure, as far as implementation goes maybe a

channel_reorder = ReorderChannels(from='rgb', to='ycbcr')( tensor )

makes more sense. In this case, the layer is more of a FormatChange than a ReorderChannels

@LukeWood LukeWood changed the title Proposal: Reorder channel layer for smooth native RGB - BGR Reorder channel layer for smooth native RGB - BGR Apr 29, 2022
@LukeWood
Copy link
Contributor Author

Does not seem like this is a priority on the immediate roadmap - might as well close until theres a strong need.

freedomtan pushed a commit to freedomtan/keras-cv that referenced this issue Jul 20, 2023
* add nn ops

* fix style

* small

* conv

* Testing

* add docstring

* fix typo

* add more testing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants