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

Redo SRS export, add command-line options #34

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

benjaminhottell
Copy link

Hi team,

Thank you for your work on this project. I've made the following modifications. Please feel free to accept or reject as much as you'd like.

  • TSV generation code was rewritten to use Python's built-in CSV module, meaning we do not need to use pandas/numpy
  • User can explicitly disable screenshot exports (via --no-export-screenshot). Screenshots are only generated if there is a video stream, and the user has not disabled screenshots.
  • User can explicitly disable audio clip exports (via --no-export-audio). Audio clips are only generated if there is an audio stream, and the user has not disabled audio clips.
  • User can opt into experimental video exports (via --video-export). Video clips are only generated if there is a video stream, and the user has enabled video exports.
  • User can specify a directory to save exported media files. I personally use this to write the media files directly to my Anki collection (this way, I can skip manually copying them over).
  • .gitignore updated to include __pycache__
  • pandas dependency dropped. numpy dependency also dropped

@aaron-meyers
Copy link

This is great - I was hitting issues with srs2cia pypi package (an error about "append" not valid on "DataFrame") which unsurprisingly didn't happen when using @benjaminhottell 's fork that doesn't use pandas anymore. Not sure if there's an incompatibility with a newer version of pandas - I could try running in a python virtualenv to isolate the issue but removing an unneeded pandas dependency seems like a better approach anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants