From 0c72a8aed40d93cebb8a69f08e2449888913f6bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Mon, 3 Aug 2009 21:32:03 +0000 Subject: [PATCH] * 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 --- .../media/plugins/ffmpeg/AVCodecEncoder.cpp | 51 ++++++-- .../media/plugins/ffmpeg/AVFormatWriter.cpp | 116 +++++++++++++----- 2 files changed, 129 insertions(+), 38 deletions(-) diff --git a/src/add-ons/media/plugins/ffmpeg/AVCodecEncoder.cpp b/src/add-ons/media/plugins/ffmpeg/AVCodecEncoder.cpp index d7680beb45..197c65c276 100644 --- a/src/add-ons/media/plugins/ffmpeg/AVCodecEncoder.cpp +++ b/src/add-ons/media/plugins/ffmpeg/AVCodecEncoder.cpp @@ -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); diff --git a/src/add-ons/media/plugins/ffmpeg/AVFormatWriter.cpp b/src/add-ons/media/plugins/ffmpeg/AVFormatWriter.cpp index b311a28371..bbcf53d907 100644 --- a/src/add-ons/media/plugins/ffmpeg/AVFormatWriter.cpp +++ b/src/add-ons/media/plugins/ffmpeg/AVFormatWriter.cpp @@ -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((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)