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

Move helper functions out of common utility for better locality #2512

Closed
wants to merge 2 commits into from

Conversation

mthrok
Copy link
Collaborator

@mthrok mthrok commented Jun 28, 2022

This commits move helper functions/definitions around so that better locality of logics are achieved.

Detail

ffmpeg.[h|cpp] implements classes that convert FFmpeg structures into RAII semantics.
Initially it these classes included the construction logic in their constructors, but such logics were
extracted to factory functions in #2373.

Now the reason why the factory functions stayed in ffmpeg.[h|cpp] was because the logic for
the initialization and clean-up of AVDictionary class was only available in ffmpeg.cpp.

Now AVDictionary class handling is properly defined in #2507, the factory functions, which are not
that reusable better stay with the implementation that use them.

This makes ffmpeg.h lean and clean, makes it easier to see what can be reused.

@facebook-github-bot
Copy link
Contributor

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

@mthrok mthrok marked this pull request as ready for review July 6, 2022 19:36
@facebook-github-bot
Copy link
Contributor

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

@github-actions
Copy link

github-actions bot commented Jul 7, 2022

Hey @mthrok.
You merged this PR, but labels were not properly added. Please add a primary and secondary label (See https://github.com/pytorch/audio/blob/main/.github/process_commit.py)

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.

3 participants