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

Add support for OV5640 camera #3063

Open
wants to merge 14 commits into
base: rolling
Choose a base branch
from
Open

Conversation

jasaw
Copy link
Contributor

@jasaw jasaw commented May 6, 2024

Add support for OV5640 5MP camera. The camera has a resolution of 2560 x 1920 but the output will be scaled down to the configured resolution e.g. 640 x 480. When zoom is enabled, a 640 x 480 window is cropped from the full 2560 x 1920 frame, therefore giving double the magnification compared to OV2640.

This is a zoom comparison between OV5640 and OV2640.

OV5640:
ov5640

OV2640:
ov2640

@jasaw jasaw mentioned this pull request May 6, 2024
@caco3
Copy link
Collaborator

caco3 commented May 6, 2024

what is the impact on the RAM usage?
Does it need to store a larger framebuffer for the higher resolution?

@jasaw
Copy link
Contributor Author

jasaw commented May 6, 2024

@caco3 There's no impact on RAM usage at all. It works the same way as ov2640 where cropping is done on the camera module itself.

@SybexX
Copy link
Collaborator

SybexX commented May 6, 2024

Process:

  1. The camera takes an image with a resolution of 2560x1920
  2. The camera cuts out the desired area from the image
  3. The camera scales the cropped area to 640x480, if needed (desired area is larger 640x480)
    This process applies both when using the zoom function and when zoom is deactivated.

However, further changes to the implementation are necessary, e.g. at the sharpness....

Ablauf:

  1. Die Kamera nimmt ein Bild mit einer Auflösung von 2560x1920 auf
  2. Die Kamera schneidet den gewünschten Bereich aus dem Bild aus
  3. Die Kamera skaliert den ausgeschnittenen Bereich bei Bedarf auf 640x480 (wenn der ausgeschnittene Bereich größer 640x480 ist).
    Dieser Ablauf wird sowohl bei der Nutzung der Zoom-Funktion als auch bei deaktivierten Zoom benutzt/verwendet.

Allerdings sind noch weitere Änderungen an der Implementierung notwendig, z.B. bei sharpness.....

@jasaw
Copy link
Contributor Author

jasaw commented May 7, 2024

However, further changes to the implementation are necessary

@SybexX May I know what you would like to change?

@SybexX
Copy link
Collaborator

SybexX commented May 7, 2024

e.g. so:
sharpness

I made a few quick adjustments, but I don't know if it works because I don't have an OV5640.
ClassControllCamera.zip

@jasaw
Copy link
Contributor Author

jasaw commented May 9, 2024

@SybexX I've adjusted the sharpness handling to your version and it's working fine on my OV5640 system. I changed the sharpness range because the officially supported sharpness range is from -3 to +3.
_sharpnessLevel = min(2, max(-2, _sharpnessLevel));

Your ClassControllCamera code enforces 4:3 ratio on the zoom parameters, but OV5640 is a lot more flexible. It doesn't have the 4:3 restriction. Currently this PR only supports zoom mode 0, but it might be possible to achieve other zoom modes by passing different parameters to set_res_raw with scaling and binning set to true. That needs more experimentation, which I think could be left for future enhancement.

@SybexX
Copy link
Collaborator

SybexX commented May 9, 2024

I specifically set the sharpnessLevel to 2.
After longer tests, I found that a value of 3 sometimes causes problems.

I know that the OV5640 can do more, but we should adapt everything to the OV2640,
otherwise we would need at least two pages, i.e. one for each camera model and it simply becomes too confusing for the user.

Sometimes less is more^^

@SybexX
Copy link
Collaborator

SybexX commented May 9, 2024

kamera

@jasaw
Copy link
Contributor Author

jasaw commented May 9, 2024

@SybexX Thank you for reviewing. I've refactored my code based on your sample code but had to make a few changes to make it work with the OV5640.

only else, otherwise the other supported camera models will be ignored

My apologies, it is not clear to me that OV3660 (which is the other supported camera) behaves the same way as OV2640 and parts of the code are hitting the OV2640 registers directly (presumably the OV3660 has the same registers as OV2640?). Personally, I prefer to be more explicit in the code when it comes to handling hardware specific behaviour, like

