-
Notifications
You must be signed in to change notification settings - Fork 0
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
[REF/#198] ContentUriRequest Class의 이미지 압축 방식을 개선합니다. #200
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
재밌는거 잘 하셨네요 크크
ImageMetadata.EMPTY | ||
} | ||
} ?: ImageMetadata.EMPTY | ||
}.getOrDefault(ImageMetadata.EMPTY) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p4) 위에 import하면 이렇게도 접근 가능하답니다~ 그런데 명시적으로 표시하기 위해 import하지 않는것도 좋은 방법이라 생각해서 참고만 하시길!
ImageMetadata.EMPTY | |
} | |
} ?: ImageMetadata.EMPTY | |
}.getOrDefault(ImageMetadata.EMPTY) | |
EMPTY | |
} | |
} ?: EMPTY | |
}.getOrDefault(EMPTY) |
@RequiresApi(Build.VERSION_CODES.Q) | ||
private suspend fun loadBitmapWithImageDecoder(uri: Uri): Result<Bitmap> = | ||
withContext(Dispatchers.IO) { | ||
runCatching { | ||
val source = ImageDecoder.createSource(contentResolver, uri) | ||
ImageDecoder.decodeBitmap(source) { decoder, info, _ -> | ||
decoder.allocator = ImageDecoder.ALLOCATOR_SOFTWARE | ||
decoder.isMutableRequired = true | ||
calculateTargetSize(info.size.width, info.size.height).let { size -> | ||
decoder.setTargetSize(size.width, size.height) | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
신기하네여~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
근데 지금 찾아보니 ImageDecoder 도입이 API 28이네요! 분기 제거해도 될듯..????
private suspend fun loadBitmap(uri: Uri): Result<Bitmap> = | ||
withContext(Dispatchers.IO) { | ||
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { | ||
loadBitmapWithImageDecoder(uri) | ||
} else { | ||
loadBitmapLegacy(uri) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p4) 사소하지만 함수의 선언 순서대로 사용하는 것을 저는 더 선호합니다.
그렇게 작성해야 위에서 아래로 코드를 읽으며 더욱 흐름을 이해하는데 도움이 되는 것 같더라구요
아래 순서대로 작성하는건 어떨까요~
loadBitmap
loadBitmapWithImageDecoder
loadBitmapLegacy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 더 좋은것 같습니다~ 가독성 챙기는 방향으로 수정하겠습니다!
if (BuildConfig.DEBUG) { | ||
Timber.d("Compression completed - Quality: $bestQuality, Size: ${buffer.size()} bytes") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p3) 저희 Timeber 셋팅에서 이미 DEBUG 모드에서만 뜨게 설정해뒀을껄용?
안해뒀으면 여기에서만 설정하는 것이 아니라 app 단위에서 해주시면 감사하겠습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이미 설정 해놨지롱!!ㅎㅎㅎ 그냥 바로 Timber 사용해도 됩니다😊
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
몰랐어잉~ 수정할게요~
) | ||
|
||
companion object { | ||
private const val ORIENTATION_NORMAL = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p4) 이건 어떨지? Normal이라는게 의미가 모호한거같아여
private const val ORIENTATION_NORMAL = 0 | |
private const val ORIENTATION_DEFAULT = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
음~ 확실히 Default가 더 친숙하네요! 수정하겠습니당
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM~ 알고리즘까지 멋져요,,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM🚀🚀
와 진짜 신기하다.. 간만에 진짜 어려운 코드 열심히 읽었네요ㅎㅎㅎ 이걸 구현한 한민재 진짜 최고다!!
if (BuildConfig.DEBUG) { | ||
Timber.d("Compression completed - Quality: $bestQuality, Size: ${buffer.size()} bytes") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이미 설정 해놨지롱!!ㅎㅎㅎ 그냥 바로 Timber 사용해도 됩니다😊
postService.registerPost( | ||
data = deferredRequestBody.await(), | ||
photos = deferredPhotoParts.awaitAll() | ||
).data | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P4: 오호 이제 발견했는데 repository에서 datasource를 사용하지 않고 바로 service를 사용하시네요!! 혹시 이렇게 구현하신 이유가 있으신지 궁금해요!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RegisterImpl에서 DataSource를 사용하지 않고 PostService를 직접 사용한 이유는!
RegisterRepository만 이미지 등록이라는 특수한 케이스를 다룹니다. 다른 Repository들은 단순 API 호출이지만, 여기선 멀티파트 요청과 이미지 처리가 필요해서 DataSource를 추가하면 처리를 위한 중간 계층이 불필요하게 생긴다고 생각해요.
DataSource를 추가하면 멀티파트 요청 구성, 이미지 처리 로직 등을 위한 추가 인터페이스와 구현이 필요한 반면
Service를 직접 사용하는 게 이 경우엔 더 단순하고 명확한 구조가 된다고 생각합니다 :)
Related issue 🛠
Work Description ✏️
=== 🚀 성능 개선율 분석 ===
📊 압축률: 81.7% → 94.0% (+15.1% 향상)
⚡ 처리 시간: 975ms → 314ms (67.7% 감소)
💾 메모리 사용량: 1.44MB → 0.36MB (74.9% 감소)
Screenshot 📸
To Reviewers 📢
메이커스 과제 이슈로 docs제작은 나중에 하겠습니다!
간단하게 변경점을 말씀드리자면..
ContentUriRequestBody 개선 버전은 기존 대비 다음과 같이 개선됐습니다.
Dispatchers.IO
를 활용해 이미지 처리 작업을 백그라운드로 이동, 메인 스레드 블로킹 및 ANR 위험 감소.ImageMetadata
클래스 도입으로 산재된 데이터를 모듈화해 가독성 및 유지보수성 향상.