Skip to content
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.

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

Fix rotation detection for HDR videos #50854

Merged
merged 1 commit into from Jan 23, 2024

Conversation

railsbob
Copy link
Contributor

@railsbob railsbob commented Jan 23, 2024

Motivation

Fixes #50853

Detail

The video analyzer was relying on the positional reference of side_data to fetch the rotation metadata. However, the side_data is not guaranteed to be in the same position for all videos. For instance, HDR videos shot on iOS in portrait mode have "DOVI configuration record" side_data in the first position, followed by the "Display matrix" side data containing the rotation value.

This fix removes the positional reference and explicitely searches for the "Display Matrix" side_data to retrieve the rotation angle.

Additional information

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

Fixes rails#50853

The video analyzer was relying on the positional reference of the Display
Matrix side_data to fetch the rotation value. However, the side_data is
not guaranteed to be in the same position. For instance, HDR videos shot
on iOS have "DOVI configuration record" side_data in the first position,
followed by the "Display Matrix" side data containing the rotation value.

This fix removes the positional reference and explicitely searches for
the "Display Matrix" side_data to retrieve the rotation angle.
@railsbob railsbob force-pushed the activestorage/analyze-rotated-video branch from 9f2a62b to 0ff649a Compare January 23, 2024 17:32
@railsbob
Copy link
Contributor Author

The build timed out waiting on test/cases/reaper_test.rb. Could someone with access trigger it once more, please?

@dhh dhh merged commit 776626f into rails:main Jan 23, 2024
4 checks passed
viralpraxis pushed a commit to viralpraxis/rails that referenced this pull request Mar 24, 2024
Fixes rails#50853

The video analyzer was relying on the positional reference of the Display
Matrix side_data to fetch the rotation value. However, the side_data is
not guaranteed to be in the same position. For instance, HDR videos shot
on iOS have "DOVI configuration record" side_data in the first position,
followed by the "Display Matrix" side data containing the rotation value.

This fix removes the positional reference and explicitely searches for
the "Display Matrix" side_data to retrieve the rotation angle.
fractaledmind pushed a commit to fractaledmind/rails that referenced this pull request May 13, 2024
Fixes rails#50853

The video analyzer was relying on the positional reference of the Display
Matrix side_data to fetch the rotation value. However, the side_data is
not guaranteed to be in the same position. For instance, HDR videos shot
on iOS have "DOVI configuration record" side_data in the first position,
followed by the "Display Matrix" side data containing the rotation value.

This fix removes the positional reference and explicitely searches for
the "Display Matrix" side_data to retrieve the rotation angle.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Video analyzer does not detect rotation of HDR videos
2 participants