if (sensor_info->model == CAMERA_OV5640)
{
}
else if ((sensor_info->model == CAMERA_OV2640) || (sensor_info->model == CAMERA_OV3660))
{
}

Either way, I am not particular with how the code is written as long as it is well documented via comments or code itself so other developers understand the intention of the author.

@SybexX
Copy link
Collaborator

SybexX commented May 9, 2024

The registers don't matter, the properties of the camera are more important.
The https://github.com/espressif/esp32-camera relieves us of having to pay attention to the registers.
Since no distinction was made between the camera models before I made my changes, I assumed that the OV2640 and OV3660 were very similar. After I quick look, the OV3660 is very close to the OV5640 in terms of functions.

@jasaw
Copy link
Contributor Author

jasaw commented May 9, 2024

The registers don't matter

ov2640_set_sharpness and ov2640_enable_auto_sharpness functions are called for OV3660 camera as well and eventually those functions call sensor->set_reg which writes directly to the OV3660's registers. Keep in mind that OV2640 sharpness is not officially supported by Espressif SDK. I haven't looked into the OV3660 datasheet, so I don't know what the camera will do if those registers don't exist or worse, have a different function. I don't have an OV3660 to test, so can't validate.

Since no distinction was made between the camera models before I made my changes, I assumed that the OV2640 and OV3660 were very similar

That was an oversight on my part as well. We should probably test sharpness against OV3660 and update the code accordingly?

@SybexX
Copy link
Collaborator

SybexX commented May 9, 2024

sharpness

If something is not available on a camera model, the esp32-camera library actually returns a -1

https://github.com/espressif/esp32-camera/blob/master/sensors/ov2640.c
https://github.com/espressif/esp32-camera/blob/master/sensors/ov3660.c
https://github.com/espressif/esp32-camera/blob/master/sensors/ov5640.c

@jasaw
Copy link
Contributor Author

jasaw commented May 9, 2024

I propose this sharpness handling code:

void CCamera::SetCamSharpness(bool _autoSharpnessEnabled, int _sharpnessLevel)
{
    sensor_t *s = esp_camera_sensor_get();

    if (s != NULL)
    {
        _sharpnessLevel = min(2, max(-2, _sharpnessLevel));

        camera_sensor_info_t *sensor_info = esp_camera_sensor_get_info(&(s->id));

        if (sensor_info != NULL)
        {
            if (sensor_info->model == CAMERA_OV5640 || sensor_info->model == CAMERA_OV3660)
            {
                if (_autoSharpnessEnabled)
                {
                    // autoSharpness is not supported, default to zero
                    s->set_sharpness(s, 0);
                }
                else
                {
                    s->set_sharpness(s, _sharpnessLevel);
                }
            }
            else if (sensor_info->model == CAMERA_OV2640)
            {
                // The OV2640 does not officially support sharpness, so the detour is made with the ov2640_sharpness.cpp.
                if (_autoSharpnessEnabled)
                {
                    s->set_sharpness(s, 0);
                    ov2640_enable_auto_sharpness(s);
                }
                else
                {
                    ov2640_set_sharpness(s, _sharpnessLevel);
                }
            }
        }
    }
    else
    {
        LogFile.WriteToFile(ESP_LOG_ERROR, TAG, "SetCamSharpness, Failed to get Cam control structure");
    }
}

@jasaw
Copy link
Contributor Author

jasaw commented May 9, 2024

@SybexX Looks like OV3660 set_res_raw function is the same as OV5640 as well. I should probably handle OV3660 explicitly.

@SybexX
Copy link
Collaborator

SybexX commented May 9, 2024

or so, then the ESP is spared the second comparison ^^

