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

Add 'multiply_stereo_volume' FX; rename 'volumex' to 'multiply_volume' #1424

Merged
merged 10 commits into from
Jan 15, 2021

Conversation

mondeja
Copy link
Collaborator

@mondeja mondeja commented Jan 12, 2021

This implements the FX that controls the volume level in two channels for stereo, but also works for more channels: mono (volume control), or more than 2 channels, in this case works properly if first channel of every pair is the left.

  • I have added suitable tests demonstrating a fixed bug or new/changed feature to the test suite in tests/
  • I have properly documented new or changed features in the documentation or in the docstrings
  • I have properly explained unusual or unexpected code in the comments around it
  • I have formatted my code using black -t py36

@tburrows13 tburrows13 added the feature New addition to the API i.e. a new class, method or parameter. label Jan 12, 2021
@tburrows13
Copy link
Collaborator

This is great. A couple of thoughts:
I don't really like the name audio_volume_lr. I'd prefer not to use l and r in place of left and right. I can't think of a good name yet, so maybe you had some more ideas? audio_left_right isn't great either. audio_adjust_stereo?

Would you like to add a changelog entry? If not I'll do it before merging.

@mondeja
Copy link
Collaborator Author

mondeja commented Jan 12, 2021

I don't really like the name audio_volume_lr. I'd prefer not to use l and r in place of left and right.

Yes, I was thinking the same, but audio_left_right seems too much general to me, I prefer audio_adjust_stereo, but this could be confused with panning. Maybe audio_stereo_volume, or audio_volume_stereo, or audio_volume_left_right?

Would you like to add a changelog entry?

I will add it 👍

@mondeja mondeja added the fx Related to advanced effects applied via clip.fx(), or effects in general. label Jan 15, 2021
@mondeja mondeja requested review from tburrows13 and removed request for tburrows13 January 15, 2021 11:10
moviepy/audio/fx/audio_volume_lr.py Outdated Show resolved Hide resolved
moviepy/audio/fx/audio_volume_lr.py Outdated Show resolved Hide resolved
moviepy/audio/fx/audio_volume_lr.py Outdated Show resolved Hide resolved
@mondeja mondeja marked this pull request as draft January 15, 2021 12:59
@mondeja mondeja added the audio Related to AudioClip, or handling of audio in general. label Jan 15, 2021
@mondeja mondeja changed the title Implement left-right audio volume FX Implement left-right audio volume FX; rename 'volumex' to 'audio_volume' Jan 15, 2021
@mondeja mondeja added the breaking-change Must not merge without proper approval. Requires full documentation (own section) in the changelog. label Jan 15, 2021
@mondeja mondeja changed the title Implement left-right audio volume FX; rename 'volumex' to 'audio_volume' Implement 'audio_stereo_volume' FX; rename 'volumex' to 'audio_volume' Jan 15, 2021
@mondeja mondeja changed the title Implement 'audio_stereo_volume' FX; rename 'volumex' to 'audio_volume' Add 'audio_stereo_volume' FX; rename 'volumex' to 'audio_volume' Jan 15, 2021
@mondeja mondeja marked this pull request as ready for review January 15, 2021 14:02
@mondeja

This comment has been minimized.

@mondeja mondeja added the design-decisions Discussion of ideas, suggestions, questions relevant to (re)design decision-making process. label Jan 15, 2021
@mondeja mondeja marked this pull request as draft January 15, 2021 14:14
@mondeja

This comment has been minimized.

@mondeja mondeja changed the title Add 'audio_stereo_volume' FX; rename 'volumex' to 'audio_volume' Add 'multiply_stereo_volume' FX; rename 'volumex' to 'multiply_volume' Jan 15, 2021
@mondeja mondeja removed the design-decisions Discussion of ideas, suggestions, questions relevant to (re)design decision-making process. label Jan 15, 2021
@mondeja mondeja marked this pull request as ready for review January 15, 2021 19:36
@coveralls
Copy link

coveralls commented Jan 15, 2021

Coverage Status

Coverage increased (+0.2%) to 66.159% when pulling 3d3464e on mondeja:audio-volume-left-right into 727276f on Zulko:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audio Related to AudioClip, or handling of audio in general. breaking-change Must not merge without proper approval. Requires full documentation (own section) in the changelog. feature New addition to the API i.e. a new class, method or parameter. fx Related to advanced effects applied via clip.fx(), or effects in general.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants