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

Expose metadata as dataclasses - first episode #52

Closed
wants to merge 13 commits into from

Conversation

NicolasHug
Copy link
Member

@NicolasHug NicolasHug commented Jun 26, 2024

This PR Implements docs.google.com/document/d/1OislJC2eeaI2X38Ufy8fDRJDrEj1lcSNtZ-8L-CeQm8/edit from @scotts, with the amendments recently suggested after first review rounds.

Main changes:

  • Add the dataclasses from the design doc
  • Expose the _core.get_video_metadata() (still private within _core).
  • Expose the public stream_metadata attribute of SimpleVideoDecoder
  • Added 2 new private custom ops:
    _get_container_json_metadata(decoder)
    _get_stream_json_metadata(decoder, stream_index)

Notes:

  • We're keeping the existing get_json_metadata() C++ and Python functions untouched. They are currently in use internally, and it'll be easier to migrate that internal code in a separate diff.
  • We are still in the business of manually creating a json string from C++. We may prefer doing things differently in the future but for now this is OK, and in any case revamping this isn't in scope for this PR. We left a TODO for that.

Still TODO, in following diffs:

  • Expose probe_video_metadata_headers() from the proposal.
  • Find more descriptive names than num_frames_computed and num_frames_retrieved. There's a TODO for that
  • Decide and implement logic of the bit_rate and duration_seconds properties. There are TODOs for that.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jun 26, 2024
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

@NicolasHug merged this pull request in c82f142.

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. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants