coil-kt / coil

Image loading for Android and Compose Multiplatform.
https://coil-kt.github.io/coil/
Apache License 2.0
10.86k stars 668 forks source link

[coil-video] Fails to load preview for video #413

Closed gabin8 closed 4 years ago

gabin8 commented 4 years ago

Describe the bug When I'm trying to load preview for video that I picked from my device I'm getting an exception:

2020-05-22 23:40:14.907 27387-27387/com.example.android I/System.out: load uri: content://com.android.providers.media.documents/document/video%3A158545
2020-05-22 23:40:14.956 27387-27462/com.example.android D/skia: --- Failed to create image decoder with message 'unimplemented'
2020-05-22 23:40:14.956 27387-27462/com.example.android D/skia: --- Failed to create image decoder with message 'unimplemented'
2020-05-22 23:40:14.965 27387-27387/com.example.android I/RealImageLoader: 🚨 Failed - content://com.android.providers.media.documents/document/video%3A158545 - java.lang.IllegalStateException: BitmapFactory returned a null Bitmap. Often this means BitmapFactory could not decode the image data read from the input source (e.g. network or disk) as it's not encoded as a valid image format.
2020-05-22 23:40:14.967 27387-27387/com.example.android E/RealImageLoader: java.lang.IllegalStateException: BitmapFactory returned a null Bitmap. Often this means BitmapFactory could not decode the image data read from the input source (e.g. network or disk) as it's not encoded as a valid image format.
        at coil.decode.BitmapFactoryDecoder.decode(BitmapFactoryDecoder.kt:177)
        at coil.RealImageLoader$loadData$2.invokeSuspend(RealImageLoader.kt:349)
        at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
        at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:56)
        at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:571)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:738)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:678)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:665)
2020-05-22 23:40:17.922 27387-27523/com.example.android V/FA: Inactivity, disconnecting from the service

Expected behavior Would like to see thumbnail

To Reproduce The code is pretty standard


val imageLoader = ImageLoader.Builder(requireContext())
            .componentRegistry {
                add(VideoFrameFileFetcher(requireContext()))
                add(VideoFrameUriFetcher(requireContext()))
            }
            .build()

val request = LoadRequestBuilder(requireContext())
                .data(uri)
                .videoFrameMillis(2000)
                .target(myImageView)
                .build()
            imageLoader.execute(request)

Version coil-version 0.11.0 Android 9 coil-video is added

colinrtwhite commented 4 years ago

VideoFrameFetcher relies on the file's extension to automatically handle video files. Since your URI doesn't have an extension, you'll have to set the fetcher explicitly for the request:

val request = LoadRequestBuilder(requireContext())
                .data(uri)
                .videoFrameMillis(2000)
                .target(myImageView)
                .fetcher(VideoFrameUriFetcher(requireContext())
                .build()
imageLoader.execute(request)
gabin8 commented 4 years ago

VideoFrameFetcher relies on the file's extension to automatically handle video files. Since your URI doesn't have an extension, you'll have to set the fetcher explicitly for the request:

val request = LoadRequestBuilder(requireContext())
                .data(uri)
                .videoFrameMillis(2000)
                .target(myImageView)
                .fetcher(VideoFrameUriFetcher(requireContext())
                .build()
imageLoader.execute(request)

@colinrtwhite Thanks, it works. I think it should be specified in docs somehow. It wasn't clear from the exception. Do you have any plans to support thumbnails from remote sources, like video url's?

colinrtwhite commented 4 years ago

Good point - I'll add a section to the docs about it.

As for video frames from a url, I'd like to, but I don't think it's possible. MediaMetadataRetriever requires the ability to read backwards and forwards in a source. We can handle short backwards reads by buffering them into memory, however even small video files (10MB) would consume a lot of memory to decode. Large video files would throw an out of memory error easily. By restricting it to file/uris only we avoid having to buffer into memory.

AndroidDeveloperLB commented 4 years ago

Can anyone please share a POC of this? I can't find LoadRequestBuilder, and all I want is to go over frames of a video (short duration between frames, so that if I wished, I could even play them one after another).

colinrtwhite commented 4 years ago

@AndroidDeveloperLB LoadRequestBuilder was replaced with ImageRequest.Builder.

AndroidDeveloperLB commented 4 years ago

@colinrtwhite Seems not to work as I expect. I chose to show the image of the first second of some video file, and when I play the video file, it has content, but when I debug here, I get a black bitmap.

See attached project and screenshot:

test.zip

colinrtwhite commented 4 years ago

@AndroidDeveloperLB Please open a new bug report. Also enabling logs will print out the exception stack trace for the request.

OhhhThatVarun commented 3 years ago

Here is a complete function if anybody is looking.

fun ImageView.setThumbnail(uri: Uri, frameMillis: Long = 2000) {

    val imageLoader = ImageLoader.Builder(context)
        .componentRegistry {
            add(VideoFrameFileFetcher(context))
            add(VideoFrameUriFetcher(context))
        }.build()

    val request = ImageRequest.Builder(context)
        .data(uri)
        .videoFrameMillis(frameMillis)
        .target(this)
        .fetcher(VideoFrameUriFetcher(context))
        .build()

    findViewTreeLifecycleOwner()?.lifecycleScope?.launch(Dispatchers.Main) {
        imageLoader.execute(request)
    }
}
AndroidDeveloperLB commented 3 years ago

@OhhhThatVarun What's the purpose of "findViewTreeLifecycleOwner" in your code? Can't you just execute it, and set an annotation to run on UI thread? Also, which dependency you used to have it? For some reason I can't see.

OhhhThatVarun commented 3 years ago

@AndroidDeveloperLB

Q) What's the purpose of "findViewTreeLifecycleOwner" in your code? A) We need a Coroutine Scope to run the execute function. We are using findViewTreeLifecycleOwner to find the LifecycleOwner of this ImageView. So, instead of running it on GlobalScope which is a bad idea, we are running it in a CoroutineScope which is tied to this LifecycleOwner's (fragment or activity) Lifecycle.

Q) Can't you just execute it, and set an annotation to run on the UI thread? A) No, as execute is a suspend function we cannot run it outside a coroutine.

