0

Mods - please close this question. I found the mistake in the code. Unfortunately I can't delete this.

Is there any difference between the below two code snippets? Maybe with respect to padding? Because I am getting strange image with respect to the first ( statically allocated ). The second is ok.

Produces a distorted image ( missing blue I guess and the pixels are shifted )

function1(char *image) { 
 char image_data_[image_info_.imgSize];
 memcpy(image_data_, image, image_info_.imgSize); // 144000 bytes
 cv::Mat color_image_opencv(image_info_.height, image_info_.width, CV_8UC3, image_data_);
 cv::imwrite("image.png", color_image_opencv);
}

AND ( this works )

function2(char *image) { 
 char *image_data_ = NULL;
 image_data_ = reinterpret_cast<char*>(malloc(image_info_.imgSize));
 memcpy(image_data_, image, image_info_.imgSize);
 cv::Mat color_image_opencv(image_info_.height, image_info_.width, CV_8UC3, image_data_);
 cv::imwrite("image.png", color_image_opencv);
}
infoclogged
  • 3,641
  • 5
  • 32
  • 53
  • 1
    Can you add the "different" returns ? – Hearner Jul 19 '18 at 14:31
  • 3
    It perfectly possible for malloced and static arrays to be aligned differently. Since malloc doesn't know what kind of data will be stored in the allocated memory it's quite conservative with regard to alignment. – john Jul 19 '18 at 14:32
  • 4
    `image_info_.imgSize` is a compile-time constant? What is `image`? – Daniel Langr Jul 19 '18 at 14:35
  • @Hearner : the image is distorted and seemingly the blue part is missing from the snippet with static allocation. – infoclogged Jul 19 '18 at 14:36
  • on the margin: dont use malloc in C++ – Jacek Cz Jul 19 '18 at 14:36
  • 4
    You don't have static allocation, you have automatic allocation on the stack, which you have probably overflowed. –  Jul 19 '18 at 14:37
  • @DanielLangr let me edit my post for completion.. Jacek - the malloc is fine, static allocation is ruining things. – infoclogged Jul 19 '18 at 14:38
  • What is the value of `image_info_.imgSize`? If large, you can have stack overflow. – Daniel Langr Jul 19 '18 at 14:45
  • 3
    Variable-length arrays are non-standard and quite dangerous. You're generally better off avoiding their use. In this case it doesn't look like you need to make an extra copy of your image data at all though. Just use it in place. – Miles Budnek Jul 19 '18 at 14:46
  • @DanielLangr its 144KB – infoclogged Jul 19 '18 at 14:46
  • @infoclogged And this value is hard-coded in the code? – Daniel Langr Jul 19 '18 at 14:47
  • @DanielLangr - yes hard coded.. as said, the malloc works, but as soon as I replace with automatic allocation - I get a distored image.. nothing changes else in the entire code. So, I guess somewhere a padding is created. Just dont know where. – infoclogged Jul 19 '18 at 14:48
  • @MilesBudnek yes, thats wht I will do and use image directly instead of saving it in a temporary buffer... But I gotta understand, why this is happenning for learning purpose. – infoclogged Jul 19 '18 at 14:49
  • @john made some coments about aligning..Can anyone throw some light on this? This looks like a valuable input. – infoclogged Jul 19 '18 at 14:51
  • @infoclogged if you know the size of the image why don't you write it down directly in your static allocation and see if it solves the issue, just for the sake of testing. – AlexG Jul 19 '18 at 14:51
  • 1
    @infoclogged What if you use `alignas(64) char image_data_[image_info_.imgSize];`? – Daniel Langr Jul 19 '18 at 14:56
  • Alex and Daniel - I am going to try both your solutions and see if it works. Gimme a minute. – infoclogged Jul 19 '18 at 14:57
  • How big is your stack? 144,000 on the stack could blow out the stack, depending on your platform. Replace the automatic allocation with `char image_data_[144000];` just to make sure its not doing a variable-length array (which is not standard C++). – Eljay Jul 19 '18 at 15:22
  • 1
    @DanielLangr - am sorry folks, the problem was at my end.. The image pointer is calculated using some arithmetic and the calculation was wrong for the code snippet with automatic allocation. Since the image pointer calculation was correct for malloc, it worked ! Please excuse me for the blunder. – infoclogged Jul 19 '18 at 15:32

1 Answers1

1

We can use our psychic debugging powers to infer that you are returning the cv::Mat from a function, or otherwise extending its lifetime beyond that of the char array to which it points. While with malloc() the memory used to store the image bytes is available until explicitly freed, by contrast with the automatic variable you must not use the image data once it has gone out of scope.

The documentation for the cv::Mat constructor you are using says:

Matrix constructors that take data and step parameters do not allocate matrix data. Instead, they just initialize the matrix header that points to the specified data, which means that no data is copied. This operation is very efficient and can be used to process external data using OpenCV functions. The external data is not automatically deallocated, so you should take care of it.

And of course when they say "take care of it," they do not mean "destroy it prior to destroying the cv::Mat."

John Zwinck
  • 239,568
  • 38
  • 324
  • 436
  • everything is in the same scope.. reading, allocating and writing. The only difference is malloc or automatic allocation. Please have a look at the edited post. – infoclogged Jul 19 '18 at 14:43
  • @infoclogged I don't know `cv` but presumably you're reading from `color_image_opencv` outside of this scope in order to determine if it has loaded the image data correctly? If that's the case then the automatically allocated `image_data_` will be out of scope by that point and your matrix relies on it for storage. – Fibbs Jul 19 '18 at 14:52
  • @Fibbles - the scope is out of question.. because I am making a filesystem output using cv::imwrite. – infoclogged Jul 19 '18 at 14:54
  • @infoclogged Ah, I see. Ignore me then. – Fibbs Jul 19 '18 at 14:55