vitrivr / vitrivr-engine

vitrivr's next-generation retrieval engine. It is capable of extracting and retrieving a wider range of multimedia objects such as audio, video, images or 3d models.
https://vitrivr.org
MIT License
6 stars 3 forks source link

VideoDecoder to AverageColor Pipeline result in IllegalArgumentException #107

Closed faberf closed 2 months ago

faberf commented 2 months ago

https://github.com/user-attachments/assets/bcbd82fb-197f-40ff-975d-271f1b905471

For the file above, the following pipeline gives an IllegalArgumentException resulting from an RGB value that is negative.

        val schema = Schema("test", BlackholeConnection("test", BlackholeConnectionProvider()))

        schema.addResolver("test", DiskResolver().newResolver(schema, mapOf()))

        val contextConfig = IngestionContextConfig("CachedContentFactory", "test", global = emptyMap(), local = mapOf(
            "enumerator" to mapOf("path" to ".")
        ))

        contextConfig.schema = schema

        val context = IndexContextFactory.newContext(contextConfig)

        val fileSystemEnumerator = FileSystemEnumerator().newEnumerator("enumerator", context, listOf(MediaType.VIDEO))

        val decoder = BroadcastOperator(VideoDecoder().newDecoder("decoder", input = fileSystemEnumerator, context = context))

        val averageColorAll = AverageColor().newExtractor("averageColorAll", input = decoder, context = context)

        val out = averageColorAll.toFlow(this).toList()
16:41:46.476 [Test worker @kotlinx.coroutines.test runner#2] ERROR org.vitrivr.engine.core.features.AbstractExtractor - Error during extraction of Ingested(id=49b0a86e-ff52-438d-9e69-27c4ecee299a, type=SEGMENT, transient=false)
java.lang.IllegalArgumentException: RGBFloatColorContainer components must be between 0.0 and 1.0.
    at org.vitrivr.engine.core.model.color.RGBColorContainer.constructor-impl(RGBColorContainer.kt:22) ~[vitrivr-engine-core-0.0.1-SNAPSHOT.jar:?]
    at org.vitrivr.engine.core.model.color.RGBColorContainer.constructor-impl(RGBColorContainer.kt:30) ~[vitrivr-engine-core-0.0.1-SNAPSHOT.jar:?]
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.vitrivr.engine.core.model.color.RGBColorContainer.constructor-impl(RGBColorContainer.kt:27) ~[vitrivr-engine-core-0.0.1-SNAPSHOT.jar:?]
    at org.vitrivr.engine.core.model.color.RGBColorContainer.constructor-impl$default(RGBColorContainer.kt:27) ~[vitrivr-engine-core-0.0.1-SNAPSHOT.jar:?]
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
    at org.vitrivr.engine.core.model.color.RGBColorContainer.constructor-impl(RGBColorContainer.kt:26) ~[vitrivr-engine-core-0.0.1-SNAPSHOT.jar:?]
    at org.vitrivr.engine.core.features.averagecolor.AverageColor.analyse(AverageColor.kt:125) ~[vitrivr-engine-core-0.0.1-SNAPSHOT.jar:?]
    at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
    at org.vitrivr.engine.core.features.averagecolor.AverageColorExtractor.extract(AverageColorExtractor.kt:45) ~[vitrivr-engine-core-0.0.1-SNAPSHOT.jar:?]
    at org.vitrivr.engine.core.features.AbstractExtractor$toFlow$1.invokeSuspend(AbstractExtractor.kt:58) ~[vitrivr-engine-core-0.0.1-SNAPSHOT.jar:?]
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
    at org.vitrivr.engine.core.features.AbstractExtractor$toFlow$1.invoke(AbstractExtractor.kt) ~[vitrivr-engine-core-0.0.1-SNAPSHOT.jar:?]
    at org.vitrivr.engine.core.features.AbstractExtractor$toFlow$1.invoke(AbstractExtractor.kt) ~[vitrivr-engine-core-0.0.1-SNAPSHOT.jar:?]
faberf commented 2 months ago

Here are some values that end up as part of the ImageContent (as an RGB array)

image

lucaro commented 2 months ago

What tells you that any of the components of the container are negative rather than too large?

faberf commented 2 months ago

I don't exactly know what you are referring to, but I am working on a fix now.

ppanopticon commented 2 months ago

I think @lucaro is right: I'd presume the problem to be AverageColor.analyse().

We use an IntArray for the sum. This can lead to an overflow of Int's max range (and thus negative values).

faberf commented 2 months ago

I think the problem is in the constructor for RGBColorContainer. I will make a PR now.

lucaro commented 2 months ago

The negative values you are showing in your screenshot are not necessarily related to the problem you are observing, as they are concatenated byte values. If the most significant byte is >=127, the most significant bit will be high, and hence the whole int will be negative.

If there is indeed an overflow problem in averaging the values together, switching to a DoubelArray should be more than enough to prevent this, at the cost of using twice as much memory. Since all that stuff is using a lot of memory already, I don't think this will make a meaningful difference

faberf commented 2 months ago

109 Why is this not a simple fix?

faberf commented 2 months ago

I think I disagree. The problem is not the int that is aggregated. The problem is in the constructor of the RGBColorContainer when an Integer is being turned into four bytes. These bytes should be interpreted as values between 0 and 255 not as values between -127 and 127. What am I missing?

ppanopticon commented 2 months ago

Yes my bad - I was thinking about the main constructor.

What you are proposing makes sense. However, are you sure the culprit isn't the part that converts single Int to component-wise Byte representation?

lucaro commented 2 months ago

The problem is that the .toFloat() on the byte value, you would need to do something like (byte & 0xff).toInt().toFloat(). Using the UByte is essentially that but with some syntactic sugar to make it more readable. I'd say since Kotlin already has unsigned types, we might as well use them, so the pull request is fine by me...

ppanopticon commented 2 months ago

Merged! Thanks @faberf

lucaro commented 2 months ago

So I guess this can be closed now?

ppanopticon commented 2 months ago

Yes