Skip to content

Implement subtitle parser and display it in VideoActivity view #46

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
tuancoltech wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Implement subtitle parser and display it in VideoActivity view #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
32 changes: 30 additions & 2 deletions app/src/main/java/ai/elimu/filamu/ui/video/VideoActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import android.os.Bundle
import androidx.annotation.OptIn
import androidx.appcompat.app.AppCompatActivity
import androidx.lifecycle.ViewModelProvider
import androidx.lifecycle.lifecycleScope
import androidx.media3.common.MediaItem
import androidx.media3.common.MimeTypes
import androidx.media3.common.PlaybackException
Expand All @@ -19,6 +20,9 @@ import androidx.media3.exoplayer.ExoPlayer
import androidx.media3.exoplayer.source.ProgressiveMediaSource
import androidx.media3.exoplayer.trackselection.DefaultTrackSelector
import dagger.hilt.android.AndroidEntryPoint
import kotlinx.coroutines.Job
import kotlinx.coroutines.delay
import kotlinx.coroutines.launch
import timber.log.Timber

@AndroidEntryPoint
Expand All @@ -29,14 +33,14 @@ class VideoActivity : AppCompatActivity() {
private lateinit var videoPlayer: ExoPlayer
private lateinit var binding: ActivityVideoBinding
private lateinit var videoViewModel: VideoViewModel
private var synchronizationJob: Job? = null

@OptIn(UnstableApi::class)
override fun onCreate(savedInstanceState: Bundle?) {
Timber.tag(TAG).i("onCreate")
super.onCreate(savedInstanceState)

binding = ActivityVideoBinding.inflate(layoutInflater)

setContentView(binding.root)

initViewModels()
Expand Down Expand Up @@ -92,7 +96,6 @@ class VideoActivity : AppCompatActivity() {
super.onRenderedFirstFrame()
Timber.tag(TAG).d("onRenderedFirstFrame")
}

})

// Prepare and play
Expand All @@ -101,10 +104,35 @@ class VideoActivity : AppCompatActivity() {
videoPlayer.prepare()
videoPlayer.play()
}

// Load subtitles
videoViewModel.loadSubtitles("sample.srt")

Comment on lines +107 to +110
Copy link

@coderabbitai coderabbitai bot Apr 25, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Hardcoded subtitle filename limits flexibility

Using a hardcoded subtitle filename "sample.srt" limits the application's ability to load different subtitle files for different videos. Consider making this dynamic based on the video being played.

- // Load subtitles
- videoViewModel.loadSubtitles("sample.srt")
+ // Load subtitles based on video ID
+ val subtitleFileName = "video_${videoId}_subtitles.srt"
+ videoViewModel.loadSubtitles(subtitleFileName)
📝 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
// Load subtitles
videoViewModel.loadSubtitles("sample.srt")
// Load subtitles based on video ID
val subtitleFileName = "video_${videoId}_subtitles.srt"
videoViewModel.loadSubtitles(subtitleFileName)

// Synchronize subtitles with video playback
videoPlayer.addListener(object : Player.Listener {
override fun onPlaybackStateChanged(playbackState: Int) {
if (playbackState == Player.STATE_READY) {
synchronizeSubtitles()
}
}
})
}

private fun synchronizeSubtitles() {
synchronizationJob?.cancel()
synchronizationJob = lifecycleScope.launch {
while (true) {
val currentPosition = videoPlayer.currentPosition
val subtitle = videoViewModel.getCurrentSubtitle(currentPosition)
binding.subtitleTextView.text = subtitle?.text ?: ""
delay(500) // Check subtitle updates every 500ms
}
}
}
Comment on lines +121 to 131
Copy link

@coderabbitai coderabbitai bot Apr 25, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for subtitle synchronization

The synchronization coroutine doesn't include any error handling. An uncaught exception could terminate the subtitle display.

