[Sample] [Media] - Implementation of "UltraHDR to HDR Video" #83

madebymozart · 2023-08-16T00:12:33Z

Description

This CL contains sample code for converting a series of UltraHDR images into an HDR video using GPU hardware acceleration. When rendering an UltraHDR image into a 10-bit frame, the gain map is considered.

This implementation is very detailed as there is many moving parts of it, do excessive notes have been added to guide developer through the various steps needed to make this a reality.

How Has This Been Tested?

Test Configuration #1 - Pixel 7 Pro
Test Configuration #2 - Pixel 7

Checklist:

I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation

Dem

ultrahdr_to_hdrvideo_screen_recording.mp4

Resulting File

This file have been taken from device and depending on the device you view it on, you will see it in true 10-bit HDR HLG.
https://github.com/android/platform-samples/assets/5649571/934421b8-6059-4828-8686-b3bec05a26d8

samples/media/ultrahdr/README.md

...media/ultrahdr/src/main/java/com/example/platform/media/ultrahdr/video/UltraHDRToHDRVideo.kt

marcelpinto · 2023-08-16T11:00:33Z

...media/ultrahdr/src/main/java/com/example/platform/media/ultrahdr/video/UltraHDRToHDRVideo.kt

+     * [ImageWriter.setOnImageReleasedListener] callbacks.
+     */
+    private lateinit var imageWriterHandler: Handler
+    private val imageWriterThread: HandlerThread by lazy {


Is it possible to use coroutines instead? This sample mixes coroutines with threads and it's a bit confusing.

Unfortunately, ImageWriter and MediaCodec.Callback need their own handlers to process callback calls on their own thread.

Not sure if or when they will switch to coroutine scopes if ever.

Not familiar with this APIs but I think if you pass null to the handler it will use the thread handler
https://developer.android.com/reference/android/media/ImageWriter#setOnImageReleasedListener(android.media.ImageWriter.OnImageReleasedListener,%20android.os.Handler)

If you then call this method within a coroutine using a Dispatcher.IO shouldn't that work?

In anycase shouldn't you stop the thread on stop/destroy?

You can create a Dispatcher from a Handler using Handler.asCoroutineDispatcher()

Ah going through again the above suggestion won't change anything as you have to pass a Handler to the API.

tunjid · 2023-08-17T12:20:07Z

...media/ultrahdr/src/main/java/com/example/platform/media/ultrahdr/video/UltraHDRToHDRVideo.kt

+    tags = ["UltraHDR"],
+)
+@RequiresApi(Build.VERSION_CODES.UPSIDE_DOWN_CAKE)
+class UltraHDRToHDRVideo : Fragment() {


Please consider moving the concurrency and processing bits into a separate state holder class. As it stands, the UI and its state are really strongly coupled.

tunjid · 2023-08-17T12:27:37Z

...media/ultrahdr/src/main/java/com/example/platform/media/ultrahdr/video/UltraHDRToHDRVideo.kt

+            CoroutineScope(Dispatchers.IO).launch {
+                val threadName = Thread.currentThread().name
+                Log.i(TAG, "Starting Conversion to Video on $threadName")
+                doImageToVideoConversion()


This coroutine is launched on the IO Dispatcher, and subsequently used withContext to also place it on the same dispatcher. The latter alone should be enough.

mmoczkowski

I recommend the following:

Use Compose for UI
Increase separation of concerns by abstracting the video processing to a separate class.

mmoczkowski · 2023-08-17T12:45:25Z

...media/ultrahdr/src/main/java/com/example/platform/media/ultrahdr/video/UltraHDRToHDRVideo.kt

+    tags = ["UltraHDR"],
+)
+@RequiresApi(Build.VERSION_CODES.UPSIDE_DOWN_CAKE)
+class UltraHDRToHDRVideo : Fragment() {


Have you considered using Compose? It is required that all new samples are written in Compose.

mmoczkowski · 2023-08-17T12:47:04Z

...media/ultrahdr/src/main/java/com/example/platform/media/ultrahdr/video/UltraHDRToHDRVideo.kt

+     *
+     * Using [AtomicBoolean] since it can be accessed by multiple threads.
+     */
+    private var isFrameBufferProcessed = AtomicBoolean(true)


The video processing code is mixed with presentation code. I highly recommend to abstract video processing to a separate class.

mmoczkowski · 2023-08-17T12:53:20Z

...media/ultrahdr/src/main/java/com/example/platform/media/ultrahdr/video/UltraHDRToHDRVideo.kt

+     * The [MediaCodec.Callback], used to asynchronously process frame produced by the
+     * [ImageWriter].
+     */
+    private fun setUpMediaCodecCallback() = object : MediaCodec.Callback() {


It might be beneficial to move this class to its own file. By doing so, we can inject it as a dependency into the relevant video processor. This would align better with the Single Responsibility Principle, ensuring each class has a clear, distinct function. This change would likely enhance code readability and maintainability. What do you think?

mmoczkowski · 2023-08-17T12:54:19Z

...media/ultrahdr/src/main/java/com/example/platform/media/ultrahdr/video/UltraHDRToHDRVideo.kt

+        // Create path to cache directory. Delete any previously generated videos.
+        val dir = File(folderPath)
+        if (dir.exists() && dir.isDirectory) dir.listFiles()?.let {
+            for (file in it) file.delete()


Would it be possible to use File::deleteRecursively here?

mmoczkowski · 2023-08-17T12:55:20Z

...media/ultrahdr/src/main/java/com/example/platform/media/ultrahdr/video/UltraHDRToHDRVideo.kt

+
+        bitmaps.forEach { bitmap ->
+            var frameCount = 1
+            while (frameCount <= FORMAT_FRAME_RATE) {


Could repeat(FORMAT_FRAME_RATE) be sufficient?

mmoczkowski · 2023-08-17T12:58:12Z

...media/ultrahdr/src/main/java/com/example/platform/media/ultrahdr/video/UltraHDRToHDRVideo.kt

+        /**
+         * Folder path name where .mp4 files will be encoded to
+         */
+        private val FOLDER_PATH_NAME = UltraHDRToHDRVideo::class.java.simpleName


I'd be cautious here. If the code obfuscation is enabled the name of the class will be changed to something random. I think it's best to simply hardcode a string.

Talked to PR author

madebymozart · 2023-08-21T21:20:53Z

Will add a second PR what uses the same logic that will split video processing from sample @mmoczkowski @marcelpinto :)

* Testing Implementation for UltraHDR to HDR Video * Complete HW aceel implementation * Add temp muxer to write file * working implementation of output * Clean up implementation of UltraHDR to HDR Video * Slight fixes to implementation, better coroutines * pr nits

madebymozart and others added 6 commits August 7, 2023 19:58

Testing Implementation for UltraHDR to HDR Video

d4daae5

Complete HW aceel implementation

181b3c4

Add temp muxer to write file

ceb2da5

working implementation of output

cd2aa2c

Clean up implementation of UltraHDR to HDR Video

3539731

Slight fixes to implementation, better coroutines

9b8d9fa

madebymozart added enhancement New feature or request new sample This denotes the creation of a new sample in the platform samples app labels Aug 16, 2023

madebymozart requested a review from marcelpinto August 16, 2023 00:12

madebymozart self-assigned this Aug 16, 2023

marcelpinto suggested changes Aug 16, 2023

View reviewed changes

pr nits

9d35528

madebymozart requested a review from marcelpinto August 16, 2023 19:45

marcelpinto approved these changes Aug 17, 2023

View reviewed changes

tunjid reviewed Aug 17, 2023

View reviewed changes

mmoczkowski self-requested a review August 17, 2023 12:37

mmoczkowski previously requested changes Aug 17, 2023

View reviewed changes

This comment was marked as duplicate.

Sign in to view

madebymozart merged commit 1b30ef0 into main Aug 21, 2023
4 checks passed

madebymozart deleted the ultrahdr-to-video branch August 21, 2023 21:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Sample] [Media] - Implementation of "UltraHDR to HDR Video" #83

[Sample] [Media] - Implementation of "UltraHDR to HDR Video" #83

madebymozart commented Aug 16, 2023

marcelpinto Aug 16, 2023

madebymozart Aug 16, 2023 •

edited

marcelpinto Aug 17, 2023

tunjid Aug 17, 2023

tunjid Aug 17, 2023

tunjid Aug 17, 2023

tunjid Aug 17, 2023

mmoczkowski left a comment

mmoczkowski Aug 17, 2023

mmoczkowski Aug 17, 2023

mmoczkowski Aug 17, 2023

mmoczkowski Aug 17, 2023

mmoczkowski Aug 17, 2023

mmoczkowski Aug 17, 2023

This comment was marked as duplicate.

madebymozart commented Aug 21, 2023

[Sample] [Media] - Implementation of "UltraHDR to HDR Video" #83

[Sample] [Media] - Implementation of "UltraHDR to HDR Video" #83

Conversation

madebymozart commented Aug 16, 2023

Description

How Has This Been Tested?

Checklist:

Dem

Resulting File

Choose a reason for hiding this comment

madebymozart Aug 16, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmoczkowski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as duplicate.

madebymozart commented Aug 21, 2023

madebymozart Aug 16, 2023 •

edited