Message ID | 20170422172609.5292-2-jamrial@gmail.com |
---|---|
State | Superseded |
Headers | show |
On 4/22/2017 10:26 AM, James Almer wrote: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/options.c | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/libavcodec/options.c b/libavcodec/options.c > index b98da9378a..c721aa8d43 100644 > --- a/libavcodec/options.c > +++ b/libavcodec/options.c > @@ -190,15 +190,21 @@ void avcodec_free_context(AVCodecContext **pavctx) > #if FF_API_COPY_CONTEXT > static void copy_context_reset(AVCodecContext *avctx) > { > + int i; > + > 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); > + for (i = 0; i < avctx->nb_coded_side_data; i++) > + av_freep(&avctx->coded_side_data[i].data); > + av_freep(&avctx->coded_side_data); > av_buffer_unref(&avctx->hw_frames_ctx); > avctx->subtitle_header_size = 0; > avctx->extradata_size = 0; > + avctx->nb_coded_side_data = 0; > } > > int avcodec_copy_context(AVCodecContext *dest, const AVCodecContext *src) > @@ -262,6 +268,28 @@ FF_ENABLE_DEPRECATION_WARNINGS > alloc_and_copy_or_fail(subtitle_header, src->subtitle_header_size, 1); > av_assert0(dest->subtitle_header_size == src->subtitle_header_size); > #undef alloc_and_copy_or_fail > + if (src->nb_coded_side_data) { > + int i; > + > + dest->nb_coded_side_data = 0; > + dest->coded_side_data = av_realloc_array(NULL, src->nb_coded_side_data, > + sizeof(*dest->coded_side_data)); > + if (!dest->coded_side_data) > + goto fail; > + > + for (i = 0; i < src->nb_coded_side_data; i++) { > + const AVPacketSideData *sd_src = &src->coded_side_data[i]; > + AVPacketSideData *sd_dst = &dest->coded_side_data[i]; > + > + sd_dst->data = av_malloc(sd_src->size); > + if (!sd_dst->data) > + goto fail; > + memcpy(sd_dst->data, sd_src->data, sd_src->size); > + sd_dst->size = sd_src->size; > + sd_dst->type = sd_src->type; > + dest->nb_coded_side_data++; > + } > + } > > if (src->hw_frames_ctx) { > dest->hw_frames_ctx = av_buffer_ref(src->hw_frames_ctx); > If src has coded_side_data and nb_coded_side_data set to non-zero values, as a result of the earlier call to memcpy(), it will copy those values into dest. Then, if it does a goto to fail early, as a result of your changes to copy_context_reset(), it will call av_freep() on those elements in dest, even though they are owned by src. After doing the memcpy() call, I think you can fix this by setting dest->coded_side_data to NULL and dest->nb_coded_side_data to 0. Everything else looks good. Aaron Levinson
On 4/23/2017 8:27 PM, Aaron Levinson wrote: > On 4/22/2017 10:26 AM, James Almer wrote: >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavcodec/options.c | 28 ++++++++++++++++++++++++++++ >> 1 file changed, 28 insertions(+) >> >> diff --git a/libavcodec/options.c b/libavcodec/options.c >> index b98da9378a..c721aa8d43 100644 >> --- a/libavcodec/options.c >> +++ b/libavcodec/options.c >> @@ -190,15 +190,21 @@ void avcodec_free_context(AVCodecContext **pavctx) >> #if FF_API_COPY_CONTEXT >> static void copy_context_reset(AVCodecContext *avctx) >> { >> + int i; >> + >> 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); >> + for (i = 0; i < avctx->nb_coded_side_data; i++) >> + av_freep(&avctx->coded_side_data[i].data); >> + av_freep(&avctx->coded_side_data); >> av_buffer_unref(&avctx->hw_frames_ctx); >> avctx->subtitle_header_size = 0; >> avctx->extradata_size = 0; >> + avctx->nb_coded_side_data = 0; >> } >> >> int avcodec_copy_context(AVCodecContext *dest, const AVCodecContext >> *src) >> @@ -262,6 +268,28 @@ FF_ENABLE_DEPRECATION_WARNINGS >> alloc_and_copy_or_fail(subtitle_header, >> src->subtitle_header_size, 1); >> av_assert0(dest->subtitle_header_size == src->subtitle_header_size); >> #undef alloc_and_copy_or_fail >> + if (src->nb_coded_side_data) { >> + int i; >> + >> + dest->nb_coded_side_data = 0; >> + dest->coded_side_data = av_realloc_array(NULL, >> src->nb_coded_side_data, >> + >> sizeof(*dest->coded_side_data)); >> + if (!dest->coded_side_data) >> + goto fail; >> + >> + for (i = 0; i < src->nb_coded_side_data; i++) { >> + const AVPacketSideData *sd_src = &src->coded_side_data[i]; >> + AVPacketSideData *sd_dst = &dest->coded_side_data[i]; >> + >> + sd_dst->data = av_malloc(sd_src->size); >> + if (!sd_dst->data) >> + goto fail; >> + memcpy(sd_dst->data, sd_src->data, sd_src->size); >> + sd_dst->size = sd_src->size; >> + sd_dst->type = sd_src->type; >> + dest->nb_coded_side_data++; >> + } >> + } >> >> if (src->hw_frames_ctx) { >> dest->hw_frames_ctx = av_buffer_ref(src->hw_frames_ctx); >> > > If src has coded_side_data and nb_coded_side_data set to non-zero > values, as a result of the earlier call to memcpy(), it will copy those > values into dest. Then, if it does a goto to fail early, as a result of > your changes to copy_context_reset(), it will call av_freep() on those > elements in dest, even though they are owned by src. After doing the > memcpy() call, I think you can fix this by setting dest->coded_side_data > to NULL and dest->nb_coded_side_data to 0. Good catch, will send a new version of the set. > > Everything else looks good. > > Aaron Levinson > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
diff --git a/libavcodec/options.c b/libavcodec/options.c index b98da9378a..c721aa8d43 100644 --- a/libavcodec/options.c +++ b/libavcodec/options.c @@ -190,15 +190,21 @@ void avcodec_free_context(AVCodecContext **pavctx) #if FF_API_COPY_CONTEXT static void copy_context_reset(AVCodecContext *avctx) { + int i; + 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); + for (i = 0; i < avctx->nb_coded_side_data; i++) + av_freep(&avctx->coded_side_data[i].data); + av_freep(&avctx->coded_side_data); av_buffer_unref(&avctx->hw_frames_ctx); avctx->subtitle_header_size = 0; avctx->extradata_size = 0; + avctx->nb_coded_side_data = 0; } int avcodec_copy_context(AVCodecContext *dest, const AVCodecContext *src) @@ -262,6 +268,28 @@ FF_ENABLE_DEPRECATION_WARNINGS alloc_and_copy_or_fail(subtitle_header, src->subtitle_header_size, 1); av_assert0(dest->subtitle_header_size == src->subtitle_header_size); #undef alloc_and_copy_or_fail + if (src->nb_coded_side_data) { + int i; + + dest->nb_coded_side_data = 0; + dest->coded_side_data = av_realloc_array(NULL, src->nb_coded_side_data, + sizeof(*dest->coded_side_data)); + if (!dest->coded_side_data) + goto fail; + + for (i = 0; i < src->nb_coded_side_data; i++) { + const AVPacketSideData *sd_src = &src->coded_side_data[i]; + AVPacketSideData *sd_dst = &dest->coded_side_data[i]; + + sd_dst->data = av_malloc(sd_src->size); + if (!sd_dst->data) + goto fail; + memcpy(sd_dst->data, sd_src->data, sd_src->size); + sd_dst->size = sd_src->size; + sd_dst->type = sd_src->type; + dest->nb_coded_side_data++; + } + } if (src->hw_frames_ctx) { dest->hw_frames_ctx = av_buffer_ref(src->hw_frames_ctx);
Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/options.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)