From 62d7fb54596219b993cf432d0a437de6e1fdb7a6 Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Thu, 4 Jun 2020 18:52:19 -0500 Subject: [PATCH 1/4] Changing some libopenshot code based on Codacy feedback. Small refactoring, scope limiing. --- include/FFmpegWriter.h | 2 +- src/FFmpegWriter.cpp | 42 ++++++++++++------------------------------ 2 files changed, 13 insertions(+), 31 deletions(-) diff --git a/include/FFmpegWriter.h b/include/FFmpegWriter.h index 37fa22dce..b3bec7add 100644 --- a/include/FFmpegWriter.h +++ b/include/FFmpegWriter.h @@ -253,7 +253,7 @@ namespace openshot { /// @brief Constructor for FFmpegWriter. Throws one of the following exceptions. /// @param path The file path of the video file you want to open and read - FFmpegWriter(std::string path); + FFmpegWriter(const std::string path); /// Close the writer void Close(); diff --git a/src/FFmpegWriter.cpp b/src/FFmpegWriter.cpp index e6a7e5ef3..5b5c54107 100644 --- a/src/FFmpegWriter.cpp +++ b/src/FFmpegWriter.cpp @@ -83,7 +83,7 @@ static int set_hwframe_ctx(AVCodecContext *ctx, AVBufferRef *hw_device_ctx, int6 } #endif // HAVE_HW_ACCEL -FFmpegWriter::FFmpegWriter(std::string path) : +FFmpegWriter::FFmpegWriter(const std::string path) : path(path), fmt(NULL), oc(NULL), audio_st(NULL), video_st(NULL), samples(NULL), audio_outbuf(NULL), audio_outbuf_size(0), audio_input_frame_size(0), audio_input_position(0), initial_audio_input_frame_size(0), img_convert_ctx(NULL), cache_size(8), num_of_rescalers(32), @@ -864,9 +864,6 @@ void FFmpegWriter::flush_encoders() { return; #endif - int error_code = 0; - int stop_encoding = 1; - // FLUSH VIDEO ENCODER if (info.has_video) for (;;) { @@ -940,13 +937,9 @@ void FFmpegWriter::flush_encoders() { ZmqLogger::Instance()->AppendDebugMethod("FFmpegWriter::flush_encoders ERROR [" + (std::string) av_err2str(error_code) + "]", "error_code", error_code); } if (!got_packet) { - stop_encoding = 1; break; } - // Override PTS (in frames and scaled to the codec's timebase) - //pkt.pts = write_video_count; - // set the timestamp if (pkt.pts != AV_NOPTS_VALUE) pkt.pts = av_rescale_q(pkt.pts, video_codec->time_base, video_st->time_base); @@ -961,10 +954,6 @@ void FFmpegWriter::flush_encoders() { if (error_code < 0) { ZmqLogger::Instance()->AppendDebugMethod("FFmpegWriter::flush_encoders ERROR [" + (std::string)av_err2str(error_code) + "]", "error_code", error_code); } - - // Deallocate memory (if needed) - if (video_outbuf) - av_freep(&video_outbuf); } // FLUSH AUDIO ENCODER @@ -986,20 +975,15 @@ void FFmpegWriter::flush_encoders() { pkt.pts = pkt.dts = write_audio_count; /* encode the image */ - int got_packet = 0; + int error_code = 0; #if IS_FFMPEG_3_2 - avcodec_send_frame(audio_codec, NULL); - got_packet = 0; + error_code = avcodec_send_frame(audio_codec, NULL); #else error_code = avcodec_encode_audio2(audio_codec, &pkt, NULL, &got_packet); #endif if (error_code < 0) { ZmqLogger::Instance()->AppendDebugMethod("FFmpegWriter::flush_encoders ERROR [" + (std::string)av_err2str(error_code) + "]", "error_code", error_code); } - if (!got_packet) { - stop_encoding = 1; - break; - } // Since the PTS can change during encoding, set the value again. This seems like a huge hack, // but it fixes lots of PTS related issues when I do this. @@ -1583,29 +1567,27 @@ void FFmpegWriter::write_audio_packets(bool is_final) { channels_in_frame = frame->GetAudioChannelsCount(); channel_layout_in_frame = frame->ChannelsLayout(); - // Get audio sample array float *frame_samples_float = NULL; // Get samples interleaved together (c1 c2 c1 c2 c1 c2) frame_samples_float = frame->GetInterleavedAudioSamples(sample_rate_in_frame, NULL, &samples_in_frame); - // Calculate total samples total_frame_samples = samples_in_frame * channels_in_frame; // Translate audio sample values back to 16 bit integers with saturation - float valF; - int16_t conv; const int16_t max16 = 32767; const int16_t min16 = -32768; for (int s = 0; s < total_frame_samples; s++, frame_position++) { - valF = frame_samples_float[s] * (1 << 15); - if (valF > max16) + float valF = frame_samples_float[s] * (1 << 15); + int16_t conv; + if (valF > max16) { conv = max16; - else if (valF < min16) + } else if (valF < min16) { conv = min16; - else + } else { conv = int(valF + 32768.5) - 32768; // +0.5 is for rounding + } // Copy into buffer all_queued_samples[frame_position] = conv; @@ -1731,10 +1713,11 @@ void FFmpegWriter::write_audio_packets(bool is_final) { // Determine how many samples we need int diff = 0; - if (remaining_frame_samples >= remaining_packet_samples) + if (remaining_frame_samples >= remaining_packet_samples) { diff = remaining_packet_samples; - else if (remaining_frame_samples < remaining_packet_samples) + } else { diff = remaining_frame_samples; + } // Copy frame samples into the packet samples array if (!is_final) @@ -1746,7 +1729,6 @@ void FFmpegWriter::write_audio_packets(bool is_final) { audio_input_position += diff; samples_position += diff * (av_get_bytes_per_sample(output_sample_fmt) / av_get_bytes_per_sample(AV_SAMPLE_FMT_S16)); remaining_frame_samples -= diff; - remaining_packet_samples -= diff; // Do we have enough samples to proceed? if (audio_input_position < (audio_input_frame_size * info.channels) && !is_final) From 86b83abf5d74f65c67ed6e8c3b5869110e66b43e Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Fri, 5 Jun 2020 15:41:23 -0500 Subject: [PATCH 2/4] Fixing undefined got_packet int --- src/FFmpegWriter.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/FFmpegWriter.cpp b/src/FFmpegWriter.cpp index 5b5c54107..7eb24156c 100644 --- a/src/FFmpegWriter.cpp +++ b/src/FFmpegWriter.cpp @@ -976,6 +976,7 @@ void FFmpegWriter::flush_encoders() { /* encode the image */ int error_code = 0; + int got_packet = 0; #if IS_FFMPEG_3_2 error_code = avcodec_send_frame(audio_codec, NULL); #else From 51fe8549e6364e8d685dd4e5c3a1f5de2fda50d3 Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Fri, 5 Jun 2020 17:53:21 -0500 Subject: [PATCH 3/4] Fix regression caused by Codacy tweaks --- src/FFmpegWriter.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/FFmpegWriter.cpp b/src/FFmpegWriter.cpp index 7eb24156c..5a21c6ba4 100644 --- a/src/FFmpegWriter.cpp +++ b/src/FFmpegWriter.cpp @@ -985,6 +985,9 @@ void FFmpegWriter::flush_encoders() { if (error_code < 0) { ZmqLogger::Instance()->AppendDebugMethod("FFmpegWriter::flush_encoders ERROR [" + (std::string)av_err2str(error_code) + "]", "error_code", error_code); } + if (!got_packet) { + break; + } // Since the PTS can change during encoding, set the value again. This seems like a huge hack, // but it fixes lots of PTS related issues when I do this. From 7b94ac838b76f1211ed3c14183fdd15deb354b04 Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Fri, 5 Jun 2020 18:07:25 -0500 Subject: [PATCH 4/4] Removing bad suggestion. In theory, we could change this signature to take a const reference to a string, but for now, I'm reverting it. --- include/FFmpegWriter.h | 2 +- src/FFmpegWriter.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/FFmpegWriter.h b/include/FFmpegWriter.h index b3bec7add..37fa22dce 100644 --- a/include/FFmpegWriter.h +++ b/include/FFmpegWriter.h @@ -253,7 +253,7 @@ namespace openshot { /// @brief Constructor for FFmpegWriter. Throws one of the following exceptions. /// @param path The file path of the video file you want to open and read - FFmpegWriter(const std::string path); + FFmpegWriter(std::string path); /// Close the writer void Close(); diff --git a/src/FFmpegWriter.cpp b/src/FFmpegWriter.cpp index 5a21c6ba4..e3dedb287 100644 --- a/src/FFmpegWriter.cpp +++ b/src/FFmpegWriter.cpp @@ -83,7 +83,7 @@ static int set_hwframe_ctx(AVCodecContext *ctx, AVBufferRef *hw_device_ctx, int6 } #endif // HAVE_HW_ACCEL -FFmpegWriter::FFmpegWriter(const std::string path) : +FFmpegWriter::FFmpegWriter(std::string path) : path(path), fmt(NULL), oc(NULL), audio_st(NULL), video_st(NULL), samples(NULL), audio_outbuf(NULL), audio_outbuf_size(0), audio_input_frame_size(0), audio_input_position(0), initial_audio_input_frame_size(0), img_convert_ctx(NULL), cache_size(8), num_of_rescalers(32),