Skip to content

feat(FFmpegBuilder): support -f for input#22

Merged
bramp merged 1 commit intobramp:masterfrom
davinkevin:master
May 23, 2016
Merged

feat(FFmpegBuilder): support -f for input#22
bramp merged 1 commit intobramp:masterfrom
davinkevin:master

Conversation

@davinkevin
Copy link
Copy Markdown
Contributor

In order to support the input format parameters, a format attribute have been added to the FFmpegBuilder class and handle in the build method.
The parameter is mandatory for some operation like concatenation without re-encode (https://www.ffmpeg.org/ffmpeg-formats.html#concat). With this extra parameter, we can generate a command line like this : /usr/local/bin/ffmpeg -f concat -i /path/to/file.txt -vcodec copy -acodec copy /path/to/file.mp4

In order to support the input format parameters, a format attribute have been added to the FFmpegBuilder class and handle in the build method.
The parameter is mandatory for some operation like concatenation without re-encode (https://www.ffmpeg.org/ffmpeg-formats.html#concat). With this extra parameter, we can generate a command line like this : /usr/local/bin/ffmpeg -f concat -i /path/to/file.txt -vcodec copy -acodec copy /path/to/file.mp4
davinkevin added a commit to davinkevin/Podcast-Server that referenced this pull request May 22, 2016
In version 3.0.2, the concat operation doesn't working with FILE_1|FILE_2|FILE_3 syntax, so we have to create an intermediate file which contains all the information about the source to be concatenated. This modification is linked to the need of a better implementation for ffmpeg command line call.

Linked to #68 and waiting for bramp/ffmpeg-cli-wrapper#22
@davinkevin
Copy link
Copy Markdown
Contributor Author

@bramp Do you see any problem with this PR ?

I'm currently working on a project using ffmpeg-cli-wrapper (https://github.com/davinkevin/Podcast-Server), but this functionality (in this PR) is missing, so now I'm working on a SNAPSHOT locally released from my fork, but I would like to know if it will be possible to use the official release with PR in a near future.

Thanks for your help

@bramp bramp merged commit c7a7028 into bramp:master May 23, 2016
@bramp
Copy link
Copy Markdown
Owner

bramp commented May 23, 2016

Thanks. Looks good to me!

@bramp
Copy link
Copy Markdown
Owner

bramp commented May 23, 2016

I'll roll a new release shortly.

@davinkevin
Copy link
Copy Markdown
Contributor Author

davinkevin commented May 23, 2016

Thanks.

In futur, I will surely need to have the functionality #21 so I will discuss with you to find the best way to implement it, and like that, I could do a PR ;)

Thanks again and have a good day

@bramp
Copy link
Copy Markdown
Owner

bramp commented May 23, 2016

Happy to accept a PR for #21. I honestly haven't given it much thought, but if you have a proposal please add it to the bug #21, and I'd be very happy to accept PRs.

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