I am attaching a snippet of code. I have two functions. One simply returns an integer (function_A). The other returns a struct (function_B). When I leave the code for fuction A uncommented in DSP_testing, everything works as expected and the log_d logs the integer. When I leave the code for function B uncommented in DSP_testing, the arduino goes into an infinite loop of restarting. Function B returns a ImageArray structure. Why does one work and not the other?? I have spent many hours on this and can't find a solution. After making the array in the functions "static", the one that returns an integer (function A) works without any problems. The one for function B goes into an infinte loop.
#define CAMERA_MODEL_AI_THINKER // Has PSRAM (ESP32-CAM)
#include <Arduino.h>
#include <esp_camera.h>
#include "camera_pins.h"
#include "img_converters.h"
#define FRAME_SIZE FRAMESIZE_QQVGA
#define WIDTH 160
#define HEIGHT 120
struct ImageArray
{
uint8_t data[HEIGHT][WIDTH];
};
bool setup_camera(framesize_t);
bool DSP_testing();
int function_A(camera_fb_t *frame);
ImageArray function_B(camera_fb_t *frame);
void setup()
{
Serial.begin(115200);
Serial.println(setup_camera(FRAME_SIZE) ? "OK" : "ERR INIT");
}
void loop()
{
if (!DSP_testing())
{
Serial.println("Failed capture");
delay(2000);
return;
}
delay(1000);
}
bool setup_camera(framesize_t frameSize)
{
camera_config_t config;
config.ledc_channel = LEDC_CHANNEL_0;
config.ledc_timer = LEDC_TIMER_0;
config.pin_d0 = Y2_GPIO_NUM;
config.pin_d1 = Y3_GPIO_NUM;
config.pin_d2 = Y4_GPIO_NUM;
config.pin_d3 = Y5_GPIO_NUM;
config.pin_d4 = Y6_GPIO_NUM;
config.pin_d5 = Y7_GPIO_NUM;
config.pin_d6 = Y8_GPIO_NUM;
config.pin_d7 = Y9_GPIO_NUM;
config.pin_xclk = XCLK_GPIO_NUM;
config.pin_pclk = PCLK_GPIO_NUM;
config.pin_vsync = VSYNC_GPIO_NUM;
config.pin_href = HREF_GPIO_NUM;
config.pin_sccb_sda = SIOD_GPIO_NUM;
config.pin_sccb_scl = SIOC_GPIO_NUM;
config.pin_pwdn = PWDN_GPIO_NUM;
config.pin_reset = RESET_GPIO_NUM;
config.xclk_freq_hz = 20000000;
config.pixel_format = PIXFORMAT_GRAYSCALE;
config.frame_size = frameSize;
config.jpeg_quality = 12;
config.fb_count = 1;
bool ok = esp_camera_init(&config) == ESP_OK;
sensor_t *sensor = esp_camera_sensor_get();
sensor->set_framesize(sensor, frameSize);
return ok;
}
bool DSP_testing()
{
camera_fb_t *frame = NULL;
frame = esp_camera_fb_get();
int value = function_A(frame);
log_d("Int is %d", value);
// ImageArray value = function_B(frame);
// log_d("Int is %d", value.data[0][0]);
esp_camera_fb_return(frame); // release frame buffer
if (!frame)
return false;
return true;
}
int function_A(camera_fb_t *frame)
{
int len = frame->len;
static uint8_t new_array[HEIGHT][WIDTH] = {{0}};
for (int h = 0; h < HEIGHT; h++)
{
for (int w = 0; w < WIDTH; w++)
{
int position = h * (len / HEIGHT) + w;
new_array[h][w] = {frame->buf[position]};
}
}
return 6;
}
ImageArray function_B(camera_fb_t *frame)
{
int len = frame->len;
static uint8_t new_array[HEIGHT][WIDTH] = {{0}};
for (int h = 0; h < HEIGHT; h++)
{
for (int w = 0; w < WIDTH; w++)
{
int position = h * (len / HEIGHT) + w;
new_array[h][w] = {frame->buf[position]};
}
}
ImageArray myimage = {{0}};
myimage.data[0][0] = 5;
return myimage;
}
You appear to be returning a pointer to an automatic variable myimage which may not exist when the function returns. Make it static if that works here or otherwise restructure your code.
It is also quite big at > 19kB.
Just to get further you could probably make the array myimage global (or static) then before the statement myimage.data[0][0] = 5; you'd then have to clean the array by setting each element to 0.
However, I can't really follow your program logic because in function_B() you manipulate an array called new_array but then do nothing with it then you return a reference to a huge 19.2kB struct myimage with only one element filled. But since that array is dynamically created within function_B it disappears when function_B returns, which is probably why you get the continuous crash cycle.
Is your goal to return new_array to the calling program? If so, you have to define first a containing struct of type ImageArray. Anyway, say what you are trying to achieve.
When you return the pointer to the 'ImageArray' the function is exited and whatever it is pointing at does no longer exist.
You should declare
ImageArray value;
function_B(frame, value);
and create a parameter for the function that takes that pointer as an argument.
Then you write to whatever that pointer is pointing at, and there is no need to return anything from the function.
ImageArray is a struct and in C++ returning a structure by value involves creating a copy of the structure.
➜ When a function returns a structure, a copy of the structure's data is typically made to transfer the value from the function to the caller and modern C++ compilers often optimize this process using Return Value Optimization (RVO) or Copy Elision, eliminating the need for an actual copy. With these optimizations, the returned structure is constructed directly in the caller's memory space, avoiding unnecessary copying.
I have not read the full code but I'd explore if the stack is not large enough for the operations ( The function returns an ImageArray structure by value, which could lead to performance issues because copying such a large structure (160x120 bytes = 19,200 bytes) is expensive. While modern compilers may optimize this, it's still unnecessary for this case).
Think is that what is returned is the pointer, which is then replacing the pointer to the structure that does exist, with a pointer to a structure that doesn't exist anymore.
I do believe there is anyway a maximum size to any variable created on the stack which i think is 4KB. and anything bigger should be declared on the heap (could be that that is the limit for esp8266 not esp32 again i am not fully sure on this)
(you could add a constructor and destructor to see what's happening)
EDIT: here you go if you want to see what's going on
struct Message {
char msg[10];
size_t id;
static size_t counter; // Static counter to track instances
// Constructor
Message() {
id = counter++;
Serial.print("Message ID: "); Serial.print(id);
Serial.print(" constructed. Total live instances: ");
Serial.println(counter);
Serial.flush();
}
// Destructor
~Message() {
counter--;
Serial.print("Message ID: "); Serial.print(id);
Serial.print(" destructed. Remaining instances: ");
Serial.println(counter);
Serial.flush();
}
};
// Initialize static member
size_t Message::counter = 0;
Message fillMsg() {
Serial.println("in fillMsg(), just before creating a local instance");
Message aMessage;
strlcpy(aMessage.msg, "HELLO", 10);
Serial.println("in fillMsg(), just before returning the local instance by copy");
return aMessage;
}
void setup() {
Serial.begin(115200);
Serial.println();
Serial.println("In setup, creating first instance");
Message message;
Serial.println("Before calling fillMsg()");
message = fillMsg();
Serial.println("After calling fillMsg()");
Serial.println(message.msg);
}
void loop() {}
➜ you'll see
In setup, creating first instance
Message ID: 0 constructed. Total live instances: 1
Before calling fillMsg()
in fillMsg(), just before creating a local instance
Message ID: 1 constructed. Total live instances: 2
in fillMsg(), just before returning the local instance by copy
Message ID: 1 destructed. Remaining instances: 1
After calling fillMsg()
HELLO
Message ID: 1 destructed. Remaining instances: 0
it's interesting to see that you see a second time "Message ID: 1 destructed", that's because we returned by copy the message ID 1 and so we have overwritten the original ID 0 from the instance in the setup.
Also we don't see a third copy being created by the return, so the compiler is likely using the local memory of the instance in the setup directly as discussed above (Return Value Optimization or Copy Elision)
Anyway, the OP should say what he is trying to do. It does look rather like the code that causes the crash was added because he got a compiler error with his original attempt to pass the array he was manipulating out of the function.
Thanks for all the great discussion above. It is very helpful. I should have mentioned one other thing. The struct returns from function_B without causing a crash if I comment out the code below. The array commented out is static so I don't see why it is causing the crash. I added that code just for debugging purposes. I was just trying to figure out what is causing the crash. The main thing I was trying to do is find the best way to return a 2D array from a function. In my original code, I was assigning the struct data in the nested for loop.
I am not a memory guru, but shouldn't the 'static' put new_array in the SRAM? The esp32-cam board has 4MB and the array (new_array) should be about 20kB. I have to believe something is going on with the memory since adding this code or commenting it out causes the reboot crash.
// static uint8_t new_array[HEIGHT][WIDTH] = {{0}};
// for (int h = 0; h < HEIGHT; h++)
// {
// for (int w = 0; w < WIDTH; w++)
// {
// int position = h * (len / HEIGHT) + w;
// new_array[h][w] = {frame->buf[position]};
// }
// }
I don't understand why you're bothering to pass all that data around as a return value anyway. Just create the array locally in the calling function and pass it by reference. Something like:
template<size_t NCOLS, size_t NROWS>
void function_B(camera_fb_t *frame, uint8_t (&arrayToFill)[NCOLS][NROWS]);
bool DSP_testing();
bool DSP_testing() {
camera_fb_t *frame = NULL;
frame = esp_camera_fb_get();
if (!frame) {
return false;
}
uint8_t imageArray[120][160];
function_B(frame, imageArray);
//
// Do something with imageArray here
//
return true;
}
template<size_t NCOLS, size_t NROWS>
void function_B(camera_fb_t *frame, uint8_t (&arrayToFill)[NCOLS][NROWS]) {
size_t len = frame->len;
for (size_t h = 0; h < NCOLS; h++) {
for (size_t w = 0; w < NROWS; w++) {
size_t position = h * (len / NCOLS) + w;
arrayToFill[h][w] = {frame->buf[position]};
}
}
}
First of all if the image is in frame->buf, in the correct order (as it seems to be) then why do you insist on copying it byte per byte. Unless you just want a smaller section of that image.
also what do the curly braces actually do in this case
new_array[h][w] = {frame->buf[position]};
and so if the 'frame' has more width you will skip some bytes with this calculation ?
int position = h * (len / HEIGHT) + w;
I was suggesting something like that, and thinking it was the only way. Thinking about it i think it is still the way to go. It should be faster as well.