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

Color distorted on ov5647 (AEGHB-907) #16

Open
3 tasks done
mutatrum opened this issue Dec 6, 2024 · 25 comments
Open
3 tasks done

Color distorted on ov5647 (AEGHB-907) #16

mutatrum opened this issue Dec 6, 2024 · 25 comments

Comments

@mutatrum
Copy link

mutatrum commented Dec 6, 2024

Checklist

  • Checked the issue tracker for similar issues to ensure this is not a duplicate
  • Read the documentation to confirm the issue is not addressed there and your configuration is set correctly
  • Tested with the latest version to ensure the issue hasn't been fixed

How often does this bug occurs?

always

Expected behavior

Proper color reproduction. I have a test image, picture takes with my phone:

image

Colors are good when running f.e. the simple_video_server_example:

image

Picture is flipped, but color reproduction is correct.

Actual behavior (suspected bug)

I'm trying to capture frames and show them on an LVGL 9 RGB565 canvas, on an ST7789 display. I push the frame to the canvas, I get the following result:

image

The display is only 320x240, and this is unscaled so it's a small crop of the frame buffer.

Error logs or terminal output

No response

Steps to reproduce the behavior

I've takes the setup code from the esp_video_server_example, set up the capture format:

    struct v4l2_format format = {
        .type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
        .fmt.pix.width = 800,
        .fmt.pix.height = 800,
        .fmt.pix.pixelformat = V4L2_PIX_FMT_RGB565,
    };

    if (ioctl(fd, VIDIOC_S_FMT, &format) != 0) {
        ESP_LOGE(TAG, "failed to set format");
        goto exit_0;
    }

Reading back the format shows it's set up as RGB565.

The deviation from the video_server_example is the capture loop, instead of reading the fd and push it into a JPEG converter, I push the frame directly onto an lv_image:

    lv_image_dsc_t img_dsc = {
        .header = {
            .magic = LV_IMAGE_HEADER_MAGIC,
            .cf = LV_COLOR_FORMAT_RGB565,
            .w = 800,
            .h = 800,
        }
    };

    while (1) {
        memset(&buf, 0, sizeof(buf));
        buf.type   = type;
        buf.memory = MEMORY_TYPE;

        esp_err_t res = ioctl(video_fd, VIDIOC_DQBUF, &buf);
        if (res != 0) {
            ESP_LOGE(TAG, "failed to receive video frame");
            break;
        }

        lvgl_port_lock(0);

        img_dsc.data_size = buf.bytesused;
        img_dsc.data = buffer[buf.index];
        lv_image_set_src(img1, &img_dsc);

        lv_refr_now(NULL);

        lvgl_port_unlock();

        if (ioctl(video_fd, VIDIOC_QBUF, &buf) != 0) {
            ESP_LOGE(TAG, "failed to free video frame");
        }
   }

Project release version

0.7.0

System architecture

Intel/AMD 64-bit (modern PC, older Mac)

Operating system

Linux

Operating system version

Ubuntu 24.04

Shell

Bash

Additional context

ESP32P4 Function Evaluation Board chip revision: v0.1

ESP-IDF v5.4-dev-4291-g7a305c0284-dirty 2nd stage bootloader

Dependencies:

[1/9] espressif/cmake_utilities (0.5.3)
[2/9] espressif/esp_cam_sensor (0.7.0)
[3/9] espressif/esp_h264 (1.0.4)
[4/9] espressif/esp_ipa (0.1.0)
[5/9] espressif/esp_lvgl_port (2.4.2)
[6/9] espressif/esp_sccb_intf (0.0.4)
[7/9] espressif/esp_video (0.7.0)
[8/9] lvgl/lvgl (9.2.2)
[9/9] idf (5.5.0)

Maybe similar to #8 ?

The only setting in sdkconfig.default is CONFIG_CAMERA_OV5647=y, the rest is not altered from default value.

@github-actions github-actions bot changed the title Color distorted on ov5647 Color distorted on ov5647 (AEGHB-907) Dec 6, 2024
@mutatrum
Copy link
Author

