Skip to content

Align variable and type names across the codebase #462

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 49 commits into from
Jan 23, 2025

Conversation

NicolasHug
Copy link
Member

@NicolasHug NicolasHug commented Jan 22, 2025

Needs #440 to be merged first.

This is the continuation of #454, extending it to the whole codebase. This PR renames types and variable names to make them more consistent, and to make it more obvious what they refer to. Specifically:

  • use streamInfo for StreamInfo structs
  • use avStream for AVStream structs
  • use streamIndex for stream indices (instead of streamNumber)
  • use avFrame for AVFrame structs
  • use streamMetadata for StreamMetadata objects
  • rename the streams_ field of ContainerMetadata into streamMetadatas. The s at the end feels a bit weird. Maybe this should be streamsMetadata, but that would be inconsistent with other similar vec fields, like StreamInfos (where the s is at the end). No strong opinion.
  • renamed VideoStreamDecoderOptions into VideoStreamOptions
  • use videoStreamOptions for VideoStreamOptions structs instead of just options. I realized the option were per-stream, not per-decoder (as I previously assumed, since this is what it's like in Python).

scotts and others added 30 commits December 16, 2024 07:41
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jan 22, 2025
@scotts
Copy link
Contributor

scotts commented Jan 22, 2025

rename the streams_ field of ContainerMetadata into streamMetadatas. The s at the end feels a bit weird. Maybe this should be streamsMetadata, but that would be inconsistent with other similar vec fields, like StreamInfos (where the s is at the end). No strong opinion.

I prefer streamsMetadata to streamMetadatas. Yeah, it's inconsistent with streamInfos, but "datas" just read wrong to me. But maybe allStreamMetadata is better than both.

@NicolasHug
Copy link
Member Author

CPU jobs are failling with an unrelated dep-related issue. All GPU jobs are compiling and running the tests, so this is good to land. Thanks for the review!

@NicolasHug NicolasHug merged commit f565e86 into pytorch:main Jan 23, 2025
29 of 41 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.

3 participants