Skip to content

Conversation

matthewjt
Copy link

Added -filter_complex argument filter to FFmpeg Stream Builder
Resolved error in ExamplesTest

@coveralls
Copy link

coveralls commented Nov 29, 2017

Coverage Status

Changes Unknown when pulling 1643aa5 on matthewjt:video_filter_complex into ** on bramp:master**.

@bramp
Copy link
Owner

bramp commented Nov 29, 2017

The change looks good. A couple of comments/questions

  1. Please add a test case, to ensure this continues to work in the future.

  2. Can the -complex_filter flag per used once per output, or only once? As implemented it looks like we can do:

FFmpegBuilder builder = new FFmpegBuilder()
  .setInput("input.avi")
  .addOutput("output1.mp3")
    .setComplexFilter("...")
  .addOutput("output2.mp3")
    .setComplexFilter("...")

Is that valid? I suspect ComplexFilter is only allowed to be invoked once? Thus it might be better to move it to the FFmpegBuilder? But I'm not sure I fully understand the semantics of this flag.

@matthewjt
Copy link
Author

Wow you're right. It should only be set once. I went to move it into the FFmpegBuilder and found that it already exists there! I guess we don't need this pull request after all.

@bramp
Copy link
Owner

bramp commented Dec 1, 2017

Oh great, sorry I didn't realise that before. Happy to receive contribution in future.

@bramp bramp closed this Dec 1, 2017
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.

3 participants