[MOD/#207] 이미지 압축 로직을 다시한번 개선합니다 :( #210
Conversation
angryPodo
requested review from
chattymin,
Roel4990 and
Hyobeen-Park
and removed request for
a team
| * 이 방식은 메모리 기반 측정보다 대용량 이미지 처리 시 OutOfMemory 위험을 낮춥니다. | ||
| */ | ||
| private fun measureCompressedSize(bitmap: Bitmap, quality: Int): Int { | ||
| val tempFile = File.createTempFile("measure", ".tmp") |
| bitmap | ||
| } | ||
| val orientation = getOrientation(uri) | ||
| if (orientation != ORIENTATION_NORMAL) { |
There was a problem hiding this comment.
사소하지만 if-else를 사용한다면 if문 내부에는 긍정을 적어주는게 가독성에 좋습니답
책 어디에 적혀있었는데 그건 기억이 안나네요~
There was a problem hiding this comment.
맞습니다! 단순 null체킹 하는 부분의 let을 제거하면서 저렇게 작성했었어요ㅎㅎ
그러나 EXIF관련 조정이 필요없어져서 삭제했습니다
| fun toFormData(name: String): MultipartBody.Part = | ||
| MultipartBody.Part.createFormData( | ||
| name = name, | ||
| filename = metadata?.fileName ?: DEFAULT_FILE_NAME, |
There was a problem hiding this comment.
서버 내부에서 어떻게 처리를 하는지 모르겠지만, 클라이언트에 저장하는 경우에 이름이 같으면 덮어쓰기를 해버려요. 물론 서버에서 이를 처리한다면 괜찮겠지만, 애초에 예방하는 방법으로 timestemp를 사용하는 것도 있답니다
There was a problem hiding this comment.
지금 동일 이미지로 여러번 테스트 했을때 덮어쓰기가 발생하지 않는 것을 확인했습니다.
따라서 지금은 timestamp를 추가할 필요가 없지만, 혹시 추후 파일명 충돌 이슈가 생긴다면 적용하겠습니다요
| @@ -60,85 +71,122 @@ class ContentUriRequestBody @Inject constructor( | |||
| } | |||
|
|
|||
| init { | |||
| uri?.let { | |||
| metadata = extractMetadata(it) | |||
| if (uri != null) { | |||
There was a problem hiding this comment.
- let을 if로 변경한 이유가 있을까요?
- init 부분에서 uri가 null이라면 아래에 있는 모든 로직이 안돌아갈 것 같아요. 코드를 자세히 읽지는 못했지만, uri가 null이라면 Exception을 early return 하는 방식도 있을 것 같네요
There was a problem hiding this comment.
- 단순
null체크에서는let보다if문이 가독성이 더 좋다고 생각하여 변경했습니다. uri가null일 경우 로직이 실행되지 않는 것은 의도된 부분이긴 합니다만 얼리리턴 생각하면 let으로 유지시켜도 될것 같아요..ㅋㅋㅋㅋ
There was a problem hiding this comment.
if (uri == null) {
return
}
metadata = extractMetadata(uri)
이렇게도 돼서 if문으로 얼리 리턴도 조아요~~
그리고 let의 경우 내부적으로 변수를 하나 만들어서 null check를 하기 때문에 단순한 null check 는 if문을 사용하는게 성능상 더 좋아서 남긴 코멘트입니다! null을 체이닝으로 계속 검사를 해야하거나, let을 쓰는게 가독성이 더 좋은 경우가 있는데 그때는 let을 사용하고 단순 null 체크의 경우 if를 쓰는게 더 조아요
물론... 성능에 거의 차이가 없다싶이해서 let 쓰는것도 전 좋다고 봐요~
| @@ -40,6 +42,15 @@ class ContentUriRequestBody @Inject constructor( | |||
| } | |||
| } | |||
|
|
|||
| /** | |||
| * 이미지 압축을 위한 설정값을 담는 데이터 클래스입니다. | |||
| * loadBitmap(): ImageDecoder를 사용하여 URI에서 비트맵을 로드합니다. | ||
| * 이미지 크기는 설정된 최대 크기(maxWidth, maxHeight) 이하로 리사이즈하며, | ||
| * EXIF 정보를 고려하여 회전이 필요한 경우 rotateBitmap()을 호출합니다. | ||
| */ | ||
| private suspend fun loadBitmap(uri: Uri): Result<Bitmap> = | ||
| withContext(Dispatchers.IO) { |
There was a problem hiding this comment.
이미 Dispatchers.IO에서 실행되고 있는 것 같아요. withContext로 나타낸 이유가 있을까요?
There was a problem hiding this comment.
이미 Dispatchers.IO에서 실행하지만 withContext 사용해 I/O 스레드에서의 실행을 보장할수있어서 따로 명시했습니다!
- 1MB 미만 파일은 압축을 건너뛰고 100% 품질을 사용하도록 `compressImage()` 리팩토링. - CPU 집약적 작업을 `Dispatchers.Default`에서 처리하고 메모리 효율적 압축을 위해 버퍼 크기를 동적으로 조정하도록 `compressBitmap()` 최적화. - 동일한 URI에 대한 중복 이미지 처리를 방지하기 위해 `ConcurrentHashMap`을 사용한 캐싱 추가. - `prepareImage()`에 대한 여러 동시 호출이 발생할 때 첫 번째 압축 작업의 결과를 공유하기 위한 지연 실행 메커니즘 구현. - 최소 버퍼 크기를 보장하기 위해 `max()` 함수 사용. - 메타데이터에 mimeType 추가.
| @Volatile | ||
| private var prepareImageDeferred: Deferred<Result<Unit>>? = null |
There was a problem hiding this comment.
와 진짜 잘 모르는건데 공부하겠습니다...
Related issue 🛠
Work Description ✏️
Uncompleted Tasks 😅
To Reviewers 📢
주석으로 바뀐 부분이나 저번에 수정한 부분 설명 첨부해뒀읍니다.
노션으로 문서도 만들긴했는데 좀더 첨삭해서 스푸니 노션에 올려 놓겠습니다요~
3/9 추가