void CCamera::SetCamSharpness(bool _autoSharpnessEnabled, int _sharpnessLevel)
{
sensor_t *s = esp_camera_sensor_get();

if (s != NULL)
{
    _sharpnessLevel = min(2, max(-2, _sharpnessLevel));

    camera_sensor_info_t *sensor_info = esp_camera_sensor_get_info(&(s->id));

    if (sensor_info != NULL)
    {
        if (sensor_info->model == CAMERA_OV2640)
        {
            // The OV2640 does not officially support sharpness, so the detour is made with the ov2640_sharpness.cpp.
            if (_autoSharpnessEnabled)
            {
                s->set_sharpness(s, 0);
                ov2640_enable_auto_sharpness(s);
            }
            else
            {
                ov2640_set_sharpness(s, _sharpnessLevel);
            }
        }
        else
        {
			// for CAMERA_OV5640 and CAMERA_OV3660
            if (_autoSharpnessEnabled)
            {
                // autoSharpness is not supported, default to zero
                s->set_sharpness(s, 0);
            }
            else
            {
                s->set_sharpness(s, _sharpnessLevel);
            }
        }			
    }
}
else
{
    LogFile.WriteToFile(ESP_LOG_ERROR, TAG, "SetCamSharpness, Failed to get Cam control structure");
}

}

@SybexX
Copy link
Collaborator

SybexX commented May 9, 2024

        if (sensor_info->model == CAMERA_OV2640)
        {
            // The OV2640 does not officially support sharpness, so the detour is made with the ov2640_sharpness.cpp.
            if (_autoSharpnessEnabled)
            {
                s->set_sharpness(s, 0);	<<<<<<<<<< oh yes, this is actually not necessary, as it only returns a -1(not supported)
                ov2640_enable_auto_sharpness(s);
            }
            else
            {
                ov2640_set_sharpness(s, _sharpnessLevel);
            }
        }

@jasaw
Copy link
Contributor Author

jasaw commented May 9, 2024

@SybexX I have updated the code to your suggested sharpness handling and also updated zoom handling code to better deal with OV3660 but I don't have an OV3660 so can't test. I have only tested with OV5640 so far.

@SybexX
Copy link
Collaborator

SybexX commented May 9, 2024

            case CAMERA_OV5640:
                frameSizeX = 2560;
                frameSizeY = 1920;
                // max imageSize = ((frameSizeX - CCstatus.ImageWidth) / 8 / 4) - 1
                // 59 = ((2560 - 640) / 8 / 4) - 1
                if (imageSize < 59)
                {
                    _imageSize_temp = (59 - imageSize);
                }
                SanitizeZoomParams(_imageSize_temp, frameSizeX, frameSizeY, _imageWidth, _imageHeight, _offsetx, _offsety);
                SetCamWindow(s, unused, frameSizeX, frameSizeY, _offsetx, _offsety, _imageWidth, _imageHeight, CCstatus.ImageWidth, CCstatus.ImageHeight);
                break;

            case CAMERA_OV3660:
                frameSizeX = 2048;
                frameSizeY = 1536;
                // max imageSize = ((frameSizeX - CCstatus.ImageWidth) / 8 / 4) -1
                // 43 = ((2048 - 640) / 8 / 4) - 1
                if (imageSize < 43)
                {
                    _imageSize_temp = (43 - imageSize);
                }
                SanitizeZoomParams(_imageSize_temp, frameSizeX, frameSizeY, _imageWidth, _imageHeight, _offsetx, _offsety);
                SetCamWindow(s, unused, frameSizeX, frameSizeY, _offsetx, _offsety, _imageWidth, _imageHeight, CCstatus.ImageWidth, CCstatus.ImageHeight);
                break;

            case CAMERA_OV2640:
                // ov2640_sensor_mode_t _mode = OV2640_MODE_UXGA; // 1600x1200
                // ov2640_sensor_mode_t _mode = OV2640_MODE_SVGA; // 800x600
                // ov2640_sensor_mode_t _mode = OV2640_MODE_CIF;  // 400x296
                _mode = 0;
                frameSizeX = 1600;
                frameSizeY = 1200;
                // max imageSize = ((frameSizeX - CCstatus.ImageWidth) / 8 / 4) -1
                // 29 = ((1600 - 640) / 8 / 4) - 1
                if (imageSize < 29)
                {
                    _imageSize_temp = (29 - imageSize);
                }
                SanitizeZoomParams(_imageSize_temp, frameSizeX, frameSizeY, _imageWidth, _imageHeight, _offsetx, _offsety);
                // _mode sets the sensor resolution (3 options available),
                // _offsetx and _offsety set the start of the ROI,
                // _imageWidth and _imageHeight set the size of the ROI,
                // CCstatus.ImageWidth and CCstatus.ImageHeight set the output window size.
                SetCamWindow(s, _mode, frameSizeX, frameSizeY, _offsetx, _offsety, _imageWidth, _imageHeight, CCstatus.ImageWidth, CCstatus.ImageHeight);
                break;

