From 0a063b84b73ac41d2154f627d2dbed035b3739ff Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" Date: Fri, 31 Jan 2020 03:53:53 -0500 Subject: [PATCH 1/4] FFmpegWriter: Overload Set___Options() methods Add overloaded forms of SetVideoOptions() and SetAudioOptions() that apply some sensible defaults to rarely-changed parameters. --- include/FFmpegWriter.h | 21 +++++++++++++++++++++ src/FFmpegWriter.cpp | 16 ++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/include/FFmpegWriter.h b/include/FFmpegWriter.h index dc3a2cf7..654e72cc 100644 --- a/include/FFmpegWriter.h +++ b/include/FFmpegWriter.h @@ -287,6 +287,15 @@ namespace openshot { /// @param bit_rate The audio bit rate used during encoding 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. + /// + /// Enables the stream and configures a default 2-channel stereo layout. + /// + /// @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 + void SetAudioOptions(std::string codec, int sample_rate, int bit_rate); + /// @brief Set the cache size /// @param new_size The number of frames to queue before writing to the file void SetCacheSize(int new_size) { cache_size = new_size; }; @@ -303,8 +312,20 @@ namespace openshot { /// @param bit_rate The video bit rate used during encoding 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. + /// + /// 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 bit_rate The video bit rate used during encoding + void SetVideoOptions(std::string codec, openshot::Fraction fps, int width, int height, 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. + /// /// @param stream The stream (openshot::StreamType) this option should apply to /// @param name The name of the option you want to set (i.e. qmin, qmax, etc...) /// @param value The new value of this option diff --git a/src/FFmpegWriter.cpp b/src/FFmpegWriter.cpp index 245bd9bd..c01a09ed 100644 --- a/src/FFmpegWriter.cpp +++ b/src/FFmpegWriter.cpp @@ -277,6 +277,14 @@ void FFmpegWriter::SetVideoOptions(bool has_video, std::string codec, Fraction f info.has_video = has_video; } +// Set video export options (overloaded function) +void FFmpegWriter::SetVideoOptions(std::string codec, Fraction fps, int width, int height, 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); +} + + // Set audio export options void FFmpegWriter::SetAudioOptions(bool has_audio, std::string codec, int sample_rate, int channels, ChannelLayout channel_layout, int bit_rate) { // Set audio options @@ -312,6 +320,14 @@ void FFmpegWriter::SetAudioOptions(bool has_audio, std::string codec, int sample info.has_audio = has_audio; } + +// 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); +} + + // Set custom options (some codecs accept additional params) void FFmpegWriter::SetOption(StreamType stream, std::string name, std::string value) { // Declare codec context From bad0a34a654f15f7509ac1358663bb5a3fa87014 Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" Date: Fri, 31 Jan 2020 04:35:43 -0500 Subject: [PATCH 2/4] Add unit test for overloads --- tests/FFmpegWriter_Tests.cpp | 46 +++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/tests/FFmpegWriter_Tests.cpp b/tests/FFmpegWriter_Tests.cpp index 21940b1b..a4646eb0 100644 --- a/tests/FFmpegWriter_Tests.cpp +++ b/tests/FFmpegWriter_Tests.cpp @@ -36,7 +36,8 @@ using namespace std; using namespace openshot; -TEST(FFmpegWriter_Test_Webm) +SUITE(FFMpegWriter) { +TEST(Webm) { // Reader stringstream path; @@ -82,3 +83,46 @@ TEST(FFmpegWriter_Test_Webm) CHECK_CLOSE(23, (int)pixels[pixel_index + 2], 5); CHECK_CLOSE(255, (int)pixels[pixel_index + 3], 5); } + +TEST(Options_Overloads) +{ + // Reader + stringstream path; + path << TEST_MEDIA_PATH << "sintel_trailer-720p.mp4"; + FFmpegReader r(path.str()); + r.Open(); + + /* WRITER ---------------- */ + FFmpegWriter w("output1.mp4"); + + // Set options + w.SetAudioOptions("aac", 48000, 192000); + w.SetVideoOptions("libx264", Fraction(30,1), 1280, 720, 5000000); + + // Open writer + w.Open(); + + // Write some frames + w.WriteFrame(&r, 24, 50); + + // Close writer & reader + w.Close(); + r.Close(); + + FFmpegReader r1("output1.mp4"); + r1.Open(); + + // Verify implied settings + CHECK_EQUAL(true, r1.info.has_audio); + CHECK_EQUAL(true, r1.info.has_video); + + CHECK_EQUAL(2, r1.GetFrame(1)->GetAudioChannelsCount()); + CHECK_EQUAL(LAYOUT_STEREO, r1.info.channel_layout); + + CHECK_EQUAL(1, r1.info.pixel_ratio.num); + CHECK_EQUAL(1, r1.info.pixel_ratio.den); + CHECK_EQUAL(false, r1.info.interlaced_frame); + CHECK_EQUAL(true, r1.info.top_field_first); +} + +} // SUITE() From 7867cf01b8da91af65a3f8b4234107acffa06e07 Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" Date: Fri, 14 Feb 2020 11:42:31 -0500 Subject: [PATCH 3/4] 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(); From 895c2f0e2466008f845ddc27b32fb83d1c662339 Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" Date: Fri, 14 Feb 2020 12:07:02 -0500 Subject: [PATCH 4/4] FFmpegReader/Writer: Reformat example code - Reduced crazy-long line lengths by moving trailing comments to previous line - Added more openshot:: prefixing, which causes Doxygen to link to the referenced object's documentation. (It doesn't always pick up cross-class links, without the prefix.) --- include/FFmpegReader.h | 4 ++-- include/FFmpegWriter.h | 26 +++++++++++++++++--------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/include/FFmpegReader.h b/include/FFmpegReader.h index 9faa86a3..5f1444f9 100644 --- a/include/FFmpegReader.h +++ b/include/FFmpegReader.h @@ -76,11 +76,11 @@ namespace openshot { * * @code * // Create a reader for a video - * FFmpegReader r("MyAwesomeVideo.webm"); + * openshot::FFmpegReader r("MyAwesomeVideo.webm"); * r.Open(); // Open the reader * * // Get frame number 1 from the video - * std::shared_ptr f = r.GetFrame(1); + * std::shared_ptr f = r.GetFrame(1); * * // Now that we have an openshot::Frame object, lets have some fun! * f->Display(); // Display the frame on the screen diff --git a/include/FFmpegWriter.h b/include/FFmpegWriter.h index b25778e5..1dfb21a9 100644 --- a/include/FFmpegWriter.h +++ b/include/FFmpegWriter.h @@ -75,15 +75,19 @@ namespace openshot { * @code SIMPLE EXAMPLE * * // Create a reader for a video - * FFmpegReader r("MyAwesomeVideo.webm"); - * r.Open(); // Open thetarget_ reader + * openshot::FFmpegReader r("MyAwesomeVideo.webm"); + * r.Open(); // Open the target reader * * // Create a writer (which will create a WebM video) - * FFmpegWriter w("/home/jonathan/NewVideo.webm"); + * openshot::FFmpegWriter w("/home/jonathan/NewVideo.webm"); * * // Set options - * w.SetAudioOptions(true, "libvorbis", 44100, 2, ChannelLayout::LAYOUT_STEREO, 128000); // Sample Rate: 44100, Channels: 2, Bitrate: 128000 - * w.SetVideoOptions(true, "libvpx", openshot::Fraction(24,1), 720, 480, openshot::Fraction(1,1), false, false, 300000); // FPS: 24, Size: 720x480, Pixel Ratio: 1/1, Bitrate: 300000 + * + * // Sample Rate: 44100, Channels: 2, Bitrate: 128000 + * w.SetAudioOptions(true, "libvorbis", 44100, 2, openshot::ChannelLayout::LAYOUT_STEREO, 128000); + * + * // FPS: 24, Size: 720x480, Pixel Ratio: 1/1, Bitrate: 300000 + * w.SetVideoOptions(true, "libvpx", openshot::Fraction(24,1), 720, 480, openshot::Fraction(1,1), false, false, 300000); * * // Open the writer * w.Open(); @@ -102,15 +106,19 @@ namespace openshot { * @code ADVANCED WRITER EXAMPLE * * // Create a reader for a video - * FFmpegReader r("MyAwesomeVideo.webm"); + * openshot::FFmpegReader r("MyAwesomeVideo.webm"); * r.Open(); // Open the reader * * // Create a writer (which will create a WebM video) - * FFmpegWriter w("/home/jonathan/NewVideo.webm"); + * openshot::FFmpegWriter w("/home/jonathan/NewVideo.webm"); * * // Set options - * w.SetAudioOptions(true, "libvorbis", 44100, 2, ChannelLayout::LAYOUT_STEREO, 128000); // Sample Rate: 44100, Channels: 2, Bitrate: 128000 - * w.SetVideoOptions(true, "libvpx", openshot::Fraction(24,1), 720, 480, openshot::Fraction(1,1), false, false, 300000); // FPS: 24, Size: 720x480, Pixel Ratio: 1/1, Bitrate: 300000 + * + * // Sample Rate: 44100, Channels: 2, Bitrate: 128000 + * w.SetAudioOptions(true, "libvorbis", 44100, 2, openshot::ChannelLayout::LAYOUT_STEREO, 128000); + * + * // FPS: 24, Size: 720x480, Pixel Ratio: 1/1, Bitrate: 300000 + * w.SetVideoOptions(true, "libvpx", openshot::Fraction(24,1), 720, 480, openshot::Fraction(1,1), false, false, 300000); * * // Prepare Streams (Optional method that must be called before any SetOption calls) * w.PrepareStreams();