* AVWriter::StreamCookie::~StreamCookie() incorrectly freed packet data that it

didn't own. * We are supposed to open the AVCodecContext in the writer, even though we never use it. According to libav-users mailing list, this is necessary, since that will allocate and initialize some structures that are later needed in av_write_header(). How this is supposed to work for encoders that libavcodec does not support, or which we don't know how to map, I do not yet know. For now it doesn't matter and resolves the problem that audio tracks report the wrong stream duration. * Some more improvements with regards to what information we need to fill out and which we don't. * Use more sensible defaults for the stream bitrate, so that we get better quality video by default. This and other parameters can be calculated when we implement setting the quality. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@32103 a95241bf-73f2-0310-859d-f6bbb57e9c96
2009-08-03 21:32:03 +00:00
parent 98977abc9b
commit 0c72a8aed4
2 changed files with 129 additions and 38 deletions
@@ -132,11 +132,16 @@ AVCodecEncoder::SetUp(const media_format* inputFormat)
 //		fContext->gop_size = 12;
 		// TODO: Fix pixel format or setup conversion method...
 		fContext->pix_fmt = PIX_FMT_YUV420P;
-//		fContext->rate_emu = 0;
+
 		// TODO: Setup rate control:
+//		fContext->rate_emu = 0;
 //		fContext->rc_eq = NULL;
 //		fContext->rc_max_rate = 0;
 //		fContext->rc_min_rate = 0;
+		// TODO: Try to calculate a good bit rate...
+		fContext->bit_rate = 800000;
+
+		// Pixel aspect ratio
 		fContext->sample_aspect_ratio.num
 			= fInputFormat.u.raw_video.pixel_width_aspect;
 		fContext->sample_aspect_ratio.den
@@ -180,6 +185,8 @@ AVCodecEncoder::SetUp(const media_format* inputFormat)
 	} else if (fInputFormat.type == B_MEDIA_RAW_AUDIO) {
 		// frame rate
 		fContext->sample_rate = (int)fInputFormat.u.raw_audio.frame_rate;
+		// NOTE: From the output_example.c, it looks like we are not supposed
+		// to set this.
 		fContext->time_base.den = (int)fInputFormat.u.raw_audio.frame_rate;
 		fContext->time_base.num = 1;
 		// channels
@@ -243,6 +250,17 @@ AVCodecEncoder::SetUp(const media_format* inputFormat)
 		return B_NOT_SUPPORTED;
 	}

+	// Add some known fixes from the FFmpeg API example:
+	if (fContext->codec_id == CODEC_ID_MPEG2VIDEO) {
+		// Just for testing, we also add B frames */
+		fContext->max_b_frames = 2;
+    } else if (fContext->codec_id == CODEC_ID_MPEG1VIDEO){
+		// Needed to avoid using macroblocks in which some coeffs overflow.
+		// This does not happen with normal video, it just happens here as
+		// the motion of the chroma plane does not match the luma plane.
+		fContext->mb_decision = 2;
+    }
+
 	// Open the codec
 	int result = avcodec_open(fContext, fCodec);
 	fCodecInitDone = (result >= 0);