@jasaw
Copy link
Contributor Author

jasaw commented May 10, 2024

@SybexX The new imageSize code that you shared above appears to be working well on my OV5640 setup and thanks for adding the comments to explain how the imageSize numbers came from because I had no idea previously.

I've also increased the OV5640 full frame size to match the datasheet.

@SybexX
Copy link
Collaborator

SybexX commented May 11, 2024

still necessary adjustment in the edit_reference.html:

        <td>
                <input required type="number" id="TakeImage_CamZoomSize_value1" value="0" min="0" max="59" step="1" onchange="cameraParameterChanged()"
                oninput="(!validity.rangeOverflow||(value=59)) && (!validity.rangeUnderflow||(value=0)) && (!validity.stepMismatch||(value=parseInt(this.value)));">
        </td>

still necessary adjustment in the edit_config_template.html:

		<td>
			<input required type="number" id="TakeImage_CamZoomSize_value1" value="0" min="0" max="59" step="1" onchange="cameraParameterChanged()"
			oninput="(!validity.rangeOverflow||(value=59)) && (!validity.rangeUnderflow||(value=0)) && (!validity.stepMismatch||(value=parseInt(this.value)));">
		</td>
		<td>$TOOLTIP_TakeImage_CamZoomSize</td>			
	</tr>

	<tr class="expert" unused_id="TakeImage_CamZoomOffsetX_ex3">
		<td class="indent1">
			<class id="TakeImage_CamZoomOffsetX_text" style="color:black;">Zoom Offset X</class>
		</td>
		<td>
			<input required type="number" id="TakeImage_CamZoomOffsetX_value1" value="0" min="-960" max="960" step="8" onchange="cameraParameterChanged()"
			oninput="(!validity.rangeOverflow||(value=960)) && (!validity.rangeUnderflow||(value=-960)) && (!validity.stepMismatch||(value=parseInt(this.value)));">Pixel
		</td>
		<td>$TOOLTIP_TakeImage_CamZoomOffsetX</td>
	</tr>

	<tr class="expert" unused_id="TakeImage_CamZoomOffsetY_ex3">
		<td class="indent1">
			<class id="TakeImage_CamZoomOffsetY_text" style="color:black;">Zoom Offset Y</class>
		</td>
		<td>
			<input required type="number" id="TakeImage_CamZoomOffsetY_value1" value="0" min="-720" max="720" step="8" onchange="cameraParameterChanged()"
			oninput="(!validity.rangeOverflow||(value=720)) && (!validity.rangeUnderflow||(value=-720)) && (!validity.stepMismatch||(value=parseInt(this.value)));">Pixel
		</td>
		<td>$TOOLTIP_TakeImage_CamZoomOffsetY</td>
	</tr>

@jasaw
Copy link
Contributor Author

jasaw commented May 12, 2024

@SybexX Thank you for testing with OV2640. I have been testing with OV5640 and haven't got time to test against OV2640.

I have incorporated your recommended changes. Thanks for fixing it and adding comments to explain how imageSize is supposed to work. I have added your comments in the code to help other developers.

I have tested your multi-step zoom and it is a very nice idea. I have to use scale and binning for OV5640 to work. I don't have an OV3660 so I can only assume it works the same way as OV5640.

I have also been testing the OV5640 in less than perfect lighting conditions and have to bump the max jpeg quality down to 18 because the camera is unstable below 18. I noticed you changed the max jpeg quality to 6 for OV2640. Is the OV2640 stable at 6 in less than perfect lightning conditions?

@SybexX
Copy link
Collaborator

SybexX commented May 12, 2024

I always test on the water meter and there is no problem with the quality = 6.
Have you tried increasing the frequency, it is set to 20MHz, but I think the OV5640 can go up to 27MHz

@jasaw
Copy link
Contributor Author

jasaw commented May 13, 2024

Have you tried increasing the frequency, it is set to 20MHz, but I think the OV5640 can go up to 27MHz

The OV5640 datasheet recommends 24MHz and I tried that but it didn't work properly. The image was always half black with some ghosting and the other half of the image was normal.

