Feature: Add FFmpegControllerBuilder to core (#58)

[andcscott]

This PR closes #58 and makes the following changes:

1. Removed the input/output file vars from FFmpegSettingsManager. Other references to those vars were already removed in #51.
2. Added the header for FFmpegController. The final implementation will come down to how we progress on the current class, however I already hit my 4 for hacktoberfest - no big deal if you want it included with this PR.
3. Added FFmpegControllerBuilder. This new class will accept the settings for FFmpegSettingsManager and return a unique ptr to a FFmpegController. The intent is that ownership of all references will be transferred to the newly built Controller, which I felt was especially important in the case where the builder receives a reference and passes it back.

[andcscott]

I've realized we have a bit of redundancy in these builders, where we go through the same settings that were already set and validated (to be done) in FFmpegSettingsManager.

I'm glad you said something, because as I was writing it the whole class felt a little redundant. As it is we can technically do everything we need with the rest of the logic in the actual Controllers methods behind the scenes. I still see why you might want it for the future though, so no worries.

To improve this, let’s introduce an applySettings() private member fn that will be invoked during the build stage to handle these settings internally.

Replied above - I don't think I'm understanding correctly, if the user already defines settings with SettingsManager then what should be applied and where? It probably isn't necessary for this class to take a reference, it could also initialize the SettingsManager and the user would interface only with this class and FFmpegController. That also feels redundant though.

I’ve added some notes about const correctness and ownership semantics. Let me know if you have any feedback on those as well.

Also replied above - I'm concerned making the var const in ControllerBuilder will negate the purpose of the public methods.

Regarding hacktoberfest, I hope I haven’t missed labeling any of your PRs, and I enjoyed working with you. I hope you stick around! We’ll keep the scope of this PR focused on the current implementation. If your availability changes, please let me know. Thank you!

Sounds good! I definitely had fun with this one! At the least I'll stick around to help getting the rest of the controller done, even if we go beyond Hacktoberfest it would bother me to leave something unfinished. I had been looking at #30 too as I have some experience there, although there's a lot to consider with all the different hardware end users can have.

[omeryusufyagci]

Thanks for the feedback. At the beginning I wanted to add this layer to future proof against potentially increased complexity.

Howeverr, as you said, we could very well do all of this within the controller. Only the file I/O feels out of place. We could revert settings manager again to be a config manager..put the I/O there, pass a const ref of the settings to the controller, and provide an updateSettings() public member fn to allow for updates without re-init of the controller.

Don't hesitate to correct me if I went off track here, but unless you think of a reason to keep this class, I'm okay with abandoning it, and directly moving on to the controller.

The initial version of the controller should cover the use cases we currently have in the AudioProcessor and VideoProcessor classes.

Thanks for your understanding with the back-and-forth on this.

[omeryusufyagci]

After giving it more thought, let's just add more when we need more and not try to anticipate everything at this early stage... I've pushed an update on the dedicated branch for #60 to revert the settings manager back to what it was with some minor improvements on file path handling.

I also provided there the existing uses of ffmpeg. Covering these will be sufficient for your controller PR. I'll close this PR and the underlying issue and you can submit a PR directly for the controller.

Thanks!

[andcscott]

All good, and sorry I wasn't able to get back to you yesterday. Thanks for being receptive to feedback and detailing the use cases! I'll go ahead with the #60 branch.