@@ -292,6 +310,11 @@ AVCodecEncoder::Encode(const void* buffer, int64 frameCount,
 // #pragma mark -


+static const int64 kNoPTSValue = 0x8000000000000000LL;
+	// NOTE: For some reasons, I have trouble with the avcodec.h define:
+	// #define AV_NOPTS_VALUE          INT64_C(0x8000000000000000)
+	// INT64_C is not defined here.
+
 status_t
 AVCodecEncoder::_EncodeAudio(const void* _buffer, int64 frameCount,
 	media_encode_info* info)
@@ -311,12 +334,6 @@ AVCodecEncoder::_EncodeAudio(const void* _buffer, int64 frameCount,
 	size_t inputFrameSize = inputSampleSize
 		* fInputFormat.u.raw_audio.channel_count;

-	size_t outSampleSize = av_get_bits_per_sample_format(
-		fContext->sample_fmt) / 8;
-	size_t outSize = outSampleSize * fContext->channels;
-	TRACE("  sampleSize: %ld/%ld, frameSize: %ld/%ld\n",
-		inputSampleSize, inputFrameSize, outSampleSize, outSize);
-
 	size_t bufferSize = frameCount * inputFrameSize;
 	bufferSize = min_c(bufferSize, kDefaultChunkBufferSize);

@@ -342,6 +359,16 @@ AVCodecEncoder::_EncodeAudio(const void* _buffer, int64 frameCount,
 			return B_ERROR;
 		}

+		// Maybe we need to use this PTS to calculate start_time:
+		if (fContext->coded_frame->pts != kNoPTSValue) {
+			TRACE("  codec frame PTS: %lld (codec time_base: %d/%d)\n",
+				fContext->coded_frame->pts, fContext->time_base.num,
+				fContext->time_base.den);
+		} else {
+			TRACE("  codec frame PTS: N/A (codec time_base: %d/%d)\n",
+				fContext->time_base.num, fContext->time_base.den);
+		}
+
 		// Setup media_encode_info, most important is the time stamp.
 		info->start_time = (bigtime_t)(fFramesWritten * 1000000LL
 			/ fInputFormat.u.raw_audio.frame_rate);
@@ -407,6 +434,16 @@ AVCodecEncoder::_EncodeVideo(const void* buffer, int64 frameCount,
 			return B_ERROR;
 		}

+		// Maybe we need to use this PTS to calculate start_time:
+		if (fContext->coded_frame->pts != kNoPTSValue) {
+			TRACE("  codec frame PTS: %lld (codec time_base: %d/%d)\n",
+				fContext->coded_frame->pts, fContext->time_base.num,
+				fContext->time_base.den);
+		} else {
+			TRACE("  codec frame PTS: N/A (codec time_base: %d/%d)\n",
+				fContext->time_base.num, fContext->time_base.den);
+		}
+
 		// Setup media_encode_info, most important is the time stamp.
 		info->start_time = (bigtime_t)(fFramesWritten * 1000000LL
 			/ fInputFormat.u.raw_video.field_rate);
@@ -44,9 +44,11 @@ static const size_t kIOBufferSize = 64 * 1024;
 	// TODO: This could depend on the BMediaFile creation flags, IIRC,
 	// they allow to specify a buffering mode.

-
-// #pragma mark - URLProtocol
-// TODO: Do we need to write an URLProtocol?
+// NOTE: The following works around some weird bug in libavformat. We
+// have to open the AVFormatContext->AVStream->AVCodecContext, even though
+// we are not interested in donig any encoding here!!
+#define OPEN_CODEC_CONTEXT 1
+#define GET_CONTEXT_DEFAULTS 0


 // #pragma mark - AVFormatWriter::StreamCookie
@@ -72,10 +74,10 @@ private:
 			AVFormatContext*	fContext;
 			AVStream*			fStream;
 			AVPacket			fPacket;
+			bool				fCalculatePTS;
 			// Since different threads may write to the target,
 			// we need to protect the file position and I/O by a lock.
 			BLocker*			fStreamLock;
-			int64				fChunksWritten;
 };


@@ -85,16 +87,15 @@ AVFormatWriter::StreamCookie::StreamCookie(AVFormatContext* context,
 	:
 	fContext(context),
 	fStream(NULL),
-	fStreamLock(streamLock),
-	fChunksWritten(0)
+	fCalculatePTS(false),
+	fStreamLock(streamLock)
 {
-	av_new_packet(&fPacket, 0);
+	av_init_packet(&fPacket);
 }


 AVFormatWriter::StreamCookie::~StreamCookie()
 {
-	av_free_packet(&fPacket);
 }


@@ -102,7 +103,7 @@ status_t
 AVFormatWriter::StreamCookie::Init(const media_format* format,
 	const media_codec_info* codecInfo)
 {
-	TRACE("AVFormatWriter::StreamCookie::Init()\n");
+	TRACE("AVFormatWriter::StreamCookie::Init() (Yes, New)\n");

 	BAutolock _(fStreamLock);

@@ -114,16 +115,26 @@ AVFormatWriter::StreamCookie::Init(const media_format* format,
 		return B_ERROR;
 	}

+//	TRACE("  fStream->codec: %p\n", fStream->codec);
+	// TODO: This is a hack for now! Use avcodec_find_encoder_by_name()
+	// or something similar...
+	fStream->codec->codec_id = (CodecID)codecInfo->sub_id;
+
 	// Setup the stream according to the media format...
 	if (format->type == B_MEDIA_RAW_VIDEO) {
-		avcodec_get_context_defaults2(fStream->codec, CODEC_TYPE_VIDEO);
+		fStream->codec->codec_type = CODEC_TYPE_VIDEO;
+#if GET_CONTEXT_DEFAULTS
+// NOTE: API example does not do this:
+		avcodec_get_context_defaults(fStream->codec);
+#endif
 		// frame rate
 		fStream->codec->time_base.den = (int)format->u.raw_video.field_rate;
 		fStream->codec->time_base.num = 1;
-		fStream->r_frame_rate.den = (int)format->u.raw_video.field_rate;
-		fStream->r_frame_rate.num = 1;
-		fStream->time_base.den = (int)format->u.raw_video.field_rate;
-		fStream->time_base.num = 1;
+// NOTE: API example does not do this:
+//		fStream->r_frame_rate.den = (int)format->u.raw_video.field_rate;
+//		fStream->r_frame_rate.num = 1;
+//		fStream->time_base.den = (int)format->u.raw_video.field_rate;
+//		fStream->time_base.num = 1;
 		// video size
 		fStream->codec->width = format->u.raw_video.display.line_width;
 		fStream->codec->height = format->u.raw_video.display.line_count;
@@ -142,8 +153,26 @@ AVFormatWriter::StreamCookie::Init(const media_format* format,
 		fStream->codec->sample_aspect_ratio = fStream->sample_aspect_ratio;
 		// TODO: Don't hard code this...
 		fStream->codec->pix_fmt = PIX_FMT_YUV420P;
+
+		// Some formats want stream headers to be separate
+		if ((fContext->oformat->flags & AVFMT_GLOBALHEADER) != 0)
+			fStream->codec->flags |= CODEC_FLAG_GLOBAL_HEADER;
+
+		fCalculatePTS = true;
 	} else if (format->type == B_MEDIA_RAW_AUDIO) {
-		avcodec_get_context_defaults2(fStream->codec, CODEC_TYPE_AUDIO);
+		fStream->codec->codec_type = CODEC_TYPE_AUDIO;
+#if GET_CONTEXT_DEFAULTS
+// NOTE: API example does not do this:
+		avcodec_get_context_defaults(fStream->codec);
+#endif
+		// frame rate
+		fStream->codec->sample_rate = (int)format->u.raw_audio.frame_rate;
+// NOTE: API example does not do this:
+//		fStream->codec->time_base.den = (int)format->u.raw_audio.frame_rate;
+//		fStream->codec->time_base.num = 1;
+//		fStream->time_base.den = (int)format->u.raw_audio.frame_rate;
+//		fStream->time_base.num = 1;
+
 		// channels
 		fStream->codec->channels = format->u.raw_audio.channel_count;
 		switch (format->u.raw_audio.format) {
@@ -201,22 +230,14 @@ AVFormatWriter::StreamCookie::Init(const media_format* format,
 			// The bits match 1:1 for media_multi_channels and FFmpeg defines.
 			fStream->codec->channel_layout = format->u.raw_audio.channel_mask;
 		}
-		// frame rate
-		fStream->codec->sample_rate = (int)format->u.raw_audio.frame_rate;
-		fStream->codec->time_base.den = (int)format->u.raw_audio.frame_rate;
-		fStream->codec->time_base.num = 1;
-		fStream->time_base.den = (int)format->u.raw_audio.frame_rate;
-		fStream->time_base.num = 1;
+
+		fCalculatePTS = false;
 	}

 	TRACE("  stream->time_base: (%d/%d), codec->time_base: (%d/%d))\n",
 		fStream->time_base.num, fStream->time_base.den,
 		fStream->codec->time_base.num, fStream->codec->time_base.den);

-	// TODO: This is a hack for now! Use avcodec_find_encoder_by_name()
-	// or something similar...
-	fStream->codec->codec_id = (CodecID)codecInfo->sub_id;
-
 	return B_OK;
 }

@@ -236,14 +257,17 @@ AVFormatWriter::StreamCookie::WriteChunk(const void* chunkBuffer,
 	fPacket.data = const_cast<uint8_t*>((const uint8_t*)chunkBuffer);
 	fPacket.size = chunkSize;

-	fPacket.pts = (encodeInfo->start_time
-		* fStream->time_base.den / fStream->time_base.num) / 1000000;
-	TRACE_PACKET("  PTS: %lld  (stream->time_base: (%d/%d), "
-		"codec->time_base: (%d/%d))\n", fPacket.pts,
-		fStream->time_base.num, fStream->time_base.den,
-		fStream->codec->time_base.num, fStream->codec->time_base.den);
+	if (fCalculatePTS) {
+		fPacket.pts = (encodeInfo->start_time
+			* fStream->time_base.den / fStream->time_base.num) / 1000000;
+		TRACE_PACKET("  PTS: %lld  (stream->time_base: (%d/%d), "
+			"codec->time_base: (%d/%d))\n", fPacket.pts,
+			fStream->time_base.num, fStream->time_base.den,
+			fStream->codec->time_base.num, fStream->codec->time_base.den);
+	}

 // From ffmpeg.c::do_audio_out():
+// TODO:
 //	if (enc->coded_frame && enc->coded_frame->pts != AV_NOPTS_VALUE)
 //		fPacket.pts = av_rescale_q(enc->coded_frame->pts,
 //		enc->time_base, ost->st->time_base);
@@ -299,6 +323,17 @@ AVFormatWriter::~AVFormatWriter()
 {
 	TRACE("AVFormatWriter::~AVFormatWriter\n");

+	// Free the streams and close the AVCodecContexts
+    for(unsigned i = 0; i < fContext->nb_streams; i++) {
+#if OPEN_CODEC_CONTEXT
+		// We only need to close the AVCodecContext when we opened it.
+		// This is experimental, see WriteHeader().
+		avcodec_close(fContext->streams[i]->codec);
+#endif
+		av_freep(&fContext->streams[i]->codec);
+		av_freep(&fContext->streams[i]);
+    }
+
 	av_free(fContext);

 	delete[] fIOBuffer;
@@ -365,8 +400,24 @@ AVFormatWriter::CommitHeader()
 	if (fHeaderWritten)
 		return B_NOT_ALLOWED;

+	// According to output_example.c, the output parameters must be set even
+	// if none are specified. In the example, this call is used after the
+	// streams have been created.
+	if (av_set_parameters(fContext, NULL) < 0)
+		return B_ERROR;
+
 	for (unsigned i = 0; i < fContext->nb_streams; i++) {
 		AVStream* stream = fContext->streams[i];
+#if OPEN_CODEC_CONTEXT
+		// NOTE: Experimental, this should not be needed. Especially, since
+		// we have no idea (in the future) what CodecID some encoder uses,
+		// it may be an encoder from a different plugin.
+		AVCodecContext* codecContext = stream->codec;
+		AVCodec* codec = avcodec_find_encoder(codecContext->codec_id);
+		if (codec == NULL || avcodec_open(codecContext, codec) < 0) {
+			TRACE("  stream[%u] - failed to open AVCodecContext\n", i);
+		}
+#endif
 		TRACE("  stream[%u] time_base: (%d/%d), codec->time_base: (%d/%d)\n",
 			i, stream->time_base.num, stream->time_base.den,
 			stream->codec->time_base.num, stream->codec->time_base.den);
@@ -424,6 +475,9 @@ AVFormatWriter::AllocateCookie(void** _cookie, const media_format* format,
 {
 	TRACE("AVFormatWriter::AllocateCookie()\n");

+	if (fHeaderWritten)
+		return B_NOT_ALLOWED;
+
 	BAutolock _(fStreamLock);

 	if (_cookie == NULL)