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

Channels swap refactoring 0225 #283

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

Conversation

Centimo
Copy link

@Centimo Centimo commented Feb 3, 2025

Task

Fix the current way of processing the format of the source image in SimdSynetSetInput().

How it worked before a19e2d8: SimdSynetSetInput() either left the channel order low or swapped Red and Blue depending on the source image format. The change occurred when srcFormat was equal to Rgb24 or Rgba32. Thus, in order to pass the image in Rbg24 format as is, the format value had to be changed, leaving the actual channel order unchanged.
How it works after a19e2d8: the isRgb flag appeared in SynetSetInput(), and when set to true, srcFormat was changed to the reverse one (BGR -> RGB). Thus, depending on the source format and the value of the flag SimdSynetSetInput() either changes the channel order or not.

Issues:

  1. Swapping channel order in SimdSynetSetInput() is a non-obvious and inconvenient mechanics. isRgb is even more confusing: now the swap happens for some formats at false, for others at true.
  2. SynetSetInput() and SimdSynetSetInput() are available externally, and while the first one after a19e2d8 has the isRgb parameter, the other has none.

Solution

Replace the isRgb parameter with swapChannels and add it to SimdSynetSetInput(). The swapChannels parameter now strictly specifies the change of channel order regardless of the format. Now user does not need to mess with changing the source image, and the effect of both functions is transparent.

@teodorvlasov
Copy link

Looks like proposed approach will break the existing API of SimdSynetSetInput. If we are going break it anyway, why not to deprecate it and create a new function that will not do channel swapping at all?

@Centimo
Copy link
Author

Centimo commented Feb 5, 2025

Looks like proposed approach will break the existing API of SimdSynetSetInput. If we are going break it anyway, why not to deprecate it and create a new function that will not do channel swapping at all?

Сhanges in this PR assume that the SimdSynetSetInput function performs channel swapping only if the corresponding flag is specified. Initially swapping is performed depending on the combination of input parameters (image format and isRgb value).

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.

2 participants