Q) which dependency you used to have it? A) I'm using androidx.fragment:fragment-ktx:1.3.5.

Here is the tweet by Ian Lake.

AndroidDeveloperLB commented 3 years ago

@OhhhThatVarun I'm confused. Doesn't imageLoader.execute(request) run on the UI thread? It seems enqueue would do the same if you don't use findViewTreeLifecycleOwner... . Or maybe you mean that execute has inside some handling in case the Activity/Fragment is now dead? Which function can be used in case I don't use Coroutine?

I tried the code you wrote, but it doesn't seem to work. See attached.

CoilTest.zip xTxA4ldLzF

OhhhThatVarun commented 3 years ago

@AndroidDeveloperLB

Q) Doesn't imageLoader.execute(request) run on the UI thread? A) I think you got it wrong. execute is a suspend function that can be run on any thread. It doesn't automatically run on the UI Thread we make it run on UI Thread. As we are dealing with an ImageView that can be accessed only on the UI Thread we are running it with Dispatchers.Main(A coroutine dispatcher that is confined to the Main thread operating with UI objects.).

If you read the documentation of the execute function it actually states that Execute the [request] in the current coroutine scope. We are using findViewTreeLifecycleOwner just to get the CoroutineScope so we can run the suspend function. As you know launch returns a Job, and we might want to cancel our job if the ImageView gets detach while loading the image (user moves to some other screen) into ImageView so by using the CoroutineScope which is tied to ImageView's LifecycleOwner's we don't have to worry about at all. As it is going to get canceled automatically.

class MainActivity : AppCompatActivity() {

    private val videoPicker = registerForActivityResult(ActivityResultContracts.GetContent()) { uri ->
        Log.e("AppLog", uri.toString()) // Prints out Content URI
        findViewById<ImageView>(R.id.imageView).setThumbnail(uri,3000L)
    }

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContentView(R.layout.activity_main)
        videoPicker.launch("video/*")

        // This also won't load your image into your ImageView
        findViewById<ImageView>(R.id.imageView).load("/storage/emulated/0/your_image.png")
    }
}

Try this. And I think you would need a content URI a simple File URI won't work.

AndroidDeveloperLB commented 3 years ago

@OhhhThatVarun Oh ok. Sorry for that. Seems to work now. If I didn't use Kotlin coroutines, would I have used "enqueue " instead?

OhhhThatVarun commented 3 years ago

@AndroidDeveloperLB I don't think you can run it without coroutines. As I already said it is a suspend function it needs a coroutine scope. If you don't want to use findViewTreeLifecycleOwner() then I would say run it with a GlobalScope. So, it is going to look like this.

GlobalScope.launch(Dispatchers.Main) {
    imageLoader.execute(request)
}

This is not recommended though.

AndroidDeveloperLB commented 3 years ago

@OhhhThatVarun But "enqueue" isn't marked with "suspend".

OhhhThatVarun commented 3 years ago

@AndroidDeveloperLB You can use it obviously but it is essentially the same thing you are just substituting one thing for another. It is going to return a Disposable instead of a Job which will be returned by launch. In both cases, you would have to call dispose() or cancel() by yourself.

AndroidDeveloperLB commented 3 years ago

@OhhhThatVarun OK thanks.

yehia2030 commented 2 years ago

how to make such a concept in compose ? remember function of painter do any one have the answer?

gaohomway commented 2 years ago

@yehia2030 About Compose, there is no documentation, or the documentation has no context, I think I understand it all, but I don't understand it

anonym24 commented 10 months ago

how to make such a concept in compose ? remember function of painter do any one have the answer?

mb it's better to temporary use Glide beta compose instead - https://github.com/coil-kt/coil/discussions/2055#discussioncomment-8182319

It works fine and you don't need additional setups.