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

Restructure and clean up the video/video_reader C++ implementations #3155

Closed
6 tasks done
datumbox opened this issue Dec 10, 2020 · 2 comments · Fixed by #3311
Closed
6 tasks done

Restructure and clean up the video/video_reader C++ implementations #3155

datumbox opened this issue Dec 10, 2020 · 2 comments · Fixed by #3311
Assignees

Comments

@datumbox
Copy link
Contributor

datumbox commented Dec 10, 2020

🐛 Bug

The C++ implementations of video and video_reader need to be refactored and cleaned up similar to #2998, #3051, #3094, #3097, #3135, #3163:

  • Rename C++ files, methods & variables according to recommended naming conventions.
  • Move C++ code away from header files and into cpp files.
  • Ensure each cpp file has a separate header file with "public" functions.
  • Reduce unnecessary header inclusions.
  • Add all internal functions in anonymous namespaces.
  • Sync variable names between the cpp files and their header files.
@fmassa
Copy link
Member

fmassa commented Dec 11, 2020

For the decoder folder, I recommend leaving it unchanged and not performing any changes to its code.

My thinking was that we should instead make things a bit more organized in the sense that it now seems to be 3 different projects, but instead decoder is a generic abstraction around ffmpeg and the other two leverages decoder to expose ffmpeg functionality to Python.

My thinking was to re-structure those three libraries to instead have a more natural flow.

@datumbox
Copy link
Contributor Author

@fmassa Sounds good. I've updated the issue description to reflect that.

@datumbox datumbox changed the title Restructure and clean up the decoder/video/video_reader C++ implementations Restructure and clean up the video/video_reader C++ implementations Dec 11, 2020
@datumbox datumbox self-assigned this Jan 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants