From bdf6e9f854359ca13473ce67ef570a413d0c64a8 Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Fri, 25 Jun 2021 15:57:52 -0500 Subject: [PATCH 1/7] Fix invalid starting PTS value, preventing blank 1st frames on some thumbnailing and video players. --- src/FFmpegWriter.cpp | 109 ++++++++++++++++++++++--------------------- src/QtPlayer.cpp | 2 +- 2 files changed, 56 insertions(+), 55 deletions(-) diff --git a/src/FFmpegWriter.cpp b/src/FFmpegWriter.cpp index 4cfc46333..009d498e4 100644 --- a/src/FFmpegWriter.cpp +++ b/src/FFmpegWriter.cpp @@ -92,6 +92,10 @@ FFmpegWriter::FFmpegWriter(const std::string& path) : info.has_audio = false; info.has_video = false; + // Init timestamps + write_video_count = 0; + write_audio_count = 0; + // Initialize FFMpeg, and register all formats and codecs AV_REGISTER_ALL @@ -910,60 +914,63 @@ void FFmpegWriter::flush_encoders() { } // FLUSH AUDIO ENCODER - if (info.has_audio) - for (;;) { - - // Increment PTS (in samples and scaled to the codec's timebase) - // for some reason, it requires me to multiply channels X 2 - write_audio_count += av_rescale_q(audio_input_position / (audio_codec_ctx->channels * av_get_bytes_per_sample(AV_SAMPLE_FMT_S16)), av_make_q(1, info.sample_rate), audio_codec_ctx->time_base); - - AVPacket pkt; - av_init_packet(&pkt); - pkt.data = NULL; - pkt.size = 0; - pkt.pts = pkt.dts = write_audio_count; + if (info.has_audio) { + for (;;) { + AVPacket pkt; + av_init_packet(&pkt); + pkt.data = NULL; + pkt.size = 0; + pkt.pts = pkt.dts = write_audio_count; - /* encode the image */ - int error_code = 0; - int got_packet = 0; + /* encode the image */ + int error_code = 0; + int got_packet = 0; #if IS_FFMPEG_3_2 - error_code = avcodec_send_frame(audio_codec_ctx, NULL); + error_code = avcodec_send_frame(audio_codec_ctx, NULL); #else - error_code = avcodec_encode_audio2(audio_codec_ctx, &pkt, NULL, &got_packet); + error_code = avcodec_encode_audio2(audio_codec_ctx, &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) { - break; - } + 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. - pkt.pts = pkt.dts = write_audio_count; + // 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. + pkt.pts = pkt.dts = write_audio_count; - // Scale the PTS to the audio stream timebase (which is sometimes different than the codec's timebase) - if (pkt.pts != AV_NOPTS_VALUE) - pkt.pts = av_rescale_q(pkt.pts, audio_codec_ctx->time_base, audio_st->time_base); - if (pkt.dts != AV_NOPTS_VALUE) - pkt.dts = av_rescale_q(pkt.dts, audio_codec_ctx->time_base, audio_st->time_base); - if (pkt.duration > 0) - pkt.duration = av_rescale_q(pkt.duration, audio_codec_ctx->time_base, audio_st->time_base); + // Scale the PTS to the audio stream timebase (which is sometimes different than the codec's timebase) + if (pkt.pts != AV_NOPTS_VALUE) + pkt.pts = av_rescale_q(pkt.pts, audio_codec_ctx->time_base, audio_st->time_base); + if (pkt.dts != AV_NOPTS_VALUE) + pkt.dts = av_rescale_q(pkt.dts, audio_codec_ctx->time_base, audio_st->time_base); + if (pkt.duration > 0) + pkt.duration = av_rescale_q(pkt.duration, audio_codec_ctx->time_base, audio_st->time_base); - // set stream - pkt.stream_index = audio_st->index; - pkt.flags |= AV_PKT_FLAG_KEY; + // set stream + pkt.stream_index = audio_st->index; + pkt.flags |= AV_PKT_FLAG_KEY; - // Write packet - error_code = av_interleaved_write_frame(oc, &pkt); - if (error_code < 0) { - ZmqLogger::Instance()->AppendDebugMethod("FFmpegWriter::flush_encoders ERROR [" + (std::string)av_err2str(error_code) + "]", "error_code", error_code); - } + // Write packet + error_code = av_interleaved_write_frame(oc, &pkt); + if (error_code < 0) { + ZmqLogger::Instance()->AppendDebugMethod( + "FFmpegWriter::flush_encoders ERROR [" + (std::string) av_err2str(error_code) + "]", + "error_code", error_code); + } - // deallocate memory for packet - AV_FREE_PACKET(&pkt); - } + // deallocate memory for packet + AV_FREE_PACKET(&pkt); + } + // Increment PTS (in samples and scaled to the codec's timebase) + // for some reason, it requires me to multiply channels X 2 + write_audio_count += av_rescale_q(audio_input_position / (audio_codec_ctx->channels * av_get_bytes_per_sample(AV_SAMPLE_FMT_S16)), av_make_q(1, info.sample_rate), audio_codec_ctx->time_base); + } } @@ -2042,8 +2049,7 @@ bool FFmpegWriter::write_video_packet(std::shared_ptr frame, AVFrame *fra pkt.data = (uint8_t *) frame_final->data; pkt.size = sizeof(AVPicture); - // Increment PTS (in frames and scaled to the codec's timebase) - write_video_count += av_rescale_q(1, av_make_q(info.fps.den, info.fps.num), video_codec_ctx->time_base); + // Set PTS (in frames and scaled to the codec's timebase) pkt.pts = write_video_count; /* write the compressed frame in the media file */ @@ -2065,9 +2071,6 @@ bool FFmpegWriter::write_video_packet(std::shared_ptr frame, AVFrame *fra pkt.size = 0; pkt.pts = pkt.dts = AV_NOPTS_VALUE; - // Increment PTS (in frames and scaled to the codec's timebase) - write_video_count += av_rescale_q(1, av_make_q(info.fps.den, info.fps.num), video_codec_ctx->time_base); - // Assign the initial AVFrame PTS from the frame counter frame_final->pts = write_video_count; #if USE_HW_ACCEL @@ -2142,11 +2145,6 @@ bool FFmpegWriter::write_video_packet(std::shared_ptr frame, AVFrame *fra /* if zero size, it means the image was buffered */ if (error_code == 0 && got_packet_ptr) { - - // 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. - //pkt.pts = pkt.dts = write_video_count; - // set the timestamp if (pkt.pts != AV_NOPTS_VALUE) pkt.pts = av_rescale_q(pkt.pts, video_codec_ctx->time_base, video_st->time_base); @@ -2176,6 +2174,9 @@ bool FFmpegWriter::write_video_packet(std::shared_ptr frame, AVFrame *fra #endif // USE_HW_ACCEL } + // Increment PTS (in frames and scaled to the codec's timebase) + write_video_count += av_rescale_q(1, av_make_q(info.fps.den, info.fps.num), video_codec_ctx->time_base); + // Success return true; } diff --git a/src/QtPlayer.cpp b/src/QtPlayer.cpp index c43cdf800..f3697f717 100644 --- a/src/QtPlayer.cpp +++ b/src/QtPlayer.cpp @@ -119,7 +119,7 @@ namespace openshot void QtPlayer::Play() { - // Set mode to playing, and speed to normal + // Set mode to playing, and speed to normal mode = PLAYBACK_PLAY; Speed(1); From 64e578241a75528b0e81c3774682016a53592fdf Mon Sep 17 00:00:00 2001 From: Jackson Date: Fri, 25 Jun 2021 18:54:27 -0500 Subject: [PATCH 2/7] Reduced, but didn't eliminate empty time at the beginning --- src/FFmpegWriter.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/FFmpegWriter.cpp b/src/FFmpegWriter.cpp index 009d498e4..32fd346f1 100644 --- a/src/FFmpegWriter.cpp +++ b/src/FFmpegWriter.cpp @@ -1836,10 +1836,6 @@ void FFmpegWriter::write_audio_packets(bool is_final) { audio_encoder_buffer_size, 0); } - // Increment PTS (in samples) - write_audio_count += FFMIN(audio_input_frame_size, audio_input_position); - frame_final->pts = write_audio_count; // Set the AVFrame's PTS - // Init the packet AVPacket pkt; av_init_packet(&pkt); @@ -1911,6 +1907,10 @@ void FFmpegWriter::write_audio_packets(bool is_final) { ZmqLogger::Instance()->AppendDebugMethod("FFmpegWriter::write_audio_packets ERROR [" + (std::string) av_err2str(error_code) + "]", "error_code", error_code); } + // Increment PTS (in samples) + write_audio_count += FFMIN(audio_input_frame_size, audio_input_position); + frame_final->pts = write_audio_count; // Set the AVFrame's PTS + // deallocate AVFrame av_freep(&(frame_final->data[0])); AV_FREE_FRAME(&frame_final); From 1cec1843c628f2f9895609e4dc8154f1fbcde41c Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Sat, 26 Jun 2021 15:48:16 -0500 Subject: [PATCH 3/7] We already init values in constructor, and moving incrementing to later in function. --- src/FFmpegWriter.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/FFmpegWriter.cpp b/src/FFmpegWriter.cpp index 32fd346f1..52d0d8a27 100644 --- a/src/FFmpegWriter.cpp +++ b/src/FFmpegWriter.cpp @@ -92,10 +92,6 @@ FFmpegWriter::FFmpegWriter(const std::string& path) : info.has_audio = false; info.has_video = false; - // Init timestamps - write_video_count = 0; - write_audio_count = 0; - // Initialize FFMpeg, and register all formats and codecs AV_REGISTER_ALL @@ -1836,6 +1832,9 @@ void FFmpegWriter::write_audio_packets(bool is_final) { audio_encoder_buffer_size, 0); } + // Set the AVFrame's PTS + frame_final->pts = write_audio_count; + // Init the packet AVPacket pkt; av_init_packet(&pkt); @@ -1907,9 +1906,8 @@ void FFmpegWriter::write_audio_packets(bool is_final) { ZmqLogger::Instance()->AppendDebugMethod("FFmpegWriter::write_audio_packets ERROR [" + (std::string) av_err2str(error_code) + "]", "error_code", error_code); } - // Increment PTS (in samples) + // Increment PTS write_audio_count += FFMIN(audio_input_frame_size, audio_input_position); - frame_final->pts = write_audio_count; // Set the AVFrame's PTS // deallocate AVFrame av_freep(&(frame_final->data[0])); From 5b524aba2f8dddfaf5a7acc130114e2b9fa91c29 Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Sat, 26 Jun 2021 15:56:58 -0500 Subject: [PATCH 4/7] Experimental conversion of timestamp rescaling to use the av_packet_rescale_ts() method. I'm just not sure the backwards compatibility of this approach with older FFmpeg versions. --- src/FFmpegWriter.cpp | 35 +++++------------------------------ 1 file changed, 5 insertions(+), 30 deletions(-) diff --git a/src/FFmpegWriter.cpp b/src/FFmpegWriter.cpp index 52d0d8a27..845c10998 100644 --- a/src/FFmpegWriter.cpp +++ b/src/FFmpegWriter.cpp @@ -870,12 +870,7 @@ void FFmpegWriter::flush_encoders() { avcodec_flush_buffers(video_codec_ctx); break; } - if (pkt.pts != AV_NOPTS_VALUE) - pkt.pts = av_rescale_q(pkt.pts, video_codec_ctx->time_base, video_st->time_base); - if (pkt.dts != AV_NOPTS_VALUE) - pkt.dts = av_rescale_q(pkt.dts, video_codec_ctx->time_base, video_st->time_base); - if (pkt.duration > 0) - pkt.duration = av_rescale_q(pkt.duration, video_codec_ctx->time_base, video_st->time_base); + av_packet_rescale_ts(&pkt, video_codec_ctx->time_base, video_st->time_base); pkt.stream_index = video_st->index; error_code = av_interleaved_write_frame(oc, &pkt); } @@ -894,12 +889,7 @@ void FFmpegWriter::flush_encoders() { } // set the timestamp - if (pkt.pts != AV_NOPTS_VALUE) - pkt.pts = av_rescale_q(pkt.pts, video_codec_ctx->time_base, video_st->time_base); - if (pkt.dts != AV_NOPTS_VALUE) - pkt.dts = av_rescale_q(pkt.dts, video_codec_ctx->time_base, video_st->time_base); - if (pkt.duration > 0) - pkt.duration = av_rescale_q(pkt.duration, video_codec_ctx->time_base, video_st->time_base); + av_packet_rescale_ts(&pkt, video_codec_ctx->time_base, video_st->time_base); pkt.stream_index = video_st->index; // Write packet @@ -940,12 +930,7 @@ void FFmpegWriter::flush_encoders() { pkt.pts = pkt.dts = write_audio_count; // Scale the PTS to the audio stream timebase (which is sometimes different than the codec's timebase) - if (pkt.pts != AV_NOPTS_VALUE) - pkt.pts = av_rescale_q(pkt.pts, audio_codec_ctx->time_base, audio_st->time_base); - if (pkt.dts != AV_NOPTS_VALUE) - pkt.dts = av_rescale_q(pkt.dts, audio_codec_ctx->time_base, audio_st->time_base); - if (pkt.duration > 0) - pkt.duration = av_rescale_q(pkt.duration, audio_codec_ctx->time_base, audio_st->time_base); + av_packet_rescale_ts(&pkt, audio_codec_ctx->time_base, audio_st->time_base); // set stream pkt.stream_index = audio_st->index; @@ -1887,12 +1872,7 @@ void FFmpegWriter::write_audio_packets(bool is_final) { pkt.pts = pkt.dts = write_audio_count; // Scale the PTS to the audio stream timebase (which is sometimes different than the codec's timebase) - if (pkt.pts != AV_NOPTS_VALUE) - pkt.pts = av_rescale_q(pkt.pts, audio_codec_ctx->time_base, audio_st->time_base); - if (pkt.dts != AV_NOPTS_VALUE) - pkt.dts = av_rescale_q(pkt.dts, audio_codec_ctx->time_base, audio_st->time_base); - if (pkt.duration > 0) - pkt.duration = av_rescale_q(pkt.duration, audio_codec_ctx->time_base, audio_st->time_base); + av_packet_rescale_ts(&pkt, audio_codec_ctx->time_base, audio_st->time_base); // set stream pkt.stream_index = audio_st->index; @@ -2144,12 +2124,7 @@ bool FFmpegWriter::write_video_packet(std::shared_ptr frame, AVFrame *fra /* if zero size, it means the image was buffered */ if (error_code == 0 && got_packet_ptr) { // set the timestamp - if (pkt.pts != AV_NOPTS_VALUE) - pkt.pts = av_rescale_q(pkt.pts, video_codec_ctx->time_base, video_st->time_base); - if (pkt.dts != AV_NOPTS_VALUE) - pkt.dts = av_rescale_q(pkt.dts, video_codec_ctx->time_base, video_st->time_base); - if (pkt.duration > 0) - pkt.duration = av_rescale_q(pkt.duration, video_codec_ctx->time_base, video_st->time_base); + av_packet_rescale_ts(&pkt, video_codec_ctx->time_base, video_st->time_base); pkt.stream_index = video_st->index; /* write the compressed frame in the media file */ From 850140df7d9e2c2c1434d54341efd9e22659f56f Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Sat, 26 Jun 2021 17:24:15 -0500 Subject: [PATCH 5/7] Switch to pkt.duration for incrementing PTS, and rename the 2 timestamp variables to something more sane. --- src/FFmpegWriter.cpp | 41 ++++++++++++++++++++--------------------- src/FFmpegWriter.h | 4 ++-- 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/FFmpegWriter.cpp b/src/FFmpegWriter.cpp index 845c10998..92e9f8fa1 100644 --- a/src/FFmpegWriter.cpp +++ b/src/FFmpegWriter.cpp @@ -84,7 +84,7 @@ 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), - rescaler_position(0), video_codec_ctx(NULL), audio_codec_ctx(NULL), is_writing(false), write_video_count(0), write_audio_count(0), + rescaler_position(0), video_codec_ctx(NULL), audio_codec_ctx(NULL), is_writing(false), video_timestamp(0), audio_timestamp(0), original_sample_rate(0), original_channels(0), avr(NULL), avr_planar(NULL), is_open(false), prepare_streams(false), write_header(false), write_trailer(false), audio_encoder_buffer_size(0), audio_encoder_buffer(NULL) { @@ -847,7 +847,7 @@ void FFmpegWriter::flush_encoders() { for (;;) { // Increment PTS (in frames and scaled to the codec's timebase) - write_video_count += av_rescale_q(1, av_make_q(info.fps.den, info.fps.num), video_codec_ctx->time_base); + video_timestamp += av_rescale_q(1, av_make_q(info.fps.den, info.fps.num), video_codec_ctx->time_base); AVPacket pkt; av_init_packet(&pkt); @@ -906,7 +906,7 @@ void FFmpegWriter::flush_encoders() { av_init_packet(&pkt); pkt.data = NULL; pkt.size = 0; - pkt.pts = pkt.dts = write_audio_count; + pkt.pts = pkt.dts = audio_timestamp; /* encode the image */ int error_code = 0; @@ -927,7 +927,7 @@ void FFmpegWriter::flush_encoders() { // 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. - pkt.pts = pkt.dts = write_audio_count; + pkt.pts = pkt.dts = audio_timestamp; // Scale the PTS to the audio stream timebase (which is sometimes different than the codec's timebase) av_packet_rescale_ts(&pkt, audio_codec_ctx->time_base, audio_st->time_base); @@ -944,13 +944,12 @@ void FFmpegWriter::flush_encoders() { "error_code", error_code); } - // deallocate memory for packet + // Increment PTS by duration of packet + audio_timestamp += pkt.duration; + + // deallocate memory for packet AV_FREE_PACKET(&pkt); } - - // Increment PTS (in samples and scaled to the codec's timebase) - // for some reason, it requires me to multiply channels X 2 - write_audio_count += av_rescale_q(audio_input_position / (audio_codec_ctx->channels * av_get_bytes_per_sample(AV_SAMPLE_FMT_S16)), av_make_q(1, info.sample_rate), audio_codec_ctx->time_base); } } @@ -1015,8 +1014,8 @@ void FFmpegWriter::Close() { } // Reset frame counters - write_video_count = 0; - write_audio_count = 0; + video_timestamp = 0; + audio_timestamp = 0; // Free the context which frees the streams too avformat_free_context(oc); @@ -1818,7 +1817,7 @@ void FFmpegWriter::write_audio_packets(bool is_final) { } // Set the AVFrame's PTS - frame_final->pts = write_audio_count; + frame_final->pts = audio_timestamp; // Init the packet AVPacket pkt; @@ -1827,7 +1826,7 @@ void FFmpegWriter::write_audio_packets(bool is_final) { pkt.size = audio_encoder_buffer_size; // Set the packet's PTS prior to encoding - pkt.pts = pkt.dts = write_audio_count; + pkt.pts = pkt.dts = audio_timestamp; /* encode the audio samples */ int got_packet_ptr = 0; @@ -1869,7 +1868,7 @@ void FFmpegWriter::write_audio_packets(bool is_final) { // 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. - pkt.pts = pkt.dts = write_audio_count; + pkt.pts = pkt.dts = audio_timestamp; // Scale the PTS to the audio stream timebase (which is sometimes different than the codec's timebase) av_packet_rescale_ts(&pkt, audio_codec_ctx->time_base, audio_st->time_base); @@ -1886,8 +1885,8 @@ void FFmpegWriter::write_audio_packets(bool is_final) { ZmqLogger::Instance()->AppendDebugMethod("FFmpegWriter::write_audio_packets ERROR [" + (std::string) av_err2str(error_code) + "]", "error_code", error_code); } - // Increment PTS - write_audio_count += FFMIN(audio_input_frame_size, audio_input_position); + // Increment PTS by duration of packet + audio_timestamp += pkt.duration; // deallocate AVFrame av_freep(&(frame_final->data[0])); @@ -2028,7 +2027,7 @@ bool FFmpegWriter::write_video_packet(std::shared_ptr frame, AVFrame *fra pkt.size = sizeof(AVPicture); // Set PTS (in frames and scaled to the codec's timebase) - pkt.pts = write_video_count; + pkt.pts = video_timestamp; /* write the compressed frame in the media file */ int error_code = av_interleaved_write_frame(oc, &pkt); @@ -2050,7 +2049,7 @@ bool FFmpegWriter::write_video_packet(std::shared_ptr frame, AVFrame *fra pkt.pts = pkt.dts = AV_NOPTS_VALUE; // Assign the initial AVFrame PTS from the frame counter - frame_final->pts = write_video_count; + frame_final->pts = video_timestamp; #if USE_HW_ACCEL if (hw_en_on && hw_en_supported) { if (!(hw_frame = av_frame_alloc())) { @@ -2135,6 +2134,9 @@ bool FFmpegWriter::write_video_packet(std::shared_ptr frame, AVFrame *fra } } + // Increment PTS (in frames and scaled to the codec's timebase) + video_timestamp += pkt.duration; + // Deallocate packet AV_FREE_PACKET(&pkt); #if USE_HW_ACCEL @@ -2147,9 +2149,6 @@ bool FFmpegWriter::write_video_packet(std::shared_ptr frame, AVFrame *fra #endif // USE_HW_ACCEL } - // Increment PTS (in frames and scaled to the codec's timebase) - write_video_count += av_rescale_q(1, av_make_q(info.fps.den, info.fps.num), video_codec_ctx->time_base); - // Success return true; } diff --git a/src/FFmpegWriter.h b/src/FFmpegWriter.h index 44f7cb355..79564b5cf 100644 --- a/src/FFmpegWriter.h +++ b/src/FFmpegWriter.h @@ -151,8 +151,8 @@ namespace openshot { int cache_size; bool is_writing; bool is_open; - int64_t write_video_count; - int64_t write_audio_count; + int64_t video_timestamp; + int64_t audio_timestamp; bool prepare_streams; bool write_header; From 0f2f2e1514597e385e28b8fee9bee8845b5154b9 Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" Date: Sun, 27 Jun 2021 15:35:12 -0400 Subject: [PATCH 6/7] Unit tests: --output-on-failure --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index a3535ec98..7fd0cb151 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -200,6 +200,7 @@ if(BUILD_TESTING) if(VERBOSE_TESTS) list(APPEND CTEST_OPTIONS "-VV") endif() + list(APPEND CTEST_OPTIONS "--output-on-failure") add_subdirectory(tests) endif() add_feature_info("Unit tests" ${BUILD_TESTING} "Compile unit tests for library functions") From 9ca63b321a91da727c62b4d93b9948ed1065aca7 Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Sat, 3 Jul 2021 17:01:55 -0500 Subject: [PATCH 7/7] Reverting video_timestamp increment logic which used pkt.duration. In some codecs (such as vp8), this approach breaks due to differences in the timebase vs the framerate. For example, if the timebase is an inverse of the FPS, everything works. But if the timebase is not, for example 1/1000000, this approach breaks. --- src/FFmpegReader.cpp | 4 ++-- src/FFmpegWriter.cpp | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/FFmpegReader.cpp b/src/FFmpegReader.cpp index b97d7345b..98d39ce1f 100644 --- a/src/FFmpegReader.cpp +++ b/src/FFmpegReader.cpp @@ -768,12 +768,12 @@ void FFmpegReader::UpdateVideoInfo() { // Check for valid duration (if found) if (info.duration <= 0.0f && pFormatCtx->duration >= 0) // Use the format's duration - info.duration = pFormatCtx->duration / AV_TIME_BASE; + info.duration = float(pFormatCtx->duration) / AV_TIME_BASE; // Calculate duration from filesize and bitrate (if any) if (info.duration <= 0.0f && info.video_bit_rate > 0 && info.file_size > 0) // Estimate from bitrate, total bytes, and framerate - info.duration = (info.file_size / info.video_bit_rate); + info.duration = float(info.file_size) / info.video_bit_rate; // No duration found in stream of file if (info.duration <= 0.0f) { diff --git a/src/FFmpegWriter.cpp b/src/FFmpegWriter.cpp index 92e9f8fa1..f3b12800a 100644 --- a/src/FFmpegWriter.cpp +++ b/src/FFmpegWriter.cpp @@ -947,7 +947,7 @@ void FFmpegWriter::flush_encoders() { // Increment PTS by duration of packet audio_timestamp += pkt.duration; - // deallocate memory for packet + // deallocate memory for packet AV_FREE_PACKET(&pkt); } } @@ -1885,8 +1885,8 @@ void FFmpegWriter::write_audio_packets(bool is_final) { ZmqLogger::Instance()->AppendDebugMethod("FFmpegWriter::write_audio_packets ERROR [" + (std::string) av_err2str(error_code) + "]", "error_code", error_code); } - // Increment PTS by duration of packet - audio_timestamp += pkt.duration; + // Increment PTS (no pkt.duration, so calculate with maths) + audio_timestamp += FFMIN(audio_input_frame_size, audio_input_position); // deallocate AVFrame av_freep(&(frame_final->data[0])); @@ -2134,9 +2134,6 @@ bool FFmpegWriter::write_video_packet(std::shared_ptr frame, AVFrame *fra } } - // Increment PTS (in frames and scaled to the codec's timebase) - video_timestamp += pkt.duration; - // Deallocate packet AV_FREE_PACKET(&pkt); #if USE_HW_ACCEL @@ -2149,6 +2146,9 @@ bool FFmpegWriter::write_video_packet(std::shared_ptr frame, AVFrame *fra #endif // USE_HW_ACCEL } + // Increment PTS (in frames and scaled to the codec's timebase) + video_timestamp += av_rescale_q(1, av_make_q(info.fps.den, info.fps.num), video_codec_ctx->time_base); + // Success return true; }