@jasaw
Copy link
Contributor Author

jasaw commented May 13, 2024

@SybexX I've checked other examples of running OV5640 on ESP32 and they are running 20MHz clock from what I can see. Not sure what's the maximum the ESP32 + OV5640 combo can handle.

I have just found out that both OV3660 and OV5640 are different from OV2640 in these areas as well:

  1. OV3660 and OV5640 support de-noise. The range is from 0 to 8 where zero is auto-denoise. OV2640 does not support de-noise.
  2. OV3660 and OV5640 auto-exposure range is from -5 to +5 whereas OV2640 range is from 1 to 5.
  3. OV3660 and OV5640 sharpness range is from from -3 to +3 but no auto-sharpness whereas we limit our OV2640 range to -2 to +2 and ov2640_set_sharpness takes -3 to +3.

What would you like to do? I'm leaning towards increasing the sharpness range to -3 to +3 (clip it to -2 to +2 for OV2640 with a comment saying -3 or +3 are unstable), increasing the auto-exposure range to -5 to +5 but clip it internally for OV2640, and add de-noise option on the web interface.

@SybexX
Copy link
Collaborator

SybexX commented May 13, 2024

Personally, I would limit myself to the functions of the OV2640.
Users won't change most of the settings anyway if the image quality is right.
That's why I only put the most necessary settings on the reference page and the rest is only available on the settings page.
It is best if @jomjol or @caco3 comments on what and how he imagines the implementation of the available settings and their setting range.

@friedpa
Copy link

friedpa commented May 13, 2024

I personnaly think that the AutoSharpness (if that is the same as AutoFocus) is most important, as a lot of users have difficulties with the manual lens adjustment. So it could be better to change to the new camera as a default hardware.

@SybexX
Copy link
Collaborator

SybexX commented May 13, 2024

AutoSharpness and AutoFocus are two different functions.
The problem is that every auto function can change the image quality every time and this making the evaluation a game of chance (in addition, this may place more strain on the ESP during alignment and evaluation).
For a reasonable and consistent evaluation, the image quality must always remain the same (In theory you would have to remove and deactivate every auto function).

@jasaw
Copy link
Contributor Author

jasaw commented May 13, 2024

For my gas meter and water meter set up, I can't have a fully enclosed housing because meter readers come around every couple of months to read the meters and they need to be able to look at the digits on the meters. What this means is my image is highly subjected to ambient lighting (sun light), so auto-exposure needs to be on. Other auto functions may need to be turned on to get an optimal image. I have also turned on negative effect (I used to have both grayscale and negative). I'm quite impressed that the neural net has been getting 95% of the readings correctly. Long story short, auto features are important if lighting is not constant.

Regarding the focus issue, I agree that it's hard to get the focus right, which is why I have been researching the OV5640 with autofocus. I think it will be a more user friendly solution. The user adjusts the focus through the web interface during edit-reference step and leave the focus at the configured level during normal operation (no auto-focus). We would need to proof that focus can be adjusted manually first. I have shared my research here: #2162 (comment)

As for the OV3660 and OV5640 specific features (de-noise and bigger auto-exposure range), I'm happy to just have them on the config page as expert settings.

@SybexX
Copy link
Collaborator

SybexX commented May 13, 2024

I adjusted a few things and also added Denoise(Config-Page), all the changed files are in the zip.
However, I haven't tested it yet^^
AI-on-the-edge-device_ov5640.zip

@jasaw
Copy link
Contributor Author

jasaw commented May 14, 2024

@SybexX I've incorporated your changes and cleaned the code up a little. My OV5640 is still working, which is good. The higher level of auto-exposure is definitely having an impact on the image. I haven't got time to test de-noise yet and how much impact it has on image quality.

@SybexX
Copy link
Collaborator

SybexX commented May 14, 2024

@jasaw I bought an OV5640 for testing and found that the colors red and blue swapped in the zoom function^^
I also had this problem with the OV2640 and only noticed it when I tested it in my apartment and not on the water meter^^

With this change they are back to normal for me:
s->set_res_raw(s, xOffset, yOffset, xOffset + xTotal + 1, yOffset + yTotal + 1, 0, 0, frameSizeX, frameSizeY, xOutput, yOutput, scale, binning);

