Message ID | 20170422172609.5292-1-jamrial@gmail.com |
---|---|
State | Accepted |
Commit | 54a4c9b4e9a1524b1ac5d2be97c8042272402d0a |
Headers | show |
On 4/22/2017 10:26 AM, James Almer wrote: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/options.c | 30 +++++++++++++++--------------- > 1 file changed, 15 insertions(+), 15 deletions(-) > > diff --git a/libavcodec/options.c b/libavcodec/options.c > index 7bdb0be5af..b98da9378a 100644 > --- a/libavcodec/options.c > +++ b/libavcodec/options.c > @@ -188,6 +188,19 @@ void avcodec_free_context(AVCodecContext **pavctx) > } > > #if FF_API_COPY_CONTEXT > +static void copy_context_reset(AVCodecContext *avctx) > +{ > + av_opt_free(avctx); > + av_freep(&avctx->rc_override); > + av_freep(&avctx->intra_matrix); > + av_freep(&avctx->inter_matrix); > + av_freep(&avctx->extradata); > + av_freep(&avctx->subtitle_header); > + av_buffer_unref(&avctx->hw_frames_ctx); > + avctx->subtitle_header_size = 0; > + avctx->extradata_size = 0; > +} > + > int avcodec_copy_context(AVCodecContext *dest, const AVCodecContext *src) > { > const AVCodec *orig_codec = dest->codec; > @@ -200,12 +213,7 @@ int avcodec_copy_context(AVCodecContext *dest, const AVCodecContext *src) > return AVERROR(EINVAL); > } > > - av_opt_free(dest); > - av_freep(&dest->rc_override); > - av_freep(&dest->intra_matrix); > - av_freep(&dest->inter_matrix); > - av_freep(&dest->extradata); > - av_freep(&dest->subtitle_header); > + copy_context_reset(dest); > > memcpy(dest, src, sizeof(*dest)); > av_opt_copy(dest, src); > @@ -264,15 +272,7 @@ FF_ENABLE_DEPRECATION_WARNINGS > return 0; > > fail: > - av_freep(&dest->subtitle_header); > - av_freep(&dest->rc_override); > - av_freep(&dest->intra_matrix); > - av_freep(&dest->inter_matrix); > - av_freep(&dest->extradata); > - av_buffer_unref(&dest->hw_frames_ctx); > - dest->subtitle_header_size = 0; > - dest->extradata_size = 0; > - av_opt_free(dest); > + copy_context_reset(dest); > return AVERROR(ENOMEM); > } > #endif > There is one real difference in the behavior--it will call av_buffer_unref(&dest->hw_frames_ctx) the first time around, and this wasn't done before. The initialization of subtitle_header_size and extradata_size to 0 doesn't really matter the first time around, since they will be overwritten anyway as a result of using memcpy(), but the use of av_buffer_unref() does technically change the behavior compared to what it used to do. It is likely correct to do that the first time through, but I wonder, why not also do av_buffer_unref(&avctx->hw_device_ctx) and a bunch of the other things that avcodec_close() already does? The call to memcpy() will basically overwrite dest with the values in src, and besides the copies made of dest->codec and dest->priv_data earlier in the function, anything else that is overwritten could in theory leak. But, I guess that would have been a preexisting issue with this function and not really relevant for this patch. And this function is deprecated anyway. If there is no issue with the additional call to av_buffer_unref(), as mentioned above, this patch looks good. Aaron Levinson
diff --git a/libavcodec/options.c b/libavcodec/options.c index 7bdb0be5af..b98da9378a 100644 --- a/libavcodec/options.c +++ b/libavcodec/options.c @@ -188,6 +188,19 @@ void avcodec_free_context(AVCodecContext **pavctx) } #if FF_API_COPY_CONTEXT +static void copy_context_reset(AVCodecContext *avctx) +{ + av_opt_free(avctx); + av_freep(&avctx->rc_override); + av_freep(&avctx->intra_matrix); + av_freep(&avctx->inter_matrix); + av_freep(&avctx->extradata); + av_freep(&avctx->subtitle_header); + av_buffer_unref(&avctx->hw_frames_ctx); + avctx->subtitle_header_size = 0; + avctx->extradata_size = 0; +} + int avcodec_copy_context(AVCodecContext *dest, const AVCodecContext *src) { const AVCodec *orig_codec = dest->codec; @@ -200,12 +213,7 @@ int avcodec_copy_context(AVCodecContext *dest, const AVCodecContext *src) return AVERROR(EINVAL); } - av_opt_free(dest); - av_freep(&dest->rc_override); - av_freep(&dest->intra_matrix); - av_freep(&dest->inter_matrix); - av_freep(&dest->extradata); - av_freep(&dest->subtitle_header); + copy_context_reset(dest); memcpy(dest, src, sizeof(*dest)); av_opt_copy(dest, src); @@ -264,15 +272,7 @@ FF_ENABLE_DEPRECATION_WARNINGS return 0; fail: - av_freep(&dest->subtitle_header); - av_freep(&dest->rc_override); - av_freep(&dest->intra_matrix); - av_freep(&dest->inter_matrix); - av_freep(&dest->extradata); - av_buffer_unref(&dest->hw_frames_ctx); - dest->subtitle_header_size = 0; - dest->extradata_size = 0; - av_opt_free(dest); + copy_context_reset(dest); return AVERROR(ENOMEM); } #endif
Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/options.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)