From 7867cf01b8da91af65a3f8b4234107acffa06e07 Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" Date: Fri, 14 Feb 2020 11:42:31 -0500 Subject: [PATCH] Reorder arguments in setVideoOptions overload - The new ordering (with the frame rate AFTER width and height) doesn't match the other signature, but it *is* consistent with the Timeline constructor, and it just feels more natural - Added overloaded-function notes to doxygen strings in FFmpegWriter.h - Also added a warning about the argument order mismatch above --- include/FFmpegWriter.h | 17 +++++++++++++++-- src/FFmpegWriter.cpp | 5 +++-- tests/FFmpegWriter_Tests.cpp | 2 +- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/include/FFmpegWriter.h b/include/FFmpegWriter.h index 654e72cc..b25778e5 100644 --- a/include/FFmpegWriter.h +++ b/include/FFmpegWriter.h @@ -285,6 +285,8 @@ namespace openshot { /// @param channels The number of audio channels needed in this file /// @param channel_layout The 'layout' of audio channels (i.e. mono, stereo, surround, etc...) /// @param bit_rate The audio bit rate used during encoding + /// + /// \note This is an overloaded function. void SetAudioOptions(bool has_audio, std::string codec, int sample_rate, int channels, openshot::ChannelLayout channel_layout, int bit_rate); /// @brief Set audio export options. @@ -294,6 +296,8 @@ namespace openshot { /// @param codec The codec used to encode the audio for this file /// @param sample_rate The number of audio samples needed in this file /// @param bit_rate The audio bit rate used during encoding + /// + /// \note This is an overloaded function. void SetAudioOptions(std::string codec, int sample_rate, int bit_rate); /// @brief Set the cache size @@ -310,6 +314,8 @@ namespace openshot { /// @param interlaced Does this video need to be interlaced? /// @param top_field_first Which frame should be used as the top field? /// @param bit_rate The video bit rate used during encoding + /// + /// \note This is an overloaded function. void SetVideoOptions(bool has_video, std::string codec, openshot::Fraction fps, int width, int height, openshot::Fraction pixel_ratio, bool interlaced, bool top_field_first, int bit_rate); /// @brief Set video export options. @@ -317,11 +323,14 @@ namespace openshot { /// Enables the stream and configures non-interlaced video with a 1:1 pixel aspect ratio. /// /// @param codec The codec used to encode the images in this video - /// @param fps The number of frames per second /// @param width The width in pixels of this video /// @param height The height in pixels of this video + /// @param fps The number of frames per second /// @param bit_rate The video bit rate used during encoding - void SetVideoOptions(std::string codec, openshot::Fraction fps, int width, int height, int bit_rate); + /// + /// \note This is an overloaded function. + /// \warning Observe the argument order, which is consistent with the openshot::Timeline constructor, but differs from the other signature. + void SetVideoOptions(std::string codec, int width, int height, openshot::Fraction fps, int bit_rate); /// @brief Set custom options (some codecs accept additional params). This must be called after the /// PrepareStreams() method, otherwise the streams have not been initialized yet. @@ -337,12 +346,16 @@ namespace openshot { /// @brief Add a frame to the stack waiting to be encoded. /// @param frame The openshot::Frame object to write to this image + /// + /// \note This is an overloaded function. void WriteFrame(std::shared_ptr frame); /// @brief Write a block of frames from a reader /// @param reader A openshot::ReaderBase object which will provide frames to be written /// @param start The starting frame number of the reader /// @param length The number of frames to write + /// + /// \note This is an overloaded function. void WriteFrame(openshot::ReaderBase *reader, int64_t start, int64_t length); /// @brief Write the file trailer (after all frames are written). This is called automatically diff --git a/src/FFmpegWriter.cpp b/src/FFmpegWriter.cpp index c01a09ed..244412f6 100644 --- a/src/FFmpegWriter.cpp +++ b/src/FFmpegWriter.cpp @@ -278,7 +278,7 @@ void FFmpegWriter::SetVideoOptions(bool has_video, std::string codec, Fraction f } // Set video export options (overloaded function) -void FFmpegWriter::SetVideoOptions(std::string codec, Fraction fps, int width, int height, int bit_rate) { +void FFmpegWriter::SetVideoOptions(std::string codec, int width, int height, Fraction fps, int bit_rate) { // Call full signature with some default parameters FFmpegWriter::SetVideoOptions(true, codec, fps, width, height, openshot::Fraction(1, 1), false, true, bit_rate); @@ -324,7 +324,8 @@ void FFmpegWriter::SetAudioOptions(bool has_audio, std::string codec, int sample // Set audio export options (overloaded function) void FFmpegWriter::SetAudioOptions(std::string codec, int sample_rate, int bit_rate) { // Call full signature with some default parameters - FFmpegWriter::SetAudioOptions(true, codec, sample_rate, 2, openshot::LAYOUT_STEREO, bit_rate); + FFmpegWriter::SetAudioOptions(true, codec, sample_rate, 2, + openshot::LAYOUT_STEREO, bit_rate); } diff --git a/tests/FFmpegWriter_Tests.cpp b/tests/FFmpegWriter_Tests.cpp index a4646eb0..cb75a118 100644 --- a/tests/FFmpegWriter_Tests.cpp +++ b/tests/FFmpegWriter_Tests.cpp @@ -97,7 +97,7 @@ TEST(Options_Overloads) // Set options w.SetAudioOptions("aac", 48000, 192000); - w.SetVideoOptions("libx264", Fraction(30,1), 1280, 720, 5000000); + w.SetVideoOptions("libx264", 1280, 720, Fraction(30,1), 5000000); // Open writer w.Open();