Skip to content

Commit

Permalink
Reorder arguments in setVideoOptions overload
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
ferdnyc committed Feb 14, 2020
1 parent bad0a34 commit 7867cf0
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 5 deletions.
17 changes: 15 additions & 2 deletions include/FFmpegWriter.h
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -310,18 +314,23 @@ 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.
///
/// 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.
Expand All @@ -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<openshot::Frame> 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
Expand Down
5 changes: 3 additions & 2 deletions src/FFmpegWriter.cpp
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}


Expand Down
2 changes: 1 addition & 1 deletion tests/FFmpegWriter_Tests.cpp
Expand Up @@ -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();
Expand Down

0 comments on commit 7867cf0

Please sign in to comment.