On Wed, 22 Feb 2017 00:14:32 +0100 Marton Balint <[email protected]> wrote:
> This ensures that the wrapped avframe will not get reallocated later, which > would invalidate internal references such as extended data. > > Signed-off-by: Marton Balint <[email protected]> > --- > libavcodec/wrapped_avframe.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/wrapped_avframe.c b/libavcodec/wrapped_avframe.c > index 13c8d8a..1436032 100644 > --- a/libavcodec/wrapped_avframe.c > +++ b/libavcodec/wrapped_avframe.c > @@ -43,19 +43,31 @@ static int wrapped_avframe_encode(AVCodecContext *avctx, > AVPacket *pkt, > const AVFrame *frame, int *got_packet) > { > AVFrame *wrapped = av_frame_clone(frame); > + uint8_t *data; > + int size = sizeof(*wrapped) + AV_INPUT_BUFFER_PADDING_SIZE; > > if (!wrapped) > return AVERROR(ENOMEM); > > - pkt->buf = av_buffer_create((uint8_t *)wrapped, sizeof(*wrapped), > + data = av_mallocz(size); > + if (!data) { > + av_frame_free(&wrapped); > + return AVERROR(ENOMEM); > + } > + > + pkt->buf = av_buffer_create(data, size, > wrapped_avframe_release_buffer, NULL, > AV_BUFFER_FLAG_READONLY); > if (!pkt->buf) { > av_frame_free(&wrapped); > + av_freep(&data); > return AVERROR(ENOMEM); > } > > - pkt->data = (uint8_t *)wrapped; > + av_frame_move_ref((AVFrame*)data, wrapped); > + av_frame_free(&wrapped); I think this could be done as just av_frame_ref((AVFrame*)data, frame); without calling av_frame_clone(), but it doesn't hurt either. (Changing it might be an optional, minor improvement.) > + > + pkt->data = data; > pkt->size = sizeof(*wrapped); > > pkt->flags |= AV_PKT_FLAG_KEY; Patch looks good, and I like it better than checking the codec ID. And now something in general: wrapped_avframe is a special-case, because it stores a pointer in what is supposed to be just a raw byte array. So it's always possible that AVPacket use with it breaks in tricky ways that is fine with normal codecs. To make wrapped_avframe absolutely safe, we could either introduce a separate AVBuffer field just for wrapped_avframe, or we could make side-data refcounted (like with AVFrame), and store the AVFrame in side-data, which would probably be slightly more robust. Maybe something to consider on the next ABI bump. _______________________________________________ ffmpeg-devel mailing list [email protected] http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
