VideoFrameDecoder creates large .tmp which are not cleaned up if the app is killed

[d4rken]

Describe the bug
I think what happens is that VideoFrameDecoder creates some large .tmp files to get a thumbnail. This creation can take time, e.g. for a 4GB video from my phone, it may take a few seconds to get that thumbnail. If the app is killed during that time, then coil looses track of these .tmp files.

This can happen if by the user not waiting for thumbnails to finish loading and swipes the app away from the "recents" screen.

Disabling disk cache (diskCachePolicy(CachePolicy.DISABLED)) does not affect this.

Do I need to check for orphaned tmp files and delete them on app launch?

Is there a way to configure the file name or path for these?

To Reproduce

• Download SD Maid: https://github.com/d4rken-org/sdmaid-se
• Complete setup for file access
• Go to storage analyzer
• Browse some folder with large files (Storage > Main > User Files > /storage/emulated/0/DCIM )
• While the thumbnail for a video file is being decoded, kill the app, e.g. close it by swiping away from recents

Logs/Screenshots

Version
2.7.0 reproduced it on a Pixel 6 running Android 15

[d4rken]

Actual file seems to be created by this call:

coil/coil-video/src/main/java/coil/decode/VideoFrameDecoder.kt

Line 204 in ce14e14

setDataSource(source.file().toFile().path)

Specifically: source.file()

Which calls this:

coil/coil-base/src/main/java/coil/decode/ImageSource.kt

Lines 274 to 278 in ce14e14

	// Copy the source to a temp file.
	val tempFile = createTempFile()
	fileSystem.write(tempFile) {
	writeAll(source!!)
	}

Path comes from here:

coil/coil-base/src/main/java/coil/decode/ImageSource.kt

Lines 298 to 304 in ce14e14

	private fun createTempFile(): Path {
	val cacheDirectory = cacheDirectoryFactory!!.invoke()
	check(cacheDirectory.isDirectory) { "cacheDirectory must be a directory." }
	// Replace JVM call with https://github.com/square/okio/issues/1090 once it's available.
	return File.createTempFile("tmp", null, cacheDirectory).toOkioPath()
	}

Okay so regarding this:

Is there a way to configure the file name or path for these?

I can pass my own path via ImageSource constructor.

[d4rken]

I'm wondering if calling File.deleteOnExit() would prevent these tmp files from being orphaned if the app dies. Probably depends on whether the "on-exit" triggers if the app process dies for some reason. 🤔

[colinrtwhite]

Hmm I thought createTempFile would be deleted on exit, but sounds like we need something else. Adding deleteOnExit sounds good to me!

Unfortunately MediaMetadataRetriever needs a seekable data source, which basically means a file since it's generally not possible to keep the complete video file in memory. How are you loading these files? Ideally we fall into one of these earlier cases, which lets us avoid creating a temp file.

[d4rken]

Via Java NIO API, DELETE_ON_CLOSE might also be an option:

https://stackoverflow.com/questions/28752006/alternative-to-file-deleteonexit-in-java-nio

I currently can't test it, because I can't get coil to build on my local machine.

How are you loading these files? Ideally we fall into one of these earlier cases, which lets us avoid creating a temp file.

I've got a custom IO API that handles access via different permissions, including root and adb, that exposes a Source to Coil and a custom fetcher:

https://github.com/d4rken-org/sdmaid-se/blob/db0be9ec1262d2571d3fa933d5159507f74b395a/app/src/main/java/eu/darken/sdmse/common/coil/BitmapFetcher.kt#L35-L44

[d4rken]

deleteOnExit does not work with Android, not triggered when app is swiped away. ☹️

[d4rken]

Could I provide Coil with an Okio.FileHandle (which is a seekable source), so that creating the tmp file is not necessary?

[d4rken]

I found the experimental API for MediaSourceMetadata from #1790 🔥

I've changed my API to return a FileHandle and wrap that using MediaSourceMetaData, afaict it prevents the temporary file from being created.

@OptIn(ExperimentalCoilApi::class)
internal fun FileHandle.toImageSource(
    options: Options,
): ImageSource {
    val handle = this
    val sourceBuffer = this.source().buffer()
    val mediaDataSource = object : MediaDataSource() {
        override fun readAt(position: Long, buffer: ByteArray, offset: Int, size: Int): Int {
            return handle.read(position, buffer, offset, size)
        }

        override fun getSize(): Long {
            return handle.size()
        }

        override fun close() {
            sourceBuffer.close()
            handle.close()
        }
    }
    return ImageSource(
        source = sourceBuffer,
        context = options.context,
        metadata = MediaSourceMetadata(mediaDataSource),
    )
}

[colinrtwhite]

Nice! For cleaning up temp files, unfortunately I don't think there's a great solution. We already delete the temp file in a finally block, however that isn't run when the app is force closed.

From my research it sounds like temp files won't be cleaned up when the app is closed, however they're likely to be cleared when the device needs space or the device is rebooted. Overall, they shouldn't last forever and will be cleaned up eventually.

[colinrtwhite]

Also as an aside I used some of your code here to fix using VideoFrameDecoder when used with custom FileSystem implementations. Thanks!

[d4rken]

So as I've found a solution for my issue and the "app-death" handling of tmp files is kinda out of scope (coil would have to keep track of the tmp files in some kind of DB), I think we can close this.

What exactly did #2570 fix?

[colinrtwhite]

@d4rken Using custom Okio FileSystems like FakeFileSystem for tests would fail because VideoFrameDecoder was always assuming FileSystem.SYSTEM was used. There are other cases (for instance with Compose Multiplatform Resources) where the non-default FileSystem will be used.