Skip to content

Commit

Permalink
Merge pull request #695 from OpenShot/fix-invalid-starting-pts-timestamp
Browse files Browse the repository at this point in the history
Fix invalid starting PTS value (blank 1st frame)
  • Loading branch information
jonoomph committed Jul 3, 2021
2 parents 68f03b5 + 9ca63b3 commit 27c1f9d
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 95 deletions.
1 change: 1 addition & 0 deletions CMakeLists.txt
Expand Up @@ -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")
Expand Down
4 changes: 2 additions & 2 deletions src/FFmpegReader.cpp
Expand Up @@ -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) {
Expand Down
153 changes: 63 additions & 90 deletions src/FFmpegWriter.cpp
Expand Up @@ -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) {

Expand Down Expand Up @@ -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);
Expand All @@ -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);
}
Expand All @@ -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
Expand All @@ -910,60 +900,57 @@ 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;

/* encode the image */
int error_code = 0;
int got_packet = 0;
if (info.has_audio) {
for (;;) {
AVPacket pkt;
av_init_packet(&pkt);
pkt.data = NULL;
pkt.size = 0;
pkt.pts = pkt.dts = audio_timestamp;

/* 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 = audio_timestamp;

// 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)
av_packet_rescale_ts(&pkt, 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);
}
// Increment PTS by duration of packet
audio_timestamp += pkt.duration;

// deallocate memory for packet
AV_FREE_PACKET(&pkt);
}
}

}

Expand Down Expand Up @@ -1027,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);
Expand Down Expand Up @@ -1829,9 +1816,8 @@ 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
// Set the AVFrame's PTS
frame_final->pts = audio_timestamp;

// Init the packet
AVPacket pkt;
Expand All @@ -1840,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;
Expand Down Expand Up @@ -1882,15 +1868,10 @@ 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)
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;
Expand All @@ -1904,6 +1885,9 @@ 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 (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]));
AV_FREE_FRAME(&frame_final);
Expand Down Expand Up @@ -2042,9 +2026,8 @@ bool FFmpegWriter::write_video_packet(std::shared_ptr<Frame> 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);
pkt.pts = write_video_count;
// Set PTS (in frames and scaled to the codec's timebase)
pkt.pts = video_timestamp;

/* write the compressed frame in the media file */
int error_code = av_interleaved_write_frame(oc, &pkt);
Expand All @@ -2065,11 +2048,8 @@ bool FFmpegWriter::write_video_packet(std::shared_ptr<Frame> 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;
frame_final->pts = video_timestamp;
#if USE_HW_ACCEL
if (hw_en_on && hw_en_supported) {
if (!(hw_frame = av_frame_alloc())) {
Expand Down Expand Up @@ -2142,18 +2122,8 @@ bool FFmpegWriter::write_video_packet(std::shared_ptr<Frame> 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);
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 */
Expand All @@ -2176,6 +2146,9 @@ bool FFmpegWriter::write_video_packet(std::shared_ptr<Frame> 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;
}
Expand Down
4 changes: 2 additions & 2 deletions src/FFmpegWriter.h
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/QtPlayer.cpp
Expand Up @@ -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);

Expand Down

0 comments on commit 27c1f9d

Please sign in to comment.