Skip to content

Update C++ metadata names to match python #707

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

Merged
merged 14 commits into from
Jun 5, 2025

Conversation

Dan-Flores
Copy link
Contributor

This PR updates the fields of StreamMetadata and ContainerMetadata in Metadata.h to match the names of the equivalent fields in Python. This aligns the mismatched variables observed in #125.

In StreamMetadata:

  • durationSeconds -> durationSecondsFromHeader
  • beginStreamFromHeader -> beginStreamSecondsFromHeader
  • numFrames -> numFramesFromHeader
  • averageFps -> averageFpsFromHeader
  • minPtsSecondsFromScan -> beginStreamSecondsFromContent
  • maxPtsSecondsFromScan -> endStreamSecondsFromContent
  • numFramesFromScan -> numFramesFromContent

In ContainerMetadata:

  • durationSeconds -> durationSecondsFromHeader

These changes are reflected in other references to these metadata classes, in SingleStreamDecoder.h/.cpp, custom_ops.cpp, video_clip_sampler.py, and to the references in test files test_ops.py, VideoDecoderTest.cpp

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label May 30, 2025
@Dan-Flores Dan-Flores marked this pull request as ready for review May 30, 2025 21:05
Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @Dan-Flores , LGTM !

Feel free to rename minPtsFromScan and maxPtsFromScan in this PR or in a follow-up, up to you.

I suspect that this PR may cause internal tests to fail, because some internal code-bases are relying on the json metadata keys that are now being renamed (example: https://fburl.com/code/0c3g3xt7).
There shouldn't be too many internal instances to fix, but to be on the safe side it might be best to import this PR as a diff before we merge it on GitHub, and pre-emptively fix issues in a separate diff to keep our diff-train healthy.

There should be an "import to fbsource" button a the bottom of this PR, next to the merge button. Happy to sync more offline on this.

@facebook-github-bot
Copy link
Contributor

@Dan-Flores has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@Dan-Flores has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@Dan-Flores Dan-Flores merged commit ff35e16 into pytorch:main Jun 5, 2025
46 of 49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants