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

Refactors the video pipeline generation from video.py to pipeline_builder.py #444

Merged
merged 6 commits into from
Dec 26, 2023

Conversation

xsdg
Copy link
Collaborator

@xsdg xsdg commented Dec 26, 2023

This is not a 1-to-1 refactor. There was a lot of not-clearly-intentional
inconsistency in the original implementation which was not all carried over.
Among other changes:

  • The original implementation used the veryfast preset in some places, and
    ultrafast in others. Those presets are described at
    https://trac.ffmpeg.org/wiki/Encode/H.264#a2.Chooseapresetandtune
    The new implementation uses veryfast across the board.
  • The logic around resolution, user_command, input_file, and subtitles was
    extremely confusing. This has been significantly normalized. I wouldn't be
    surprised if this results in regressions, but we'll be better off fixing those
    regressions in the new implementation rather than trying to figure out how/why
    they worked in the old one.
  • Argument placement in the generated commands will be much more stable across
    input conditions than they were in the legacy implementation. This may cause
    regressions in cases where ffmpeg was sensitive to the specific order of those
    arguments in different conditions.

This refactor makes significant progress on all three subgoals of #434. Most
importantly, there is no longer any work done at import time when importing
video.py.

@xsdg xsdg changed the title Moves the video pipeline generation from video.py to pipeline_builder.py Refactors the video pipeline generation from video.py to pipeline_builder.py Dec 26, 2023
@xsdg xsdg merged commit 940c4c9 into muammar:master Dec 26, 2023
@xsdg xsdg deleted the video branch December 26, 2023 03:21
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.

1 participant