Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .idea/gradle.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.tejpratapsingh.motionlib.filamentrenderer

import android.content.Context

Check failure on line 3 in modules/3d-filament-renderer/src/main/java/com/tejpratapsingh/motionlib/filamentrenderer/Filament3dView.kt

View workflow job for this annotation

GitHub Actions / ktlint

Imports must be ordered in lexicographic order without any empty lines in-between with "java", "javax", "kotlin" and aliases in the end
import android.graphics.Bitmap
import android.graphics.Matrix
import android.graphics.SurfaceTexture
Expand Down Expand Up @@ -31,7 +31,7 @@
private val modelAssetPath: String,
override val startFrame: Int,
override val endFrame: Int,
override val loop: Pair<Int, Int> = Pair(0, 0),
override var loop: Pair<Int, Int> = Pair(0, 0),
effects: List<MotionEffect> = emptyList(),
) : FrameLayout(context),
MotionView {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class MotionOpenGlView(
modelAssetPath: String,
override val startFrame: Int,
override val endFrame: Int,
override val loop: Pair<Int, Int> = Pair(0, 0),
override var loop: Pair<Int, Int> = Pair(0, 0),
effects: List<MotionEffect> = emptyList(),
) : FrameLayout(context),
MotionView {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.tejpratapsingh.motionlib.core.extensions.downloadFile
import com.tejpratapsingh.motionlib.core.motion.BaseContourMotionView
import com.tejpratapsingh.motionlib.core.motion.MotionVideoProducer
import com.tejpratapsingh.motionlib.core.motion.transitions.CrossFadeTransition
import com.tejpratapsingh.motionlib.core.setCurrentConfig
import com.tejpratapsingh.motionlib.ui.custom.background.GradientView
import com.tejpratapsingh.motionlib.ui.custom.background.Orientation
Expand Down Expand Up @@ -107,7 +108,7 @@
// motionConfig = motionConfig
// )

val motionView2: MotionView =
val motionView2 =
GradientView(
context = applicationContext,
startFrame = motionView.endFrame + 1,
Expand All @@ -126,5 +127,8 @@
.with(
context = applicationContext,
motionAudio = motionAudio,
).addMotionViewToSequence(motionView = motionView)
)
.addMotionViewToSequence(motionView = motionView)

Check failure on line 131 in modules/app/src/main/java/com/tejpratapsingh/animator/presentation/SampleMotionVideo.kt

View workflow job for this annotation

GitHub Actions / ktlint

Unexpected newline before '.'
.addTransition(CrossFadeTransition(), duration = 30)
.addMotionViewToSequence(motionView = motionView2)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import kotlin.math.min
/**
* Text size variants for video overlays.
*/
internal enum class MotionTextVariant {
enum class MotionTextVariant {
H1,
H2,
H3,
Expand Down Expand Up @@ -33,6 +33,11 @@ internal data class MotionTextSizes(
* Provides scaled font sizes in pixels relative to the video dimensions.
*/
object MotionTextSizeProvider {
/**
* Base scale for all font sizes. Can be updated to scale all text sizes at once.
*/
var baseTextScale: Float = 1.5f

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Validate baseTextScale bounds.

Line [39] exposes global mutable scale without bounds checks; <= 0 can collapse or invert text sizes throughout the pipeline.

Proposed fix
 object MotionTextSizeProvider {
@@
-    var baseTextScale: Float = 1.5f
+    var baseTextScale: Float = 1.5f
+        set(value) {
+            field = value.coerceAtLeast(0.1f)
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
var baseTextScale: Float = 1.5f
var baseTextScale: Float = 1.5f
set(value) {
field = value.coerceAtLeast(0.1f)
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@modules/core/src/main/java/com/tejpratapsingh/motionlib/core/MotionTextSizeProvider.kt`
at line 39, The public mutable variable baseTextScale on MotionTextSizeProvider
is unbounded and can be set to <= 0; change it to enforce valid bounds by making
baseTextScale use a custom setter (or back it with a private var) that clamps or
rejects invalid values (e.g., ensure value > 0 and optionally cap at a
reasonable max) and either throw IllegalArgumentException for invalid inputs or
clamp to a safe range; update any call sites to set baseTextScale via the setter
to ensure the validation is applied.


/**
* Returns the [MotionTextSizes] for the given [aspectRatio].
*
Expand All @@ -44,26 +49,26 @@ object MotionTextSizeProvider {
}

return MotionTextSizes(
h1 = aspectRatio.scale(96f),
h2 = aspectRatio.scale(72f),
h3 = aspectRatio.scale(48f),
h4 = aspectRatio.scale(36f),
h5 = aspectRatio.scale(24f),
h6 = aspectRatio.scale(20f),
p = aspectRatio.scale(32f),
h1 = aspectRatio.scale(160f * baseTextScale),
h2 = aspectRatio.scale(120f * baseTextScale),
h3 = aspectRatio.scale(80f * baseTextScale),
h4 = aspectRatio.scale(60f * baseTextScale),
h5 = aspectRatio.scale(40f * baseTextScale),
h6 = aspectRatio.scale(32f * baseTextScale),
p = aspectRatio.scale(48f * baseTextScale),
)
}

/**
* Returns the font size in pixels for the given [variant] and [aspectRatio].
*
* For [VideoAspectRatio.Custom], it returns a fallback size of 32.0f.
* For [VideoAspectRatio.Custom], it returns a fallback size of (32.0f * baseTextScale).
*/
internal fun getFontSize(
fun getFontSize(
aspectRatio: VideoAspectRatio,
variant: MotionTextVariant,
): Float {
val sizes = getTextSizes(aspectRatio) ?: return 32f
val sizes = getTextSizes(aspectRatio) ?: return aspectRatio.scale(32f * baseTextScale)

return when (variant) {
MotionTextVariant.H1 -> sizes.h1
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.tejpratapsingh.motionlib.core

import android.view.ViewGroup

/**
* Interface for defining a transition between two [MotionView]s.
* A transition usually overlaps the end of the first view and the start of the second view.
*/
interface MotionTransition {
/**
* Applies the transition between [from] and [to].
*
* @param from The outgoing view.
* @param to The incoming view.
* @param duration The duration of the transition in frames.
*/
fun apply(from: MotionView, to: MotionView, duration: Int)

Check failure on line 17 in modules/core/src/main/java/com/tejpratapsingh/motionlib/core/MotionTransition.kt

View workflow job for this annotation

GitHub Actions / ktlint

Parameter should start on a newline

Check failure on line 17 in modules/core/src/main/java/com/tejpratapsingh/motionlib/core/MotionTransition.kt

View workflow job for this annotation

GitHub Actions / ktlint

Parameter should start on a newline

Check failure on line 17 in modules/core/src/main/java/com/tejpratapsingh/motionlib/core/MotionTransition.kt

View workflow job for this annotation

GitHub Actions / ktlint

Newline expected after opening parenthesis

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Wrap apply parameters per ktlint function-signature rule.

Line 17 needs multiline parameter formatting; current signature fails style checks.

Proposed fix
-    fun apply(from: MotionView, to: MotionView, duration: Int)
+    fun apply(
+        from: MotionView,
+        to: MotionView,
+        duration: Int,
+    )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fun apply(from: MotionView, to: MotionView, duration: Int)
fun apply(
from: MotionView,
to: MotionView,
duration: Int,
)
🧰 Tools
🪛 GitHub Check: ktlint

[failure] 17-17:
Parameter should start on a newline


[failure] 17-17:
Parameter should start on a newline


[failure] 17-17:
Newline expected after opening parenthesis

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@modules/core/src/main/java/com/tejpratapsingh/motionlib/core/MotionTransition.kt`
at line 17, The function signature for MotionTransition.apply should be
reformatted to satisfy ktlint's function-signature rule: break the parameters
onto multiple lines. Locate the apply method declaration in the
MotionTransition.kt file (function name: apply, class/interface:
MotionTransition) and change the single-line signature "fun apply(from:
MotionView, to: MotionView, duration: Int)" to a multiline parameter list so
each parameter (from: MotionView, to: MotionView, duration: Int) is on its own
line and the closing parenthesis and return type (if any) follow ktlint
conventions.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package com.tejpratapsingh.motionlib.core

import org.junit.Assert.assertEquals
import org.junit.Before
import org.junit.Test

class MotionTextSizeProviderTest {

@Before
fun setup() {
// Reset baseTextScale before each test
MotionTextSizeProvider.baseTextScale = 1.0f
}

@Test
fun testDefaultFontSizes() {
val aspectRatio = VideoAspectRatio.Ratio16x9_1080 // 1920x1080, min is 1080, scale is 1.0

// H1 should be 160f * 1.0 = 160f
assertEquals(160f, MotionTextSizeProvider.getFontSize(aspectRatio, MotionTextVariant.H1), 0.01f)

// P should be 48f * 1.0 = 48f
assertEquals(48f, MotionTextSizeProvider.getFontSize(aspectRatio, MotionTextVariant.P), 0.01f)
}

@Test
fun testBaseTextScale() {
val aspectRatio = VideoAspectRatio.Ratio16x9_1080 // 1920x1080, min is 1080, scale is 1.0

MotionTextSizeProvider.baseTextScale = 2.0f

// H1 should be 160f * 2.0 = 320f
assertEquals(320f, MotionTextSizeProvider.getFontSize(aspectRatio, MotionTextVariant.H1), 0.01f)

// P should be 48f * 2.0 = 96f
assertEquals(96f, MotionTextSizeProvider.getFontSize(aspectRatio, MotionTextVariant.P), 0.01f)
}

@Test
fun testScalingWithAspectRatio() {
// 480p 16:9 is 854x480, min is 480.
// Scale = 480 / 1080 = 0.4444...
val aspectRatio = VideoAspectRatio.Ratio16x9_480

val expectedH1 = 160f * (480f / 1080f)
assertEquals(expectedH1, MotionTextSizeProvider.getFontSize(aspectRatio, MotionTextVariant.H1), 0.01f)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class FFMpegVideoFrameView(
val videoFile: File,
override val startFrame: Int,
override val endFrame: Int,
override val loop: Pair<Int, Int> = Pair(0, 0),
override var loop: Pair<Int, Int> = Pair(0, 0),
effects: List<MotionEffect> = emptyList(),
) : FrameLayout(context),
MotionView {
Expand Down
2 changes: 2 additions & 0 deletions modules/lyrics-maker/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,12 @@ dependencies {
implementation libs.picasso

implementation project(path: ':modules:motionlib')
implementation project(path: ':modules:motion-video-player')
implementation project(path: ':modules:motion-store')
implementation project(path: ':modules:metadata-extractor')
implementation project(path: ':modules:ffmpeg-motion-ext')
implementation project(path: ':modules:sdui')
implementation project(path: ':modules:templates')

implementation platform(libs.androidx.compose.bom)
implementation libs.bundles.compose
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ sealed class Screen(

object Lyrics : Screen("lyrics")

object TemplateSelector : Screen("template_selector/{projectId}") {
fun createRoute(projectId: String) = "template_selector/$projectId"
}

object ProjectDetails : Screen("project_details/{projectId}") {
fun createRoute(projectId: String) = "project_details/$projectId"
}
Expand Down Expand Up @@ -106,11 +110,38 @@ fun AppNavHost(

projectsViewModel.loadProjects()

navController.navigate(Screen.ProjectDetails.createRoute(projectId))
navController.navigate(Screen.TemplateSelector.createRoute(projectId))
},
)
}

composable(route = Screen.TemplateSelector.route) { backStackEntry ->
val projectId = backStackEntry.arguments?.getString("projectId")
val projects = projectsViewModel.projects.collectAsStateWithLifecycle()
val project = projects.value.find { it.id == projectId }

project?.let {
LyricsTemplateSelector(
project = it,
onBack = { navController.popBackStack() },
onTemplateSelected = { template ->
it.metadata.addProperty("template", template.name)
// Clear old SDUI if any
it.metadata.remove("sdui")

navController.context.asLyricsApp().motionStoreDao.upsert(it)
projectsViewModel.loadProjects()

navController.navigate(Screen.ProjectDetails.createRoute(it.id)) {
// Pop the template selector so back from details goes to lyrics
popUpTo(Screen.TemplateSelector.route) { inclusive = true }
}
},
modifier = modifier,
)
}
}

composable(route = Screen.ProjectDetails.route) { backStackEntry ->
val projectId = backStackEntry.arguments?.getString("projectId")
val projects = projectsViewModel.projects.collectAsStateWithLifecycle()
Expand Down
Loading
Loading