private fun synchronizeSubtitles() {
    synchronizationJob?.cancel()
    synchronizationJob = lifecycleScope.launch {
+       try {
            while (true) {
                val currentPosition = videoPlayer.currentPosition
                val subtitle = videoViewModel.getCurrentSubtitle(currentPosition)
                binding.subtitleTextView.text = subtitle?.text ?: ""
                delay(500) // Check subtitle updates every 500ms
            }
+       } catch (e: Exception) {
+           Timber.tag(TAG).e("Subtitle synchronization error: ${e.message}")
+           // Retry synchronization after a delay
+           delay(1000)
+           synchronizeSubtitles()
+       }
    }
}
📝 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
private fun synchronizeSubtitles() {
synchronizationJob?.cancel()
synchronizationJob = lifecycleScope.launch {
while (true) {
val currentPosition = videoPlayer.currentPosition
val subtitle = videoViewModel.getCurrentSubtitle(currentPosition)
binding.subtitleTextView.text = subtitle?.text ?: ""
delay(500) // Check subtitle updates every 500ms
}
}
}
private fun synchronizeSubtitles() {
synchronizationJob?.cancel()
synchronizationJob = lifecycleScope.launch {
try {
while (true) {
val currentPosition = videoPlayer.currentPosition
val subtitle = videoViewModel.getCurrentSubtitle(currentPosition)
binding.subtitleTextView.text = subtitle?.text ?: ""
delay(500) // Check subtitle updates every 500ms
}
} catch (e: Exception) {
Timber.tag(TAG).e("Subtitle synchronization error: ${e.message}")
// Retry synchronization after a delay
delay(1000)
synchronizeSubtitles()
}
}
}


override fun onDestroy() {
super.onDestroy()
synchronizationJob?.cancel()
videoPlayer.release()
}

Expand Down
3 changes: 3 additions & 0 deletions app/src/main/java/ai/elimu/filamu/ui/video/model/Subtitle.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package ai.elimu.filamu.ui.video.model

data class Subtitle(val startTime: Long, val endTime: Long, val text: String)
50 changes: 50 additions & 0 deletions app/src/main/java/ai/elimu/filamu/util/SubtitleUtil.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package ai.elimu.filamu.util

import ai.elimu.filamu.ui.video.model.Subtitle
import android.content.res.AssetManager
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext
import timber.log.Timber
import java.io.BufferedReader
import java.io.InputStreamReader
import java.util.regex.Pattern

object SubtitleUtil {

suspend fun loadSubtitles(assetManager: AssetManager, fileName: String): List<Subtitle> {
return withContext(Dispatchers.IO) {
val subtitles = mutableListOf<Subtitle>()
try {
val inputStream = assetManager.open(fileName)
val reader = BufferedReader(InputStreamReader(inputStream))
val pattern = Pattern.compile("(\\d+)\\n(\\d{2}:\\d{2}:\\d{2},\\d{3}) --> (\\d{2}:\\d{2}:\\d{2},\\d{3})\\n(.*)")

var line: String?
val subtitleBuilder = StringBuilder()

while (reader.readLine().also { line = it } != null) {
if (line.isNullOrBlank()) {
val matcher = pattern.matcher(subtitleBuilder.toString())
if (matcher.find()) {
val startTime = parseTime(matcher.group(2)!!)
val endTime = parseTime(matcher.group(3)!!)
val text = matcher.group(4)!!
subtitles.add(Subtitle(startTime, endTime, text))
}
subtitleBuilder.clear()
} else {
subtitleBuilder.append(line).append("\n")
}
}
} catch (e: Exception) {
Timber.e("Error loading subtitles: ${e.message}")
}
subtitles
}
}
Comment on lines +12 to +44
Copy link

@coderabbitai coderabbitai bot Apr 25, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Fix subtitle parsing pattern to handle multi-line subtitles and add proper resource handling

The current implementation has a few issues that could impact subtitle parsing reliability, which is important for elimu.ai's mission to build innovative learning software that empowers out-of-school children to teach themselves basic reading📖, writing✍🏽 and math🔢 within 6 months.

  1. The regular expression pattern only captures single-line subtitles, but SRT files often contain multi-line text
  2. Resource leaks: InputStreams should be closed properly using use extension function
  3. The final subtitle block might not be captured if the file doesn't end with a blank line
  4. Error handling could be more specific
object SubtitleUtil {

