Fix DASH thumbnails cropping the incorrect tile for non-square images #1300

hakonschia · 2024-04-20T09:27:15Z

The calculation for finding the column a tileIndex corresponds to is currently tileIndex % inputFormat.tileCountVertical. This should instead be tileIndex % inputFormat.tileCountHorizontal (see this SO answer) to work for non-square images. It works for square images since tileCountVertical == tileCountHorizontal, so the same values are produced.

Using tileCountVertical for non-square images results in either:

When tileCountVertical is equal to 1 (sprites with one row of images) it always crops out the first tile
When tileCountVertical is larger than 1 it will attempt to crop out a bitmap outside the bounds of the larger bitmap, resulting in Bitmap.createBitmap() throwing an exception.

Exception thrown

Playback error
  androidx.media3.exoplayer.ExoPlaybackException: Unexpected runtime error
   at androidx.media3.exoplayer.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:703)
   at android.os.Handler.dispatchMessage(Handler.java:103)
   at android.os.Looper.loopOnce(Looper.java:232)
   at android.os.Looper.loop(Looper.java:317)
   at android.os.HandlerThread.run(HandlerThread.java:85)
 Caused by: java.lang.IllegalArgumentException: x + width must be <= bitmap.width()
   at android.graphics.Bitmap.createBitmap(Bitmap.java:910)
   at android.graphics.Bitmap.createBitmap(Bitmap.java:869)
   at androidx.media3.exoplayer.image.ImageRenderer.cropTileFromImageGrid(ImageRenderer.java:575)
   at androidx.media3.exoplayer.image.ImageRenderer.drainOutput(ImageRenderer.java:343)
   at androidx.media3.exoplayer.image.ImageRenderer.render(ImageRenderer.java:182)
   at androidx.media3.exoplayer.ExoPlayerImplInternal.doSomeWork(ExoPlayerImplInternal.java:1114)
   at androidx.media3.exoplayer.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:546)
   at android.os.Handler.dispatchMessage(Handler.java:103) 
   at android.os.Looper.loopOnce(Looper.java:232) 
   at android.os.Looper.loop(Looper.java:317) 
   at android.os.HandlerThread.run(HandlerThread.java:85)

I have updated ImageRenderTest to use a 2x3 image instead of 2x2. Reverting my change will make the updated tests fail, as expected, but please verify they still work as intended.

I have created a separate branch that displays thumbnails in the demo app that can be used for testing. Two samples are added, one for the one row case and one for non-squares with more than one row. The videos below are taken from this branch.

Before

Screen_recording_20240420_085532.mp4

After

Screen_recording_20240420_085434.mp4

microkatz · 2024-04-25T12:35:03Z

@hakonschia

Thank you for identifying the issue and providing a pull request to remedy it. Your solution makes sense to me. All that I ask is if you can just create an individual unit test and sample with the six frames rather than altering the current tests. That would be great!

hakonschia · 2024-04-26T16:46:32Z

@microkatz I've created a new test now instead, but I'm not really if what I did with overridenBitmap is the best way of using a different bitmap in this test

Let me know if you'd prefer to have these two commits squashed to avoid ImageRenderTest having changes in the first commit that were just reverted again in the second commit

microkatz · 2024-04-30T12:49:36Z

@hakonschia

Ah. I see the current issue of how its setup in setup with the fakeDecoderFactory instance providing either fakeDecodedBitmap1 or fakeDecodedBitmap2. I'm not sure I like the overridenBitmap. Instead of having the overridenBitmap, how about you just reassign and reinitalize the renderer in your singular unit test?

hakonschia · 2024-04-30T14:44:09Z

@microkatz sure, that seems like a better solution to me, I've pushed that change now. I just copied queuingImageOutput, but it might make more sense for this to be a moved to a class variable at this point instead of duplicating it

microkatz · 2024-05-01T11:22:18Z

I'm going to send this for internal review now. You may see some more commits being added as I make changes in response to review feedback. Please refrain from pushing any more substantive changes as it will complicate the internal review - thanks!

hakonschia · 2024-05-01T12:04:57Z

RELEASENOTES.md

+    *   Add support for non-square DASH thumbnail grids
+        ([#1255](https://github.com/androidx/media/pull/1300)).


@microkatz The PR number for the link text doesn't match the actual PR (and issue 1255 is unrelated to this PR)

… for cropping the correct tile from outputBitmap To find the column of an index in a matrix the formula "column = index % width" should be used, not "column = index % height" If inputFormat.tileCountVertical was equal to 1 then it would not throw an error, but instead result in the first tile of the bitmap always being returned. If inputFormat.tileCountVertical was larger than 1 then Bitmap.createBitmap() would throw an error as it would attempt to go outside the bounds of outputBitmap ImageRenderTest has been updated to test for 2x3 images so that tileCountVertical != tileCountHorizontal. These tests passed previously because they were equal, so using tileCountVertical produced the same results as tileCountHorizontal

…test for non square images instead

…esToOutput to allow it to use a separate bitmap from the other tests

tianyif assigned microkatz Apr 22, 2024

microkatz force-pushed the fix-dash-thumbnails-cropping branch from 1d242bd to ddbb0ef Compare May 1, 2024 11:21

microkatz added the should merge label May 1, 2024

hakonschia commented May 1, 2024

View reviewed changes

microkatz force-pushed the fix-dash-thumbnails-cropping branch from e5a30d4 to 420c58b Compare May 1, 2024 15:13

hakonschia and others added 6 commits May 1, 2024 16:26

Revert changes in ImageRenderTest from 3a030da and create a new unit …

e27f527

…test for non square images instead

Create new ImageRenderer in render_tiledImageNonSquare_rendersAllImag…

cf41ba3

…esToOutput to allow it to use a separate bitmap from the other tests

Format with google-java-format

6d62d34

Add release note for DASH non-square thumbnail grid fix

690bb1a

Updated non-Square thumbnail unit test to contain non-Square thumbnails

acc5a3b

microkatz force-pushed the fix-dash-thumbnails-cropping branch from 420c58b to acc5a3b Compare May 1, 2024 16:27

copybara-service bot merged commit d385072 into androidx:main May 1, 2024
1 check passed

icbaker mentioned this pull request May 30, 2024

Playback Error on Image Track (IllegalArgumentException: x + width must be <= bitmap.width()) #1411

Closed

1 task

androidx locked and limited conversation to collaborators Jul 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix DASH thumbnails cropping the incorrect tile for non-square images #1300

Fix DASH thumbnails cropping the incorrect tile for non-square images #1300

hakonschia commented Apr 20, 2024

microkatz commented Apr 25, 2024 •

edited

Loading

hakonschia commented Apr 26, 2024

microkatz commented Apr 30, 2024 •

edited

Loading

hakonschia commented Apr 30, 2024

microkatz commented May 1, 2024

hakonschia May 1, 2024 •

edited

Loading

		* Add support for non-square DASH thumbnail grids
		([#1255](https://github.com/androidx/media/pull/1300)).

Fix DASH thumbnails cropping the incorrect tile for non-square images #1300

Fix DASH thumbnails cropping the incorrect tile for non-square images #1300

Conversation

hakonschia commented Apr 20, 2024

microkatz commented Apr 25, 2024 • edited Loading

hakonschia commented Apr 26, 2024

microkatz commented Apr 30, 2024 • edited Loading

hakonschia commented Apr 30, 2024

microkatz commented May 1, 2024

hakonschia May 1, 2024 • edited Loading

Choose a reason for hiding this comment

microkatz commented Apr 25, 2024 •

edited

Loading

microkatz commented Apr 30, 2024 •

edited

Loading

hakonschia May 1, 2024 •

edited

Loading