Skip to content

Conversation

@kristofnemere
Copy link
Contributor

@kristofnemere kristofnemere commented Nov 28, 2025

Test plan

  1. Build and install the Teacher or Student app
  2. Open SpeedGrader for an assignment with submissions that have video attachments
  3. Play a video from the submission content
  4. Navigate to the Comments tab and play a video from a comment
  5. Verify that the submission video stops playing and only the comment video plays
  6. Close the comment video and play the submission video again
  7. Verify that only the submission video plays
  8. Play another video from comments
  9. Verify that only the new comment video plays

refs: MBL-19554
affects: Student, Teacher
release note: Fixed issue where multiple videos would play simultaneously in SpeedGrader

Checklist

  • Dark/light mode testing
  • Landscape/tablet testing
  • Accessibility testing
  • Product approval (if needed)

Fixed issue where multiple MP4 videos played simultaneously when playing
a video from submission and then a video from comments.

Solution: Added ExoAgent.pauseAllOtherAgents() method that pauses all
other playing videos when BaseViewMediaActivity resumes, ensuring only
one video plays at a time.

Test plan:
- Play submission video, then play comment video - verify only comment video plays
- Play comment video, then play submission video - verify only submission video plays
- Play comment video, then play another comment video - verify only new video plays

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Review Summary

This PR adds functionality to pause all other playing videos when a media activity resumes, preventing multiple videos from playing simultaneously. The implementation is straightforward and addresses a real UX issue.

✅ Positives

  • Clear intent: The feature prevents confusing multi-video playback scenarios
  • Simple implementation: Uses existing ExoAgent infrastructure effectively
  • Good code location: Companion object method is appropriately placed for static utility functionality
  • Descriptive naming: pauseAllOtherAgents() clearly communicates what it does

⚠️ Issues Found

  • Thread safety: agentInstances HashMap may have concurrency issues when accessed from multiple activities/fragments simultaneously (libs/pandautils/src/main/java/com/instructure/pandautils/utils/ExoPlayerHelper.kt:256)
  • Logic refinement: onResume() always pauses other videos, even when current video isn't playing - consider checking playback state first (libs/pandautils/src/main/java/com/instructure/pandautils/activities/BaseViewMediaActivity.kt:219)
  • Code style: Nested null checks can be simplified using Kotlin idioms (libs/pandautils/src/main/java/com/instructure/pandautils/utils/ExoPlayerHelper.kt:259-263)

📋 Testing Recommendations

Since there are no unit tests for this functionality, please verify these scenarios manually:

  1. Open video A, let it play, open video B → video A should pause
  2. Background the app while playing → verify behavior on resume
  3. Rotate screen during playback → ensure correct video continues
  4. Rapid navigation between multiple videos → no crashes or race conditions
  5. Multiple video instances in different fragments (if applicable)

🔒 Security & Performance

  • No security concerns identified
  • Performance impact is minimal (iterating over active video instances)
  • No memory leaks apparent, but verify proper cleanup in production

Overall, this is a solid improvement to the media playback experience. The main concerns are around thread safety and edge case behavior during lifecycle events.

@kristofnemere kristofnemere changed the title [MBL-19554] Fix video playback conflict in SpeedGrader [MBL-19554][Student][Teacher] Fix video playback conflict in SpeedGrader Nov 28, 2025
@kristofnemere kristofnemere changed the title [MBL-19554][Student][Teacher] Fix video playback conflict in SpeedGrader [MBL-19554][Student][Teacher] Video playback conflict: previous MP4 keeps playing when a new one starts Nov 28, 2025
@github-actions
Copy link

📊 Code Coverage Report

✅ Student

  • PR Coverage: 42.80%
  • Master Coverage: 42.80%
  • Delta: +0.00%

✅ Teacher

  • PR Coverage: 25.45%
  • Master Coverage: 25.45%
  • Delta: +0.00%

✅ Pandautils

  • PR Coverage: 22.65%
  • Master Coverage: 22.65%
  • Delta: +0.00%

📈 Overall Average

  • PR Coverage: 30.30%
  • Master Coverage: 30.30%
  • Delta: +0.00%

@github-actions
Copy link

🧪 Unit Test Results

✅ 📱 Student App

  • Tests: 1226 total, 0 failed, 0 skipped
  • Duration: 0.000s
  • Success Rate: 100%

✅ 📱 Teacher App

  • Tests: 364 total, 0 failed, 0 skipped
  • Duration: 35.285s
  • Success Rate: 100%

✅ 🌅 Horizon

  • Tests: 449 total, 0 failed, 0 skipped
  • Duration: 34.053s
  • Success Rate: 100%

✅ 📦 Submodules

  • Tests: 2418 total, 0 failed, 0 skipped
  • Duration: 47.128s
  • Success Rate: 100%

📊 Summary

  • Total Tests: 4457
  • Failed: 0
  • Skipped: 0
  • Status: ✅ All tests passed!

Last updated: Fri, 28 Nov 2025 13:46:00 GMT

@github-actions
Copy link

Teacher Install Page

@github-actions
Copy link

Student Install Page

Copy link
Contributor

@adamNagy56 adamNagy56 left a comment

Choose a reason for hiding this comment

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

QA +1

@kristofnemere kristofnemere merged commit 8f4bda2 into master Dec 5, 2025
32 checks passed
@kristofnemere kristofnemere deleted the MBL-19554-fix-video-playback-conflict branch December 5, 2025 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants