Skip to content

Conversation

@ipa-rmb
Copy link
Contributor

@ipa-rmb ipa-rmb commented Feb 1, 2019

  • fixes Extend image_flip to cope with other image formats #85
  • make .at independent of type and number of channels by creating following methods:
    template void setMatValuePtr(cv::Mat& image, int row, int index, double value);
    void setMatValuePtr(cv::Mat &image, int row, int index, double value);
    template T getMatValuePtr(cv::Mat& image, int row, int index);
    double getMatValuePtr(cv::Mat& image, int row, int index);
  • tried to take care about the intendation ;)

Copy link
Contributor Author

@ipa-rmb ipa-rmb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good, please change the commented smaller items.
Did you test whether the functions with 90/180/270 deg rotations still work and whether they work with e.g. a gray scale single channel image and a color image?

return true;
}

template <typename T> void ImageFlip::setMatValuePtr(cv::Mat& color_image, int u, int v, double value)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of double value a T value would be more explicit about the usage of the type. If it does not cause problems, please change the function head accordingly, remove type conversion (T) inside the function and add the respective type in the function calls of the other setMatValuePtr function.

void disparityDisconnectCB(const ros::SingleSubscriberPublisher& pub);


template <typename T>void setMatValuePtr(cv::Mat& image, int row, int index, double value);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brief comments on usage and meaning of the functions, please. It should be noted, e.g., how the row and index coordinates work. Maybe add an example how to compute the index for a multi-channel image (index = number_channels*u+channel_k).

color_image.ptr<T>(u)[v] = T(value);
}

void ImageFlip::setMatValuePtr(cv::Mat& color_image, int u, int v, double value)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. The naming int u, int v is misleading for multi-channel images. Please use int row, int index instead as done in the header file.
  2. Indentation down to line 242 is still spaces and should be changed to tabs as the remainder of the file.

Copy link
Member

@fmessmer fmessmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please rebase and finalize

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.

Extend image_flip to cope with other image formats

3 participants