mutatrum commented Dec 7, 2024

To rule out the display side, I've added a few coloured squares directly into the buffer, which seems to work. Colors are reproduced correctly:

image

I've tried byte endian swap, bit endian swap, but no luck getting the output right. If I read through the esp_video examples, which all seem to product correct color reproduction, I don't see anything to suggest endianness issues or other pixel layouts, but maybe I'm missing something.

@WangYuxin-esp
Copy link
Collaborator

This is a known issue that is currently being fixed. I believe there will be a new MR this week to solve this issue.

@mutatrum
Copy link
Author

mutatrum commented Dec 9, 2024

Thank you, looking forward to the update. When it's released I'll test it and report back to close the issue.

I also wanted to say thanks, these components are an impressive feat and makes handling camera operations so much more feasible!

@mutatrum
Copy link
Author

mutatrum commented Dec 10, 2024

I might be impatient and it's not complete, but I just tested 6f07326 and the color distortion looks the same. This is with MIPI_2lane_24Minput_RAW8_800x800_50fps. I tried changing the bayer format, but they all looked the same, so maybe something else is still missing.

I tested all formats, the RAW8 are all the same, the RAW10 formats produce complete noise.

While writing this, I did some more testing and tried to set V4L2_CID_HFLIP back to 0. That fixed the colors! HFLIP (and I assume VFLIP as well) flip the picture before the bayer transform, so the format goes from GBRG to something else. Logically would be BGGR, but that doesn't yield correct results. I tried all the other bayer formats, but all of them have distorted colors with HLIP=1, so there might be something else interfering with the demosaicing.

Now it also makes sense why the demos worked, as they do not flip the image!

@mutatrum
Copy link
Author

mutatrum commented Dec 10, 2024

A second thing I noticed with this change is that it looks like the auto-exposure is turned off on some formats. It's very easy to completely blow out the picture by adding a light.

@WangYuxin-esp
Copy link
Collaborator

Yes, the installation direction of the sensor lens affects the configuration of some ISPs.
To facilitate problem identification, you can take a 24 color chart to help solve the problem.
24color_checker

@mutatrum
Copy link
Author

It's easy to reproduce with esp_video_server by adding

    struct v4l2_ext_controls controls;
    struct v4l2_ext_control control[1];

    controls.ctrl_class = V4L2_CTRL_CLASS_USER;
    controls.count      = 1;
    controls.controls   = control;
    control[0].id       = V4L2_CID_HFLIP;
    control[0].value    = 1;
    if (ioctl(fd, VIDIOC_S_EXT_CTRLS, &controls) != 0) {
        ESP_LOGW(TAG, "failed to mirror the frame horizontally and skip this step");
    }    

to the end of app_video_open. That yields this result:

image

With value 0 (no HFLIP) this is the result:

image

I'll also test with the SC2336 camera.

@WangYuxin-esp
Copy link
Collaborator

Yes, if flip is enabled, this may require updating its bayer fmt. Because this flip feature comes from the sensor, it affects the Bayer format of RAW format.
If you really need the flip function, you can customize a register list at the application layer and use it through cmd VIDIOC_S_SENSOR_FMT.

@Horion0415
Copy link

To rule out the display side, I've added a few coloured squares directly into the buffer, which seems to work. Colors are reproduced correctly:

image

I've tried byte endian swap, bit endian swap, but no luck getting the output right. If I read through the esp_video examples, which all seem to product correct color reproduction, I don't see anything to suggest endianness issues or other pixel layouts, but maybe I'm missing something.

Hello, I noticed that the LCD you are using is the ST7789. For SPI interface displays like this, it is often necessary to swap the two bytes of the RGB565 format. In LVGL v8, this feature was available as an optional configuration (as shown in the image). However, since you are using LVGL v9, this configuration has been removed, and you need to manually call the lv_draw_sw_rgb565_swap interface to handle byte swapping. Therefore, I believe the color abnormality in this case is caused by the byte order issue. As for the mirror effect, it needs to be addressed on the camera sensor side.
企业微信截图_17338883614478

@WangYuxin-esp
Copy link
Collaborator