Does your camera get so warm that the temperature of the ESP rises by around 6 to 10 degrees?
With the OV2640 I had around 34 degrees, now it's around 43 degrees^^

@jasaw
Copy link
Contributor Author

jasaw commented May 15, 2024

@SybexX I don't have swapped red blue colour problem on both my OV5640. I don't understand how s->set_res_raw(s, xOffset, yOffset, xOffset + xTotal + 1, yOffset + yTotal + 1, 0, 0, frameSizeX, frameSizeY, xOutput, yOutput, scale, binning) would make any difference.

The OV5640 does run a little warmer than the OV2640, so I stuck the camera against the microSD card holder using a thermal tape, using the ESP's PCB as a heatsink. I have 2 OV5640 systems, one of them running at 85 degrees (under direct sunlight), and another running at 37 degrees (shaded). I have just purchased a few heatsinks to put on the ESP32 metal can.

Could your camera red blue colour swap problem be caused by high temperature? How good is your power supply? The OV5640 is more sensitive to power supply fluctuation. I power my OV5640 systems from 12V to 5V DC-DC converter, capable of 3A output with 220uF output capacitor.

@SybexX
Copy link
Collaborator

SybexX commented May 15, 2024

It won't be the temperature, maybe the version of the camera, because I can set the quality to 8 on this camera without any problems (in very poor lighting conditions). With the OV2640 it's the other way around, there's a color swap if it's not divisible by 8 without a remainder.
With the standard settings I find the image overall to be significantly darker compared to the ov2640.
xOffset + xTotal + 1, yOffset + yTotal + 1 increase the ISP input size and therefore it is no longer divisible by 8 without remainder.

@jasaw
Copy link
Contributor Author

jasaw commented May 15, 2024

@SybexX I can't get the xOffset + xTotal + 1, yOffset + yTotal + 1 to work with my OV5640, the image is always corrupted. What happens when you do s->set_res_raw(s, xOffset + 1, yOffset + 1, xOffset + xTotal + 1, yOffset + yTotal + 1, 0, 0, frameSizeX, frameSizeY, xOutput, yOutput, scale, binning); ? Does it fix the colour swap issue?

This is my OV5640 as reported by the software:
MAIN: Camera info: PID: 0x5640, VER: 0x00, MIDL: 0x00, MIDH: 0x00

I noticed these differences between my ov2640 and ov5640:

  1. OV5640 images are darker like what you have found. My ov2640 images are always over-exposed, so ov5640 is better.
  2. In low light conditions, ov5640 images are very noisy compared to ov2640. I have to add 4 additional super bright white LEDs to my board to provide enough lighting to minimize noise.
  3. Auto-exposure algorithm on both the cameras are completely different. The OV5640 algorithm is less responsive to the auto-exposure level that I set (sometimes it feels like auto-exposure level has no effect).

@SybexX
Copy link
Collaborator

SybexX commented May 15, 2024

s->set_res_raw(s, xOffset + 1, yOffset + 1, xOffset + xTotal + 1, yOffset + yTotal + 1, 0, 0, frameSizeX, frameSizeY, xOutput, yOutput, scale, binning); works too.

I (4955) cam_hal: cam config ok
I (4995) ov5640: Set PLL: bypass: 0, multiplier: 180, sys_div: 4, pre_div: 2, root_2x: 0, pclk_root_div: 2, pclk_manual: 1, pclk_div: 4
I (4995) ov5640: Calculated XVCLK: 20000000 Hz, REFIN: 10000000 Hz, VCO: 1800000000 Hz, PLL_CLK: 180000000 Hz, SYSCLK: 45000000 Hz, PCLK: 11250000 Hz
I (5065) CAM: OV5640 camera module detected
I (7065) MAIN: Camera info: PID: 0x5640, VER: 0x00, MIDL: 0x00, MIDH: 0x00

WhatsApp Bild 2024-05-15 um 22 47 32_09744553

@jasaw
Copy link
Contributor Author

jasaw commented May 16, 2024

Interesting... s->set_res_raw(s, xOffset + 1, yOffset + 1, xOffset + xTotal + 1, yOffset + yTotal + 1, 0, 0, frameSizeX, frameSizeY, xOutput, yOutput, scale, binning); gives me a red tint over the image.

