From acbdf1854dd4db3732cdb04852c00bfc4beeaef6 Mon Sep 17 00:00:00 2001 From: Doug Nazar Date: Thu, 28 Jul 2022 00:37:02 -0400 Subject: [PATCH] Add guard to handle resetting AVPacket after use. Also handle allocation failures. --- src/zm_ffmpeg.h | 34 ++++++++++++++++++++++++++++++++++ src/zm_ffmpeg_camera.cpp | 3 ++- src/zm_ffmpeg_input.cpp | 10 +++++++--- src/zm_ffmpeg_output.cpp | 13 +++++++------ src/zm_videostore.cpp | 18 +++++++++++++----- 5 files changed, 63 insertions(+), 15 deletions(-) diff --git a/src/zm_ffmpeg.h b/src/zm_ffmpeg.h index 94e0c895e..71c28c28b 100644 --- a/src/zm_ffmpeg.h +++ b/src/zm_ffmpeg.h @@ -250,4 +250,38 @@ struct zm_free_av_packet using av_packet_ptr = std::unique_ptr; +struct av_packet_guard +{ + av_packet_guard() : packet{nullptr} + { + } + explicit av_packet_guard(const av_packet_ptr& p) : packet{p.get()} + { + } + explicit av_packet_guard(AVPacket *p) : packet{p} + { + } + ~av_packet_guard() + { + if (packet) + av_packet_unref(packet); + } + + void acquire(const av_packet_ptr& p) + { + packet = p.get(); + } + void acquire(AVPacket *p) + { + packet = p; + } + void release() + { + packet = nullptr; + } + +private: + AVPacket *packet; +}; + #endif // ZM_FFMPEG_H diff --git a/src/zm_ffmpeg_camera.cpp b/src/zm_ffmpeg_camera.cpp index 6e28e1449..b8e284951 100644 --- a/src/zm_ffmpeg_camera.cpp +++ b/src/zm_ffmpeg_camera.cpp @@ -221,6 +221,8 @@ int FfmpegCamera::Capture(std::shared_ptr &zm_packet) { return -1; } + av_packet_guard pkt_guard{packet}; + AVStream *stream = formatContextPtr->streams[packet->stream_index]; ZM_DUMP_STREAM_PACKET(stream, packet, "ffmpeg_camera in"); @@ -243,7 +245,6 @@ int FfmpegCamera::Capture(std::shared_ptr &zm_packet) { mLastAudioPTS = packet->pts - mFirstAudioPTS; } } - zm_av_packet_unref(packet.get()); return 1; } // FfmpegCamera::Capture diff --git a/src/zm_ffmpeg_input.cpp b/src/zm_ffmpeg_input.cpp index 437fe605f..ebde2fe21 100644 --- a/src/zm_ffmpeg_input.cpp +++ b/src/zm_ffmpeg_input.cpp @@ -139,6 +139,11 @@ AVFrame *FFmpeg_Input::get_frame(int stream_id) { int frameComplete = false; av_packet_ptr packet{av_packet_alloc()}; + if (!packet) { + Error("Unable to allocate packet."); + return nullptr; + } + while ( !frameComplete ) { int ret = av_read_frame(input_format_context, packet.get()); if ( ret < 0 ) { @@ -157,9 +162,10 @@ AVFrame *FFmpeg_Input::get_frame(int stream_id) { } ZM_DUMP_STREAM_PACKET(input_format_context->streams[packet->stream_index], packet, "Received packet"); + av_packet_guard pkt_guard{packet}; + if ( (stream_id >= 0) && (packet->stream_index != stream_id) ) { Debug(1,"Packet is not for our stream (%d)", packet->stream_index ); - zm_av_packet_unref(packet.get()); continue; } @@ -175,7 +181,6 @@ AVFrame *FFmpeg_Input::get_frame(int stream_id) { if ( ret < 0 ) { Error("Unable to decode frame at frame %d: %d %s, continuing", streams[packet->stream_index].frame_count, ret, av_make_error_string(ret).c_str()); - zm_av_packet_unref(packet.get()); av_frame_free(&frame); continue; } else { @@ -194,7 +199,6 @@ AVFrame *FFmpeg_Input::get_frame(int stream_id) { input_format_context->streams[stream_id]->time_base ); - zm_av_packet_unref(packet.get()); } // end while !frameComplete return frame; } // end AVFrame *FFmpeg_Input::get_frame diff --git a/src/zm_ffmpeg_output.cpp b/src/zm_ffmpeg_output.cpp index 548822add..dc9b31242 100644 --- a/src/zm_ffmpeg_output.cpp +++ b/src/zm_ffmpeg_output.cpp @@ -88,6 +88,11 @@ AVFrame *FFmpeg_Output::get_frame( int stream_id ) { AVFrame *frame = zm_av_frame_alloc(); char errbuf[AV_ERROR_MAX_STRING_SIZE]; + if (!packet) { + Error("Unable to allocate packet."); + return nullptr; + } + while ( !frameComplete ) { int ret = av_read_frame( input_format_context, packet.get() ); if ( ret < 0 ) { @@ -105,6 +110,8 @@ AVFrame *FFmpeg_Output::get_frame( int stream_id ) { return NULL; } + av_packet_guard pkt_guard{packet}; + if ( (stream_id < 0 ) || ( packet->stream_index == stream_id ) ) { Debug(1,"Packet is for our stream (%d)", packet->stream_index ); @@ -114,7 +121,6 @@ AVFrame *FFmpeg_Output::get_frame( int stream_id ) { if ( ret < 0 ) { av_strerror( ret, errbuf, AV_ERROR_MAX_STRING_SIZE ); Error( "Unable to send packet at frame %d: %s, continuing", streams[packet->stream_index].frame_count, errbuf ); - zm_av_packet_unref( packet.get() ); continue; } else { Debug(1, "Success getting a packet"); @@ -126,14 +132,12 @@ AVFrame *FFmpeg_Output::get_frame( int stream_id ) { if ( ret < 0 ) { av_strerror( ret, errbuf, AV_ERROR_MAX_STRING_SIZE ); Error( "Unable to receive frame %d: %s, continuing", streams[packet->stream_index].frame_count, errbuf ); - zm_av_packet_unref( packet.get() ); continue; } ret = av_hwframe_transfer_data(frame, hwFrame, 0); if (ret < 0) { av_strerror( ret, errbuf, AV_ERROR_MAX_STRING_SIZE ); Error( "Unable to transfer frame at frame %d: %s, continuing", streams[packet->stream_index].frame_count, errbuf ); - zm_av_packet_unref( packet.get() ); continue; } } else { @@ -143,7 +147,6 @@ AVFrame *FFmpeg_Output::get_frame( int stream_id ) { if ( ret < 0 ) { av_strerror( ret, errbuf, AV_ERROR_MAX_STRING_SIZE ); Error( "Unable to send packet at frame %d: %s, continuing", streams[packet->stream_index].frame_count, errbuf ); - zm_av_packet_unref( packet.get() ); continue; } @@ -154,8 +157,6 @@ AVFrame *FFmpeg_Output::get_frame( int stream_id ) { frameComplete = 1; } // end if it's the right stream - zm_av_packet_unref( packet.get() ); - } // end while ! frameComplete return frame; diff --git a/src/zm_videostore.cpp b/src/zm_videostore.cpp index c86400391..05731cdf3 100644 --- a/src/zm_videostore.cpp +++ b/src/zm_videostore.cpp @@ -577,15 +577,20 @@ void VideoStore::flush_codecs() { // whatever we get. Failures are not fatal. av_packet_ptr pkt{av_packet_alloc()}; + if (!pkt) { + Error("Unable to allocate packet."); + return; + } + // I got crashes if the codec didn't do DELAY, so let's test for it. if (video_out_ctx && video_out_ctx->codec && (video_out_ctx->codec->capabilities & AV_CODEC_CAP_DELAY)) { // Put encoder into flushing mode while ((zm_send_frame_receive_packet(video_out_ctx, nullptr, *pkt)) > 0) { + av_packet_guard pkt_guard{pkt}; av_packet_rescale_ts(pkt.get(), video_out_ctx->time_base, video_out_stream->time_base); write_packet(pkt.get(), video_out_stream); - zm_av_packet_unref(pkt.get()); } // while have buffered frames Debug(1, "Done writing buffered video."); } // end if have delay capability @@ -605,11 +610,11 @@ void VideoStore::flush_codecs() { // Should probably set the frame size to what is reported FIXME if (zm_get_samples_from_fifo(fifo, out_frame)) { if (zm_send_frame_receive_packet(audio_out_ctx, out_frame, *pkt) > 0) { + av_packet_guard pkt_guard{pkt}; av_packet_rescale_ts(pkt.get(), audio_out_ctx->time_base, audio_out_stream->time_base); write_packet(pkt.get(), audio_out_stream); - zm_av_packet_unref(pkt.get()); } } // end if data returned from fifo } @@ -626,13 +631,13 @@ void VideoStore::flush_codecs() { // SHould probably set the frame size to what is reported FIXME if (av_audio_fifo_read(fifo, (void **)out_frame->data, frame_size)) { if (zm_send_frame_receive_packet(audio_out_ctx, out_frame, *pkt)) { + av_packet_guard pkt_guard{pkt}; pkt->stream_index = audio_out_stream->index; av_packet_rescale_ts(pkt.get(), audio_out_ctx->time_base, audio_out_stream->time_base); write_packet(pkt.get(), audio_out_stream); - zm_av_packet_unref(pkt.get()); } } // end if data returned from fifo } // end while still data in the fifo @@ -645,12 +650,12 @@ void VideoStore::flush_codecs() { Debug(1, "No more packets"); break; } + av_packet_guard pkt_guard{pkt}; ZM_DUMP_PACKET(pkt, "raw from encoder"); av_packet_rescale_ts(pkt.get(), audio_out_ctx->time_base, audio_out_stream->time_base); ZM_DUMP_STREAM_PACKET(audio_out_stream, pkt, "writing flushed packet"); write_packet(pkt.get(), audio_out_stream); - zm_av_packet_unref(pkt.get()); } // while have buffered frames } // end if audio_out_codec } // end flush_codecs @@ -1015,6 +1020,8 @@ int VideoStore::writePacket(const std::shared_ptr &zm_pkt) { } int VideoStore::writeVideoFramePacket(const std::shared_ptr &zm_packet) { + av_packet_guard pkt_guard; + frame_count += 1; // if we have to transcode @@ -1134,6 +1141,7 @@ int VideoStore::writeVideoFramePacket(const std::shared_ptr &zm_packet } return ret; } + pkt_guard.acquire(opkt); ZM_DUMP_PACKET(opkt, "packet returned by codec"); // Need to adjust pts/dts values from codec time to stream time @@ -1189,6 +1197,7 @@ int VideoStore::writeVideoFramePacket(const std::shared_ptr &zm_packet ZM_DUMP_STREAM_PACKET(video_in_stream, ipkt, "Doing passthrough, just copy packet"); // Just copy it because the codec is the same av_packet_ref(opkt.get(), ipkt); + pkt_guard.acquire(opkt); if (ipkt->dts != AV_NOPTS_VALUE) { if (video_first_dts == AV_NOPTS_VALUE) { @@ -1208,7 +1217,6 @@ int VideoStore::writeVideoFramePacket(const std::shared_ptr &zm_packet } // end if codec matches write_packet(opkt.get(), video_out_stream); - zm_av_packet_unref(opkt.get()); if (hw_frame) av_frame_free(&hw_frame); return 1;