keras-team / keras-cv

Industry-strength Computer Vision workflows with Keras
Other
1.01k stars 330 forks source link

Reorder channel layer for smooth native RGB - BGR #20

Closed LukeWood closed 10 months ago

LukeWood commented 2 years ago

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: https://github.com/keras-team/keras/issues/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

bhack commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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:

https://github.com/tensorflow/graphics/issues/107 https://www.tensorflow.org/graphics/api_docs/python/tfg/image/color_space

piEsposito commented 2 years ago

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

Thank you for the info thou.

Em sex., 10 de dez. de 2021 às 11:10, bhack @.***> escreveu:

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://github.com/tensorflow/graphics/issues/107 https://www.tensorflow.org/graphics/api_docs/python/tfg/image/color_space

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/keras-team/keras-cv/issues/20#issuecomment-991004475, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALLYRXXT4GBHZKEAGDPPAH3UQIC4RANCNFSM5JTDHT6Q .

--

Pi Esposito | piesposito.github.io http://piesposito.github.io

bhack commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

We have already this in SIG:

https://github.com/tensorflow/io/blob/master/tensorflow_io/python/experimental/color_ops.py#L36

bhack commented 2 years ago

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 commented 2 years ago

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

bhack commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

Feels like a mutual issue, https://github.com/keras-team/keras-io/issues/467

LukeWood commented 2 years ago

Instead of Einsum notation, let's just use:

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

This is simpler to parse.

innat commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

Ramesh will most likely take this task.

piEsposito commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 1 year ago

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