From 549d0bdca53af7a6e0c612ab4b03baecf3a5878f Mon Sep 17 00:00:00 2001 From: Anton Khirnov <anton@khirnov.net> Date: Wed, 26 Oct 2016 13:41:12 +0200 Subject: [PATCH] decode: be more explicit about storing the last packet properties The current code stores a pointer to the packet passed to the decoder, which is then used during get_buffer() for timestamps and side data passthrough. However, since this is a pointer to user data which we do not own, storing it is potentially dangerous. It is also ill defined for the new decoding API with split input/output. Fix this problem by making an explicit internally owned copy of the packet properties. --- libavcodec/decode.c | 33 ++++++++++++++++++++------------- libavcodec/internal.h | 6 +++--- libavcodec/pthread_frame.c | 2 +- libavcodec/utils.c | 8 ++++++++ 4 files changed, 32 insertions(+), 17 deletions(-) diff --git a/libavcodec/decode.c b/libavcodec/decode.c index 00085c3b4dc..b02278d3a68 100644 --- a/libavcodec/decode.c +++ b/libavcodec/decode.c @@ -99,6 +99,14 @@ fail2: return 0; } +static int extract_packet_props(AVCodecInternal *avci, const AVPacket *pkt) +{ + av_packet_unref(avci->last_pkt_props); + if (pkt) + return av_packet_copy_props(avci->last_pkt_props, pkt); + return 0; +} + static int unrefcount_frame(AVCodecInternal *avci, AVFrame *frame) { int ret; @@ -304,7 +312,10 @@ int attribute_align_arg avcodec_decode_video2(AVCodecContext *avctx, AVFrame *pi return AVERROR(ENOSYS); } - avctx->internal->pkt = avpkt; + ret = extract_packet_props(avci, avpkt); + if (ret < 0) + return ret; + ret = apply_param_change(avctx, avpkt); if (ret < 0) return ret; @@ -368,7 +379,9 @@ int attribute_align_arg avcodec_decode_audio4(AVCodecContext *avctx, return AVERROR(ENOSYS); } - avctx->internal->pkt = avpkt; + ret = extract_packet_props(avci, avpkt); + if (ret < 0) + return ret; if (!avpkt->data && avpkt->size) { av_log(avctx, AV_LOG_ERROR, "invalid packet: NULL data, size != 0\n"); @@ -408,7 +421,10 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub, { int ret; - avctx->internal->pkt = avpkt; + ret = extract_packet_props(avctx->internal, avpkt); + if (ret < 0) + return ret; + *got_sub_ptr = 0; ret = avctx->codec->decode(avctx, sub, got_sub_ptr, avpkt); if (*got_sub_ptr) @@ -739,7 +755,7 @@ int avcodec_default_get_buffer2(AVCodecContext *avctx, AVFrame *frame, int flags int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame) { - AVPacket *pkt = avctx->internal->pkt; + AVPacket *pkt = avctx->internal->last_pkt_props; int i; struct { enum AVPacketSideDataType packet; @@ -759,15 +775,6 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame) frame->chroma_location = avctx->chroma_sample_location; frame->reordered_opaque = avctx->reordered_opaque; - if (!pkt) { -#if FF_API_PKT_PTS -FF_DISABLE_DEPRECATION_WARNINGS - frame->pkt_pts = AV_NOPTS_VALUE; -FF_ENABLE_DEPRECATION_WARNINGS -#endif - frame->pts = AV_NOPTS_VALUE; - return 0; - } #if FF_API_PKT_PTS FF_DISABLE_DEPRECATION_WARNINGS diff --git a/libavcodec/internal.h b/libavcodec/internal.h index 4bde09ab8ad..2ca7a45e811 100644 --- a/libavcodec/internal.h +++ b/libavcodec/internal.h @@ -131,10 +131,10 @@ typedef struct AVCodecInternal { void *thread_ctx; /** - * Current packet as passed into the decoder, to avoid having to pass the - * packet into every function. + * Properties (timestamps+side data) extracted from the last packet passed + * for decoding. */ - AVPacket *pkt; + AVPacket *last_pkt_props; /** * hwaccel-specific private data diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c index 2736a81efea..75f4ff46247 100644 --- a/libavcodec/pthread_frame.c +++ b/libavcodec/pthread_frame.c @@ -656,7 +656,7 @@ int ff_frame_thread_init(AVCodecContext *avctx) } *copy->internal = *src->internal; copy->internal->thread_ctx = p; - copy->internal->pkt = &p->avpkt; + copy->internal->last_pkt_props = &p->avpkt; if (!i) { src = copy; diff --git a/libavcodec/utils.c b/libavcodec/utils.c index 5350eb819a9..b569b48f7a8 100644 --- a/libavcodec/utils.c +++ b/libavcodec/utils.c @@ -433,6 +433,12 @@ int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *code goto free_and_end; } + avctx->internal->last_pkt_props = av_packet_alloc(); + if (!avctx->internal->last_pkt_props) { + ret = AVERROR(ENOMEM); + goto free_and_end; + } + if (codec->priv_data_size > 0) { if (!avctx->priv_data) { avctx->priv_data = av_mallocz(codec->priv_data_size); @@ -713,6 +719,7 @@ FF_ENABLE_DEPRECATION_WARNINGS av_frame_free(&avctx->internal->to_free); av_frame_free(&avctx->internal->buffer_frame); av_packet_free(&avctx->internal->buffer_pkt); + av_packet_free(&avctx->internal->last_pkt_props); av_freep(&avctx->internal->pool); } av_freep(&avctx->internal); @@ -753,6 +760,7 @@ av_cold int avcodec_close(AVCodecContext *avctx) av_frame_free(&avctx->internal->to_free); av_frame_free(&avctx->internal->buffer_frame); av_packet_free(&avctx->internal->buffer_pkt); + av_packet_free(&avctx->internal->last_pkt_props); for (i = 0; i < FF_ARRAY_ELEMS(pool->pools); i++) av_buffer_pool_uninit(&pool->pools[i]); av_freep(&avctx->internal->pool); -- GitLab