[Bf-blender-cvs] SVN commit: /data/svn/bf-blender [52801] trunk/blender/source/blender/imbuf /intern: Fix #33433: Importing video files into movie clip editor crashes Blender

[Sergey Sharybin]

Revision: 52801
          http://projects.blender.org/scm/viewvc.php?view=rev&root=bf-blender&revision=52801
Author:   nazgul
Date:     2012-12-06 21:08:24 +0000 (Thu, 06 Dec 2012)
Log Message:
-----------
Fix #33433: Importing video files into movie clip editor crashes Blender

This was a regression in svn rev52718 caused by the fact that we can not
free packet fun until we've finished all manipulation with decoded frame
since frame and packet could share same pointers.

For now restored old behavior of next_packet which seems to be well
tested and better not do bigger refactoring here so close to release.

Memory leak fixed by that revision was fixed by calling av_free_packet
just before avcodec_decode_video2 in cases we're at the end of file.

Tested with valgrind and could not see any memory leaks in ffmpeg
area.

Revision Links:
--------------
    http://projects.blender.org/scm/viewvc.php?view=rev&root=bf-blender&revision=52718

Modified Paths:
--------------
    trunk/blender/source/blender/imbuf/intern/IMB_anim.h
    trunk/blender/source/blender/imbuf/intern/anim_movie.c

Modified: trunk/blender/source/blender/imbuf/intern/IMB_anim.h
===================================================================
--- trunk/blender/source/blender/imbuf/intern/IMB_anim.h	2012-12-06 18:02:44 UTC (rev 52800)
+++ trunk/blender/source/blender/imbuf/intern/IMB_anim.h	2012-12-06 21:08:24 UTC (rev 52801)
@@ -178,6 +178,7 @@
 	struct ImBuf *last_frame;
 	int64_t last_pts;
 	int64_t next_pts;
+	AVPacket next_packet;
 #endif
 
 #ifdef WITH_REDCODE

Modified: trunk/blender/source/blender/imbuf/intern/anim_movie.c
===================================================================
--- trunk/blender/source/blender/imbuf/intern/anim_movie.c	2012-12-06 18:02:44 UTC (rev 52800)
+++ trunk/blender/source/blender/imbuf/intern/anim_movie.c	2012-12-06 21:08:24 UTC (rev 52801)
@@ -557,6 +557,7 @@
 	anim->last_frame = 0;
 	anim->last_pts = -1;
 	anim->next_pts = -1;
+	anim->next_packet.stream_index = -1;
 
 	anim->pFormatCtx = pFormatCtx;
 	anim->pCodecCtx = pCodecCtx;
@@ -763,39 +764,41 @@
 	}
 }
 
-/* decode one video frame */
+/* decode one video frame also considering the packet read into next_packet */
 
 static int ffmpeg_decode_video_frame(struct anim *anim)
 {
 	int rval = 0;
-	AVPacket next_packet;
 
-	memset(&next_packet, 0, sizeof(AVPacket));
-
 	av_log(anim->pFormatCtx, AV_LOG_DEBUG, "  DECODE VIDEO FRAME\n");
 
-	while ((rval = av_read_frame(anim->pFormatCtx, &next_packet)) >= 0) {
+	if (anim->next_packet.stream_index == anim->videoStream) {
+		av_free_packet(&anim->next_packet);
+		anim->next_packet.stream_index = -1;
+	}
+	
+	while ((rval = av_read_frame(anim->pFormatCtx, &anim->next_packet)) >= 0) {
 		av_log(anim->pFormatCtx, 
 		       AV_LOG_DEBUG, 
 		       "%sREAD: strID=%d (VID: %d) dts=%lld pts=%lld "
 		       "%s\n",
-		       (next_packet.stream_index == anim->videoStream)
+		       (anim->next_packet.stream_index == anim->videoStream)
 		       ? "->" : "  ",
-		       next_packet.stream_index, 
+		       anim->next_packet.stream_index, 
 		       anim->videoStream,
-		       (next_packet.dts == AV_NOPTS_VALUE) ? -1 :
-		       (long long int)next_packet.dts,
-		       (next_packet.pts == AV_NOPTS_VALUE) ? -1 :
-		       (long long int)next_packet.pts,
-		       (next_packet.flags & AV_PKT_FLAG_KEY) ? 
+		       (anim->next_packet.dts == AV_NOPTS_VALUE) ? -1 :
+		       (long long int)anim->next_packet.dts,
+		       (anim->next_packet.pts == AV_NOPTS_VALUE) ? -1 :
+		       (long long int)anim->next_packet.pts,
+		       (anim->next_packet.flags & AV_PKT_FLAG_KEY) ? 
 		       " KEY" : "");
-		if (next_packet.stream_index == anim->videoStream) {
+		if (anim->next_packet.stream_index == anim->videoStream) {
 			anim->pFrameComplete = 0;
 
 			avcodec_decode_video2(
 			    anim->pCodecCtx,
 			    anim->pFrame, &anim->pFrameComplete,
-			    &next_packet);
+			    &anim->next_packet);
 
 			if (anim->pFrameComplete) {
 				anim->next_pts = av_get_pts_from_frame(
@@ -813,24 +816,28 @@
 				break;
 			}
 		}
-		av_free_packet(&next_packet);
+		av_free_packet(&anim->next_packet);
+		anim->next_packet.stream_index = -1;
 	}
+	
+	if (rval == AVERROR_EOF) {
+		/* this sets size and data fields to zero,
+		   which is necessary to decode the remaining data
+		   in the decoder engine after EOF. It also prevents a memory
+		   leak, since av_read_frame spills out a full size packet even
+		   on EOF... (and: it's save to call on NULL packets) */
 
-	/* this sets size and data fields to zero,
-	   which is necessary to decode the remaining data
-	   in the decoder engine after EOF. It also prevents a memory
-	   leak, since av_read_frame spills out a full size packet even
-	   on EOF... (and: it's save to call on NULL packets) */
+		av_free_packet(&anim->next_packet);
 
-	av_free_packet(&next_packet);
-	
-	if (rval == AVERROR_EOF) {
+		anim->next_packet.size = 0;
+		anim->next_packet.data = 0;
+
 		anim->pFrameComplete = 0;
 
 		avcodec_decode_video2(
 			anim->pCodecCtx,
 			anim->pFrame, &anim->pFrameComplete,
-			&next_packet);
+			&anim->next_packet);
 
 		if (anim->pFrameComplete) {
 			anim->next_pts = av_get_pts_from_frame(
@@ -850,6 +857,8 @@
 	}
 
 	if (rval < 0) {
+		anim->next_packet.stream_index = -1;
+
 		av_log(anim->pFormatCtx,
 		       AV_LOG_ERROR, "  DECODE READ FAILED: av_read_frame() "
 		       "returned error: %d\n",  rval);
@@ -1086,6 +1095,13 @@
 
 		anim->next_pts = -1;
 
+		if (anim->next_packet.stream_index == anim->videoStream) {
+			av_free_packet(&anim->next_packet);
+			anim->next_packet.stream_index = -1;
+		}
+
+		/* memset(anim->pFrame, ...) ?? */
+
 		if (ret >= 0) {
 			ffmpeg_decode_video_frame_scan(anim, pts_to_search);
 		}
@@ -1132,6 +1148,9 @@
 		av_free(anim->pFrameDeinterlaced);
 		sws_freeContext(anim->img_convert_ctx);
 		IMB_freeImBuf(anim->last_frame);
+		if (anim->next_packet.stream_index != -1) {
+			av_free_packet(&anim->next_packet);
+		}
 	}
 	anim->duration = 0;
 }