Skip to content

Conversation

tore-espressif
Copy link
Collaborator

@tore-espressif tore-espressif commented May 5, 2025

Change description

Added new public API to get image size without decoding it.

This is very useful for scenarios when we have a JPEG encoded image, but we do not know what resolution does it have, thus we don't know how big output buffer we need for decoding. This scenario is added to test-cases.

This is the last missing feature, before we can integrate this component to esp32-camera, in espressif/esp32-camera#740

@tore-espressif tore-espressif self-assigned this May 5, 2025
@tore-espressif
Copy link
Collaborator Author

cc @igrr since you reviewed some esp_jpeg PRs in the past :)

@tore-espressif tore-espressif requested review from espzav and igrr May 5, 2025 11:55
Copy link

github-actions bot commented May 5, 2025

Test Results

15 files  15 suites   1m 3s ⏱️
 5 tests  5 ✅ 0 💤 0 ❌
25 runs  25 ✅ 0 💤 0 ❌

Results for commit 3f723c5.

♻️ This comment has been updated with latest results.

Copy link
Member

@igrr igrr left a comment

Choose a reason for hiding this comment

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

May I suggest extending esp_jpeg_image_output_t with a buffer_size field, and instead of esp_jpeg_get_output_size adding something like esp_jpeg_get_image_info, returning esp_jpeg_image_output_t? Then we can get both the buffer size and image dimensions before actually calling the decode function.

Similar to what libpng allows doing:

TEST_ASSERT(png_image_begin_read_from_memory(&image, buf, buf_len));
image.format = PNG_FORMAT_GRAY;
int stride = PNG_IMAGE_ROW_STRIDE(image);
int buf_size = PNG_IMAGE_SIZE(image);
TEST_ASSERT_EQUAL(expected_width, image.width);
TEST_ASSERT_EQUAL(expected_height, image.height);

@@ -119,6 +119,50 @@ esp_err_t esp_jpeg_decode(esp_jpeg_image_cfg_t *cfg, esp_jpeg_image_output_t *im
return ret;
}

#define LDB_WORD(ptr) (unsigned short)(((unsigned short)*((unsigned char*)(ptr))<<8)|(unsigned short)*(unsigned char*)((ptr)+1))
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps you can use a slightly more readable function

static inline uint16_t ldb_word(const void* ptr) {
  uint16_t result;
  memcpy(&result, ptr, sizeof(result);
  return result;
}

the compiler will optimize out memcpy replacing it with either an unaligned load or a pair of byte loads, depending on whether the CPU supports unaligned loads or not.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@igrr I ended up with the following implementation to preserve the big-endian to little-endian transformation. All other comments have been incorporated!

static inline uint16_t ldb_word(const void *ptr)
{
    const uint8_t *p = (const uint8_t *)ptr;
    return ((uint16_t)p[0] << 8) | p[1];
}

@tore-espressif
Copy link
Collaborator Author

tore-espressif commented May 5, 2025

@igrr Thank you for the review, will update soon! EDIT: done

@tore-espressif tore-espressif force-pushed the feature/esp_jpeg_prepare branch from f58ef75 to 3f723c5 Compare May 5, 2025 15:53
@tore-espressif tore-espressif merged commit acb5249 into master May 7, 2025
49 checks passed
@tore-espressif tore-espressif deleted the feature/esp_jpeg_prepare branch May 7, 2025 16:21
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.

2 participants