Update:
s->set_res_raw(s, xOffset + 4, yOffset + 4, xOffset + xTotal + 4, yOffset + yTotal + 4, 0, 0, frameSizeX, frameSizeY, xOutput, yOutput, scale, binning); works properly for me. Can you please test it on your OV5640?

Any good ideas on how we can narrow down the difference between your camera and mine? How should we handle this? I have only found this so far espressif/esp32-camera#447

This is my OV5640 camera:

I (3854) camera: Detected OV5640 camera
I (4544) cam_hal: cam config ok
I (4564) ov5640: Set PLL: bypass: 0, multiplier: 180, sys_div: 4, pre_div: 2, root_2x: 0, pclk_root_div: 2, pclk_manual: 1, pclk_div: 4
I (4564) ov5640: Calculated XVCLK: 20000000 Hz, REFIN: 10000000 Hz, VCO: 1800000000 Hz, PLL_CLK: 180000000 Hz, SYSCLK: 45000000 Hz, PCLK: 11250000 Hz
I (4604) CAM: OV5640 camera module detected
I (6604) MAIN: Camera info: PID: 0x5640, VER: 0x00, MIDL: 0x00, MIDH: 0x00

ov5640_closeup

@SybexX
Copy link
Collaborator

SybexX commented May 16, 2024

I've tried a few things, but as soon as the values ​​are divisible by 2 without a remainder, the colors are swapped.

@jasaw
Copy link
Contributor Author

jasaw commented May 16, 2024

@SybexX What's your Espressif SDK version?

Mine is this:

platformio run --environment esp32cam
Processing esp32cam (board: esp32cam; framework: espidf; platform: platformio/espressif32 @ 6.5.0)
--------------------------------------------------------------------------------
Verbose mode can be enabled via `-v, --verbose` option
CONFIGURATION: https://docs.platformio.org/page/boards/espressif32/esp32cam.html
PLATFORM: Espressif 32 (6.5.0) > AI Thinker ESP32-CAM
HARDWARE: ESP32 240MHz, 320KB RAM, 4MB Flash
DEBUG: Current (cmsis-dap) External (cmsis-dap, esp-bridge, esp-prog, iot-bus-jtag, jlink, minimodule, olimex-arm-usb-ocd, olimex-arm-usb-ocd-h, olimex-arm-usb-tiny-h, olimex-jtag-tiny, tumpa)
PACKAGES:
 - framework-espidf @ 3.50102.240122 (5.1.2)
 - tool-cmake @ 3.16.4
 - tool-esptoolpy @ 1.40501.0 (4.5.1)
 - tool-ninja @ 1.7.1
 - tool-riscv32-esp-elf-gdb @ 12.1.0+20221002
 - tool-xtensa-esp-elf-gdb @ 12.1.0+20221002
 - toolchain-esp32ulp @ 1.23500.220830 (2.35.0)
 - toolchain-xtensa-esp32 @ 12.2.0+20230208
Reading CMake configuration...

@SybexX
Copy link
Collaborator

SybexX commented May 16, 2024

Processing esp32cam (board: esp32cam; framework: espidf; platform: platformio/espressif32 @ 6.5.0)
--------------------------------------------------------------------------------------------------
Verbose mode can be enabled via `-v, --verbose` option
CONFIGURATION: https://docs.platformio.org/page/boards/espressif32/esp32cam.html
PLATFORM: Espressif 32 (6.5.0) > AI Thinker ESP32-CAM
HARDWARE: ESP32 240MHz, 320KB RAM, 4MB Flash
DEBUG: Current (cmsis-dap) External (cmsis-dap, esp-bridge, esp-prog, iot-bus-jtag, jlink, minimodule, olimex-arm-usb-ocd, olimex-arm-usb-ocd-h, olimex-arm-usb-tiny-h, olimex-jtag-tiny, tumpa)
PACKAGES:
 - framework-espidf @ 3.50102.240122 (5.1.2)
 - tool-cmake @ 3.16.4
 - tool-esptoolpy @ 1.40501.0 (4.5.1)
 - tool-idf @ 1.0.1
 - tool-mconf @ 1.4060000.20190628 (406.0.0)
 - tool-ninja @ 1.9.0
 - tool-riscv32-esp-elf-gdb @ 11.2.0+20220823
 - tool-xtensa-esp-elf-gdb @ 11.2.0+20230208
 - toolchain-esp32ulp @ 1.23500.220830 (2.35.0)
 - toolchain-xtensa-esp32 @ 12.2.0+20230208