    suspend fun loadSubtitles(assetManager: AssetManager, fileName: String): List<Subtitle> {
        return withContext(Dispatchers.IO) {
            val subtitles = mutableListOf<Subtitle>()
            try {
-               val inputStream = assetManager.open(fileName)
-               val reader = BufferedReader(InputStreamReader(inputStream))
-               val pattern = Pattern.compile("(\\d+)\\n(\\d{2}:\\d{2}:\\d{2},\\d{3}) --> (\\d{2}:\\d{2}:\\d{2},\\d{3})\\n(.*)")
+               assetManager.open(fileName).use { inputStream ->
+                   BufferedReader(InputStreamReader(inputStream)).use { reader ->
+                       // Pattern that captures subtitle index, timing and multi-line text content
+                       val pattern = Pattern.compile("(\\d+)\\n(\\d{2}:\\d{2}:\\d{2},\\d{3}) --> (\\d{2}:\\d{2}:\\d{2},\\d{3})\\n((?:.*\\n?)+)")

                        var line: String?
                        val subtitleBuilder = StringBuilder()

                        while (reader.readLine().also { line = it } != null) {
                            if (line.isNullOrBlank()) {
                                val matcher = pattern.matcher(subtitleBuilder.toString())
                                if (matcher.find()) {
                                    val startTime = parseTime(matcher.group(2)!!)
                                    val endTime = parseTime(matcher.group(3)!!)
                                    val text = matcher.group(4)!!
                                    subtitles.add(Subtitle(startTime, endTime, text))
                                }
                                subtitleBuilder.clear()
                            } else {
                                subtitleBuilder.append(line).append("\n")
                            }
                        }
                        
+                       // Process the last subtitle block if it exists
+                       if (subtitleBuilder.isNotEmpty()) {
+                           val matcher = pattern.matcher(subtitleBuilder.toString())
+                           if (matcher.find()) {
+                               val startTime = parseTime(matcher.group(2)!!)
+                               val endTime = parseTime(matcher.group(3)!!)
+                               val text = matcher.group(4)!!
+                               subtitles.add(Subtitle(startTime, endTime, text))
+                           }
+                       }
+                   }
+               }
            } catch (e: Exception) {
-               Timber.e("Error loading subtitles: ${e.message}")
+               Timber.e(e, "Error loading subtitles from %s", fileName)
            }
            subtitles
        }
    }
📝 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
object SubtitleUtil {
suspend fun loadSubtitles(assetManager: AssetManager, fileName: String): List<Subtitle> {
return withContext(Dispatchers.IO) {
val subtitles = mutableListOf<Subtitle>()
try {
val inputStream = assetManager.open(fileName)
val reader = BufferedReader(InputStreamReader(inputStream))
val pattern = Pattern.compile("(\\d+)\\n(\\d{2}:\\d{2}:\\d{2},\\d{3}) --> (\\d{2}:\\d{2}:\\d{2},\\d{3})\\n(.*)")
var line: String?
val subtitleBuilder = StringBuilder()
while (reader.readLine().also { line = it } != null) {
if (line.isNullOrBlank()) {
val matcher = pattern.matcher(subtitleBuilder.toString())
if (matcher.find()) {
val startTime = parseTime(matcher.group(2)!!)
val endTime = parseTime(matcher.group(3)!!)
val text = matcher.group(4)!!
subtitles.add(Subtitle(startTime, endTime, text))
}
subtitleBuilder.clear()
} else {
subtitleBuilder.append(line).append("\n")
}
}
} catch (e: Exception) {
Timber.e("Error loading subtitles: ${e.message}")
}
subtitles
}
}
object SubtitleUtil {
suspend fun loadSubtitles(assetManager: AssetManager, fileName: String): List<Subtitle> {
return withContext(Dispatchers.IO) {
val subtitles = mutableListOf<Subtitle>()
try {
assetManager.open(fileName).use { inputStream ->
BufferedReader(InputStreamReader(inputStream)).use { reader ->
// Pattern that captures subtitle index, timing and multi-line text content
val pattern = Pattern.compile(
"(\\d+)\\n" +
"(\\d{2}:\\d{2}:\\d{2},\\d{3}) --> " +
"(\\d{2}:\\d{2}:\\d{2},\\d{3})\\n" +
"((?:.*\\n?)+)"
)
var line: String?
val subtitleBuilder = StringBuilder()
while (reader.readLine().also { line = it } != null) {
if (line.isNullOrBlank()) {
val matcher = pattern.matcher(subtitleBuilder.toString())
if (matcher.find()) {
val startTime = parseTime(matcher.group(2)!!)
val endTime = parseTime(matcher.group(3)!!)
val text = matcher.group(4)!!.trimEnd()
subtitles.add(Subtitle(startTime, endTime, text))
}
subtitleBuilder.clear()
} else {
subtitleBuilder.append(line).append("\n")
}
}
// Process the last subtitle block if it exists
if (subtitleBuilder.isNotEmpty()) {
val matcher = pattern.matcher(subtitleBuilder.toString())
if (matcher.find()) {
val startTime = parseTime(matcher.group(2)!!)
val endTime = parseTime(matcher.group(3)!!)
val text = matcher.group(4)!!.trimEnd()
subtitles.add(Subtitle(startTime, endTime, text))
}
}
}
}
} catch (e: Exception) {
Timber.e(e, "Error loading subtitles from %s", fileName)
}
subtitles
}
}
}


fun parseTime(time: String): Long {
val parts = time.split(",", ":").map { it.toInt() }
return parts[0] * 3600000 + parts[1] * 60000 + parts[2] * 1000 + parts[3]
}
Comment on lines +46 to +49
Copy link

@coderabbitai coderabbitai bot Apr 25, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling to parseTime method

The time parsing function lacks error handling which could lead to crashes if the subtitle file contains malformed timestamps.

-    fun parseTime(time: String): Long {
-        val parts = time.split(",", ":").map { it.toInt() }
-        return parts[0] * 3600000 + parts[1] * 60000 + parts[2] * 1000 + parts[3]
+    fun parseTime(time: String): Long {
+        return try {
+            val parts = time.split(",", ":").map { it.toInt() }
+            if (parts.size != 4) {
+                Timber.w("Invalid time format: $time, expected hh:mm:ss,ms")
+                0L
+            } else {
+                parts[0] * 3600000 + parts[1] * 60000 + parts[2] * 1000 + parts[3]
+            }
+        } catch (e: NumberFormatException) {
+            Timber.e(e, "Failed to parse time: $time")
+            0L
+        }
     }
📝 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 parseTime(time: String): Long {
val parts = time.split(",", ":").map { it.toInt() }
return parts[0] * 3600000 + parts[1] * 60000 + parts[2] * 1000 + parts[3]
}
fun parseTime(time: String): Long {
return try {
val parts = time.split(",", ":").map { it.toInt() }
if (parts.size != 4) {
Timber.w("Invalid time format: $time, expected hh:mm:ss,ms")
0L
} else {
parts[0] * 3600000 + parts[1] * 60000 + parts[2] * 1000 + parts[3]
}
} catch (e: NumberFormatException) {
Timber.e(e, "Failed to parse time: $time")
0L
}
}

}
14 changes: 14 additions & 0 deletions app/src/main/res/layout/activity_video.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,18 @@
app:layout_constraintTop_toTopOf="parent"
app:layout_constraintBottom_toBottomOf="parent"
android:id="@+id/player_view"/>

<TextView
android:id="@+id/subtitle_text_view"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_gravity="bottom"
android:background="@android:color/black"
android:textColor="@android:color/white"
android:padding="8dp"
android:textSize="16sp"
android:gravity="center"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintEnd_toEndOf="parent"/>
</androidx.constraintlayout.widget.ConstraintLayout>
3 changes: 3 additions & 0 deletions data-video/src/main/java/ai/elimu/filamu/data/video/viewmodel/VideoViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,11 @@ sealed interface LoadVideosUiState {

interface VideoViewModel {
val uiState: StateFlow<LoadVideosUiState>
val subtitles: StateFlow<List<ai.elimu.filamu.ui.video.model.Subtitle>>
fun getAllVideos()
fun getThumb(videoId: Long, onResult: (Bitmap?) -> Unit)
fun getThumbUrl(video: VideoGson, onResult: (String) -> Unit)
fun readVideoBytes(fileId: Long, onResult: (ByteArray?) -> Unit)
fun loadSubtitles(fileName: String)
fun getCurrentSubtitle(currentPosition: Long): ai.elimu.filamu.ui.video.model.Subtitle?
}
22 changes: 21 additions & 1 deletion data-video/src/main/java/ai/elimu/filamu/data/video/viewmodel/VideoViewModelImpl.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ package ai.elimu.filamu.data.video.viewmodel

import ai.elimu.filamu.data.video.data.repository.VideoRepository
import ai.elimu.filamu.data.video.di.IoScope
import ai.elimu.filamu.ui.video.model.Subtitle
import ai.elimu.filamu.util.SubtitleUtil
import ai.elimu.model.v2.gson.content.VideoGson
import android.app.Application
import android.graphics.Bitmap
import androidx.lifecycle.ViewModel
import dagger.hilt.android.lifecycle.HiltViewModel
Expand All @@ -18,7 +21,8 @@ import javax.inject.Inject
@HiltViewModel
class VideoViewModelImpl @Inject constructor(
@IoScope private val ioScope: CoroutineScope,
private val videoRepository: VideoRepository
private val videoRepository: VideoRepository,
private val application: Application
): ViewModel(), VideoViewModel {

private val thumbBaseUrl: String by lazy {
Expand All @@ -28,6 +32,9 @@ class VideoViewModelImpl @Inject constructor(
private val _uiState = MutableStateFlow<LoadVideosUiState>(LoadVideosUiState.Loading)
override val uiState: StateFlow<LoadVideosUiState> = _uiState.asStateFlow()

private val _subtitles = MutableStateFlow<List<Subtitle>>(emptyList())
override val subtitles: StateFlow<List<Subtitle>> = _subtitles.asStateFlow()

override fun getAllVideos() {
ioScope.launch {
_uiState.emit(LoadVideosUiState.Loading)
Expand Down Expand Up @@ -71,4 +78,17 @@ class VideoViewModelImpl @Inject constructor(
}
}
}

override fun loadSubtitles(fileName: String) {
ioScope.launch {
val loadedSubtitles = SubtitleUtil.loadSubtitles(application.assets, fileName)
_subtitles.emit(loadedSubtitles)
}
}
Comment on lines +82 to +87
Copy link

@coderabbitai coderabbitai bot Apr 25, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for subtitle loading

The subtitle loading lacks error handling. If the subtitle file doesn't exist or is malformed, this could crash the application.

override fun loadSubtitles(fileName: String) {
    ioScope.launch {
+       try {
            val loadedSubtitles = SubtitleUtil.loadSubtitles(application.assets, fileName)
            _subtitles.emit(loadedSubtitles)
+       } catch (e: Exception) {
+           // Log error and emit empty list to avoid crashing
+           Timber.e("Failed to load subtitles: ${e.message}")
+           _subtitles.emit(emptyList())
+       }
    }
}
📝 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
override fun loadSubtitles(fileName: String) {
ioScope.launch {
val loadedSubtitles = SubtitleUtil.loadSubtitles(application.assets, fileName)
_subtitles.emit(loadedSubtitles)
}
}
override fun loadSubtitles(fileName: String) {
ioScope.launch {
try {
val loadedSubtitles = SubtitleUtil.loadSubtitles(application.assets, fileName)
_subtitles.emit(loadedSubtitles)
} catch (e: Exception) {
// Log error and emit empty list to avoid crashing
Timber.e("Failed to load subtitles: ${e.message}")
_subtitles.emit(emptyList())
}
}
}


override fun getCurrentSubtitle(currentPosition: Long): Subtitle? {
return _subtitles.value.find {
it.startTime <= currentPosition && it.endTime >= currentPosition
}
}
Comment on lines +89 to +93
Copy link

@coderabbitai coderabbitai bot Apr 25, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Add unit tests for the subtitle handling logic

The subtitle synchronization is a critical feature for educational videos, but there are no unit tests for this functionality. To ensure a reliable learning experience for children using elimu.ai's software, comprehensive testing is essential.

Please verify if there are any unit tests for the subtitle functionality by running:

🏁 Script executed:

#!/bin/bash
# Check for tests related to subtitle functionality
echo "Looking for subtitle-related test files..."
find . -path "*/src/test/*" -name "*Subtitle*Test*.kt" -o -name "*Test*Subtitle*.kt"

echo "Checking for subtitle-related test methods in VideoViewModel tests..."
find . -path "*/src/test/*" -name "*VideoViewModel*Test*.kt" | xargs grep -l "subtitle" || echo "No subtitle tests found in VideoViewModel test files"

Length of output: 533

Add unit tests for subtitle synchronization logic

To uphold elimu.ai’s mission of building innovative learning software that empowers out-of-school children to teach themselves basic reading📖, writing✍🏽 and math🔢 within 6 months, we need to ensure that subtitle timing is bullet-proof. No existing tests cover getCurrentSubtitle, so let’s add them.

• Create a new test class, e.g. VideoViewModelImplSubtitleTest.kt
• Target getCurrentSubtitle(currentPosition: Long) in VideoViewModelImpl.kt
• Cover scenarios:
– Position before any subtitle (expect null)
– Position exactly at start/end boundaries
– Position within a subtitle interval
– Multiple subtitles with overlapping times
– Empty subtitle list

}
Loading