The new submission allows opening IPA to load default ISP configuration.
enable_pipeline

@mutatrum
Copy link
Author

Hello, I noticed that the LCD you are using is the ST7789. For SPI interface displays like this, it is often necessary to swap the two bytes of the RGB565 format. In LVGL v8, this feature was available as an optional configuration (as shown in the image). However, since you are using LVGL v9, this configuration has been removed, and you need to manually call the lv_draw_sw_rgb565_swap interface to handle byte swapping. Therefore, I believe the color abnormality in this case is caused by the byte order issue. As for the mirror effect, it needs to be addressed on the camera sensor side.

Color reproduction with LVGL 9.2 is good for all other screens, including when the camera is not swapped. I'm using esp_lvgl_port2.4.3, which has swap_bytes` as a setup parameter:

image

This also shows on the picture with the small RGB squares, they come out as intended. If I disable swap_bytes, there is a lot of color banding, as high and low significant bits are mangled.

@mutatrum
Copy link
Author

mutatrum commented Dec 11, 2024

Yes, if flip is enabled, this may require updating its bayer fmt. Because this flip feature comes from the sensor, it affects the Bayer format of RAW format. If you really need the flip function, you can customize a register list at the application layer and use it through cmd VIDIOC_S_SENSOR_FMT.

IMO horizontal flip is a mandatory feature, as it's usage is determined which way the camera is facing. If the camera is pointing the same way as the screen (e.g. a 'selfie' mode), the video doesn't need to be flipped, and the image makes sense as if you're looking into a mirror. If the camera is pointing through the screen (by lack of better terminology), the video needs to be flipped, otherwise everything is mirrored where it shouldn't be.

I have the camera facing away from the screen and everything is mirrored:

image

I hope this makes sense. I can try to fix the bayer demosaic for this sensor, but that would negate the tremendous advantage of the esp_video components with support for so many different sensors.

Addition: you can test this with your phone. If you use the selfie camera the preview is mirrored. If you use the back camera the preview is not mirrored.

@WangYuxin-esp
Copy link
Collaborator

WangYuxin-esp commented Dec 12, 2024

Yes, the mirroring function affects the format of Bayer. Regarding the issue of sensor initialization, we made a bold attempt to allow the initialization list to be placed in the application layer code instead of always relying on the initialization list in esp-video.

You can customize an initialization list and modify the bayer fmt in the structure describing the initialization list.

@mutatrum
Copy link
Author

If the bayer format should be adjusted, that wouldn't happen in the sensor initialisation, but in the ISP pipeline, or am I mistaken? I've seen the ISP controller, but I'm not sure how to go about that to adjust the bayer demosaic configuration, or I haven't found an example of this yet.

@WangYuxin-esp
Copy link
Collaborator

Your understanding is completely correct, we need to modify the configuration of bayer in the ISP.
This only requires changing the option in the description of this register list, and the background program will automatically initialize the corresponding ISP. Just refer to this example.

@mutatrum
Copy link
Author

mutatrum commented Dec 13, 2024

I tried setting bayer_type, but it looks like it's not being picked up, or something else is not working. I added the following code, right after esp_video_init:

static esp_cam_sensor_format_t custom_format_info;
static esp_cam_sensor_format_t custom_format_info2;
static esp_err_t init_format(int fd)
{
    memset(&custom_format_info, 0, sizeof(custom_format_info));
    if (ioctl(fd, VIDIOC_G_SENSOR_FMT, &custom_format_info) != 0) {
        ESP_LOGE(TAG, "Error reading sensor format");
    	return ESP_FAIL;
    }

    ESP_LOGI(TAG, "name: %s", custom_format_info.name);
    ESP_LOGI(TAG, "bayer_type: %i", custom_format_info.isp_info->isp_v1_info.bayer_type); // output: 2

    esp_cam_sensor_isp_info_t custom_fmt_isp_info = {
        .isp_v1_info = {
            .version = custom_format_info.isp_info->isp_v1_info.version,
            .pclk = custom_format_info.isp_info->isp_v1_info.pclk,
            .vts = custom_format_info.isp_info->isp_v1_info.vts,
            .hts = custom_format_info.isp_info->isp_v1_info.hts,
            .gain_def = custom_format_info.isp_info->isp_v1_info.gain_def,
            .exp_def = custom_format_info.isp_info->isp_v1_info.exp_def,
            .bayer_type = ESP_CAM_SENSOR_BAYER_BGGR, // value: 3
        }
    };

    custom_format_info.isp_info = &custom_fmt_isp_info;

    ESP_LOGI(TAG, "set bayer_type: %i", custom_format_info.isp_info->isp_v1_info.bayer_type); // output: 3

    /* Set custom register configuration */
    if (ioctl(fd, VIDIOC_S_SENSOR_FMT, &custom_format_info) != 0) {
        ESP_LOGE(TAG, "Error setting sensor format");
        return ESP_FAIL;
    }

    memset(&custom_format_info2, 0, sizeof(custom_format_info2));
    if (ioctl(fd, VIDIOC_G_SENSOR_FMT, &custom_format_info2) != 0) {
        ESP_LOGE(TAG, "Error reading sensor format");
    	return ESP_FAIL;
    }
    ESP_LOGI(TAG, "verify bayer_type: %i", custom_format_info2.isp_info->isp_v1_info.bayer_type); // output: 3

It sets the bayer_type from GBRG (2) to BGGR (3), and verifies it's actually set. However, nothing changes in the image output. I can set bayer_type to any value, even illegal values, the output stays the same.

When I do the same experiment with HFLIP off, the colors are correct with every bayer_type. However, I would have expected the colors to break if I set the wrong bayer_type.

@WangYuxin-esp
Copy link
Collaborator

If various bayer types are tried, the color is not normal. So this issue does not come from sensors or ISP. Perhaps it was mentioned earlier that the byte order of the LCD needs to be adjusted.
You can try using the simple web example to test it. This avoids the impact of LCD peripherals.

@mutatrum
Copy link
Author

mutatrum commented Dec 16, 2024 via email

@mutatrum
Copy link
Author

mutatrum commented Dec 16, 2024

This is a frame coming from esp_video_example, with ov5647, RAW8 800x800 50fps, MIPI 2lane 24M input with bayer type BGGR and HFLIP 1. This way the orientation is correct, but the colors are not:

image

Bayer type BGGR and HFLIP 0. Here the colors are correct - dispite trying to override the bayer type. The orientation is not correct:

image

Other bayer types have no influence on the picture.

I added the following code at the end of app_video_open of the example:

    static esp_cam_sensor_format_t custom_format_info;

    memset(&custom_format_info, 0, sizeof(custom_format_info));
    if (ioctl(fd, VIDIOC_G_SENSOR_FMT, &custom_format_info) != 0) {
        ESP_LOGE(TAG, "Error reading sensor format");
    	return ESP_FAIL;
    }

    ESP_LOGI(TAG, "name: %s", custom_format_info.name);
    ESP_LOGI(TAG, "bayer_type: %i", custom_format_info.isp_info->isp_v1_info.bayer_type); // output: 2

    esp_cam_sensor_isp_info_t custom_fmt_isp_info = {
        .isp_v1_info = {
            .version = custom_format_info.isp_info->isp_v1_info.version,
            .pclk = custom_format_info.isp_info->isp_v1_info.pclk,
            .vts = custom_format_info.isp_info->isp_v1_info.vts,
            .hts = custom_format_info.isp_info->isp_v1_info.hts,
            .gain_def = custom_format_info.isp_info->isp_v1_info.gain_def,
            .exp_def = custom_format_info.isp_info->isp_v1_info.exp_def,
            .bayer_type = ESP_CAM_SENSOR_BAYER_BGGR, // value: 3
        }
    };

    custom_format_info.isp_info = &custom_fmt_isp_info;

    if (ioctl(fd, VIDIOC_S_SENSOR_FMT, &custom_format_info) != 0) {
        ESP_LOGE(TAG, "Error setting sensor format");
        return ESP_FAIL;
    }

    struct v4l2_ext_controls controls;
    struct v4l2_ext_control control[1];

    controls.ctrl_class = V4L2_CTRL_CLASS_USER;
    controls.count      = 1;
    controls.controls   = control;
    control[0].id       = V4L2_CID_HFLIP;
    control[0].value    = 1;
    if (ioctl(fd, VIDIOC_S_EXT_CTRLS, &controls) != 0) {
        ESP_LOGW(TAG, "failed to mirror the frame horizontally and skip this step");
    }    

I hope this helps.

@WangYuxin-esp
Copy link
Collaborator

WangYuxin-esp commented Dec 16, 2024

Thank you for reporting.
The current data flow is as follows: Sensor RAW data-> ISP RGB\YUV data->Application framebuffer.
The functionality of Flip can come from the sensor, which directly changes the direction of RAW data. The Flip function can also come from ESP32P4, which changes the direction after obtaining RGB and YUV data.
For the current issue, it is actually related to the installation direction of your sensor's FPC. The default configuration of the sensors here is normal.
FPC_Install2

In addition, I added testing by installing my camera upside down on my development board. When the Flip function from the sensor is enabled, the color will be abnormal. And tried various bayer types in different situations, but could not fix the abnormality of this color.
My suggestion is to check the FCP direction of your sensor, which is the main reason. Then make sure the color is normal first, and finally try using PPA on the host to rotate or flip the image.

@mutatrum
Copy link
Author

Thank you very much for all your efforts on this issue. The camera is a Raspberry Pi Camera v1.3 connected with a reverse direction FPC cable, so it is indeed like the one to the right on your photo:

image

I understand the Flip function changes the RAW pixel data, before the bayer demosaic. I don't know where exactly this is done, as I couldn't find the code for this, and it looks like the current bayer formats are not matching whatever the sensor is sending out. I guess it's not possible to configure a complete bayer demosaic matrix configuration?

I do think either direction should work fine, as it depends on the orientation of the camera vs the display, so with the other camera in your photo I think you would need a horizontal flip if it would be used as a selfie camera.

As I'm planning to have support for multiple camera modules in my project, using PPA to fix orientation and mirroring would be a better option, at least for now. I did look into it, but it looked quite complicated :-)

Before closing this issue I have one more question: are there plans to support conversion to L8 (greyscale) with PPA? This would be very useful for f.e. QR decoding as that needs greyscale input. I can do this in pure C, but that takes quite a hit on the framerate.

Once more thank you very much for all your input, it is highly appreciated. I'm very impressed with the video components, they do a lot of very heavy lifting!

@WangYuxin-esp
Copy link
Collaborator

Yes, because the FPC installation direction is different, we cannot find a matching bayer type for the sensor you are currently using. Adjusting the installation direction of FPC or using PPA to complete rotation is a good choice.
Your suggestion is very meaningful, and we will consider adding support for grayscale images in PPA.
Finally, we will support dual cameras on ESP32-P4 and have related development boards available for developers to use. Thank you again for your feedback.

@mutatrum
Copy link
Author

I found a solution! If I set register 0x3821 to 0x07 instead of 0x03, the colors are correct and the image is no longer flipped.

This sets Bit[2], which according to the datasheet enables r_mirror_isp.

image

I'm not 100% sure, but I think before it has to do something with the combination of binning and mirroring. So, I think the HFLIP and possibly also VFLIP needs to toggle more than just bit 1, but I need to test a bit further for this.

image

@mutatrum
Copy link
Author

Picture of the test setup, with the Pi Camera v1.3, the ESP32P4 Eval board on the left and my phone on the background showing the color chart you posted earlier. In front is the ST7789 display showing the live feed, with correct orientation.

image

One more question I forgot in my excitement: what do you mean by 'adjusting the installation direction of FPC'? I can't put another cable on it, as that doesn't work electrically, but maybe I'm misunderstanding you.

@WangYuxin-esp
Copy link
Collaborator

As shown in the picture, try folding the FPC flexible cable to the back of the development board:
back_install

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

No branches or pull requests

3 participants