Reading CMake configuration...

@jasaw
Copy link
Contributor Author

jasaw commented May 17, 2024

@SybexX I'm not sure what else we can do to track down the root cause of your camera red blue colour swap.
I have 2 hypothesis:

  1. Your camera is actually different from mine, either different hardware revision or different firmware.
  2. Your development environment is different from mine causing slightly different behaviour in the compiled firmware.

I have shared my firmware here for you to test to help isolate the problem.
This firmware also reads register 0x302A (Chip Revision) and the datasheet has 2 possible values for the process (0xA or 0xB), but my camera reports process value of zero.
CAM: OV5640 camera module detected. Chip revision 0, process 0

Other than this, I'm not sure what else we can do to track this issue down, then the next step is to probably have a red blue colour swap workaround flag that can be controlled via the web interface?

AI-on-the-edge-device__update__debug_20240517.zip

@SybexX
Copy link
Collaborator

SybexX commented May 17, 2024

me too^^
I (4594) CAM: OV5640 camera module detected. Chip revision 0, process 0

I assume it's due to the camera's firmware, your number has an 8 and mine has a B.
DC-5M21-5640-8-03-V1
DC-5M21-5640-B-03-V1

Since there are no differences when querying the version and revision, the only way to do this is via the settings page.

maybe like this: AI-on-the-edge-device-ov5640_support_changes.zip

@jasaw
Copy link
Contributor Author

jasaw commented May 18, 2024

@SybexX I've pushed your changes, however I did not comment out the code that limits the jpeg quality range. Was it commented out on purpose?

    if (CCstatus.CamSensor_id == OV5640_PID)
    {
        qual = min(63, max(18, qual)); // Limit quality from 18..63 (values lower than 20 tent to be unstable)
    }
    else
    {
        qual = min(63, max(8, qual)); // Limit quality from 8..63 (values lower than 8 tent to be unstable)
    }

@SybexX
Copy link
Collaborator

SybexX commented May 18, 2024

We already query the validity of the value in MainFlowControl.cpp and ClassFlowTakeImage.cpp and my OV5640 works with a value of 8 without any problems(even in poor lighting conditions), so I commented it out.
For the OV2640 you don't actually need a limit, for the OV5640 maybe like this:

            case OV5640_PID:
                if (CCstatus.isImageColorSwaped == true)
                {
			qual = min(63, max(8, qual));
                }
                else
                {
			qual = min(63, max(18, qual)); // Limit quality from 18..63 (values lower than 20 tent to be unstable)
                }			

                frameSizeX = 2592;
                frameSizeY = 1944;
                // max imageSize = ((frameSizeX - CCstatus.ImageWidth) / 8 / 4) - 1
                // 59 = ((2560 - 640) / 8 / 4) - 1
                if (imageSize < 59)
                {
                    _imageSize_temp = (59 - imageSize);
                }
                SanitizeZoomParams(_imageSize_temp, frameSizeX, frameSizeY, _imageWidth, _imageHeight, _offsetx, _offsety);
                SetCamWindow(s, frameSizeX, frameSizeY, _offsetx, _offsety, _imageWidth, _imageHeight, CCstatus.ImageWidth, CCstatus.ImageHeight);
                break;

@jasaw
Copy link
Contributor Author

jasaw commented May 19, 2024

@SybexX OK, I have updated the jpeg limits for our 2 variants of OV5640 and removed the OV2640 jpeg limit. Do you know whether OV3660 has a limit? I guess we just update the OV3660 limit when someone can actually test it? For now we just handle it the same as OV2640?

@SybexX
Copy link
Collaborator

SybexX commented May 19, 2024

So far I don't know anyone who has the OV3660, and if anyone does, to my knowledge there have never been any complaints about problems with the camera.
Therefore it probably doesn't matter whether it is treated like the OV5640 or OV2640^^

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.

None yet

4 participants