FFMPEG Plugin: Implement audio input buffer padding.

- Padding is required by FFMPEG for correct operation of all audio decoders.
  FFMPEG performs some speed optimizations under the hood that may lead to
  reading over the end of the chunk buffer wouldn't there have been padding
  applied.

- Resolve TODOs by unifying fVideoChunkBuffer and fChunkBuffer back into
  fChunkBuffer because audio path is responsible for freeing fChunkBuffer now.
  Resolved TODOs apply to the replacing fVideoChunkBuffer variable by
  fChunkBuffer, rename some methods by removing the "Video" part, collapse two
  methods into one (_LoadNextChunkIfNeededAndAssignStartTime()).
  No functional change intended.

- Enhance "logging stream to file" functionality to write to distinct logging
  files for audio and video. Before this commit one could only log video
  streams. But with unifying the _LoadNextChunkIfNeededAndAssignStartTime()
  audio streams gained the logging functionality for free. But now audio and
  video streams would be written in the same log file when watching a media
  file containing both audio and video. This is prevented by the distinct
  logging mentioned above.

- Update documentation accordingly.
This commit is contained in:
Colin Günther 2014-08-24 23:52:49 +02:00
parent 3c68ae7c58
commit f7f6702203
2 changed files with 52 additions and 133 deletions

View File

@ -39,7 +39,11 @@
//#define LOG_STREAM_TO_FILE //#define LOG_STREAM_TO_FILE
#ifdef LOG_STREAM_TO_FILE #ifdef LOG_STREAM_TO_FILE
# include <File.h> # include <File.h>
static BFile sStreamLogFile("/boot/home/Desktop/AVCodecDebugStream.raw", static BFile sAudioStreamLogFile(
"/boot/home/Desktop/AVCodecDebugAudioStream.raw",
B_CREATE_FILE | B_ERASE_FILE | B_WRITE_ONLY);
static BFile sVideoStreamLogFile(
"/boot/home/Desktop/AVCodecDebugVideoStream.raw",
B_CREATE_FILE | B_ERASE_FILE | B_WRITE_ONLY); B_CREATE_FILE | B_ERASE_FILE | B_WRITE_ONLY);
static int sDumpedPackets = 0; static int sDumpedPackets = 0;
#endif #endif
@ -111,7 +115,6 @@ AVCodecDecoder::AVCodecDecoder()
fOutputFrameSize(0), fOutputFrameSize(0),
fChunkBuffer(NULL), fChunkBuffer(NULL),
fVideoChunkBuffer(NULL),
fChunkBufferSize(0), fChunkBufferSize(0),
fAudioDecodeError(false), fAudioDecodeError(false),
@ -145,9 +148,7 @@ AVCodecDecoder::~AVCodecDecoder()
if (fCodecInitDone) if (fCodecInitDone)
avcodec_close(fContext); avcodec_close(fContext);
free(fVideoChunkBuffer); free(fChunkBuffer);
// TODO: Replace with fChunkBuffer, once audio path is
// responsible for freeing the chunk buffer, too.
free(fDecodedData); free(fDecodedData);
av_free(fPostProcessedDecodedPicture); av_free(fPostProcessedDecodedPicture);
@ -270,10 +271,7 @@ AVCodecDecoder::SeekedTo(int64 frame, bigtime_t time)
} }
// Flush internal buffers as well. // Flush internal buffers as well.
free(fVideoChunkBuffer); free(fChunkBuffer);
// TODO: Replace with fChunkBuffer, once audio path is
// responsible for freeing the chunk buffer, too.
fVideoChunkBuffer = NULL;
fChunkBuffer = NULL; fChunkBuffer = NULL;
fChunkBufferSize = 0; fChunkBufferSize = 0;
fDecodedDataBufferOffset = 0; fDecodedDataBufferOffset = 0;
@ -360,6 +358,7 @@ AVCodecDecoder::_NegotiateAudioOutputFormat(media_format* inOutFormat)
return B_ERROR; return B_ERROR;
} }
free(fChunkBuffer);
fChunkBuffer = NULL; fChunkBuffer = NULL;
fChunkBufferSize = 0; fChunkBufferSize = 0;
fAudioDecodeError = false; fAudioDecodeError = false;
@ -472,6 +471,10 @@ AVCodecDecoder::_NegotiateVideoOutputFormat(media_format* inOutFormat)
fFormatConversionFunc = 0; fFormatConversionFunc = 0;
#endif #endif
free(fChunkBuffer);
fChunkBuffer = NULL;
fChunkBufferSize = 0;
_ResetTempPacket(); _ResetTempPacket();
status_t statusOfDecodingFirstFrame = _DecodeNextVideoFrame(); status_t statusOfDecodingFirstFrame = _DecodeNextVideoFrame();
@ -926,7 +929,7 @@ AVCodecDecoder::_DecodeNextAudioFrameChunk()
while(fDecodedDataBufferSize == 0) { while(fDecodedDataBufferSize == 0) {
status_t loadingChunkStatus status_t loadingChunkStatus
= _LoadNextAudioChunkIfNeededAndAssignStartTime(); = _LoadNextChunkIfNeededAndAssignStartTime();
if (loadingChunkStatus != B_OK) if (loadingChunkStatus != B_OK)
return loadingChunkStatus; return loadingChunkStatus;
@ -939,8 +942,7 @@ AVCodecDecoder::_DecodeNextAudioFrameChunk()
if (!fAudioDecodeError) { if (!fAudioDecodeError) {
// Report failure if not done already // Report failure if not done already
int32 chunkBufferOffset = fTempPacket.data int32 chunkBufferOffset = fTempPacket.data - fChunkBuffer;
- static_cast<uint8_t*>(const_cast<void*>(fChunkBuffer));
printf("########### audio decode error, " printf("########### audio decode error, "
"fTempPacket.size %d, fChunkBuffer data offset %ld\n", "fTempPacket.size %d, fChunkBuffer data offset %ld\n",
fTempPacket.size, chunkBufferOffset); fTempPacket.size, chunkBufferOffset);
@ -958,69 +960,6 @@ AVCodecDecoder::_DecodeNextAudioFrameChunk()
} }
/*! \brief Loads the next audio chunk into fChunkBuffer and assigns it
(including the start time) to fTempPacket accordingly only if
fTempPacket is empty.
\returns B_OK
1. meaning: Next audio chunk is loaded.
2. meaning: No need to load and assign anything. Proceed as usual.
\returns B_LAST_BUFFER_ERROR No more audio chunks available.
fChunkBuffer and fTempPacket are left untouched.
\returns Other errors Caller should bail out because fChunkBuffer and
fTempPacket are in unknown states. Normal operation cannot be
guaranteed.
*/
status_t
AVCodecDecoder::_LoadNextAudioChunkIfNeededAndAssignStartTime()
{
// TODO: Collapse _LoadNextAudioChunkIfNeededAndAssignStartTime() and
// _LoadNextVideoChunkIfNeededAndAssignStartTime() into one method called
// _LoadNextChunkIfNeededAndAssignStartTime() once the following conditions
// are met:
// 1. There is no longer a distinction between fVideoChunkBuffer and
// fChunkBuffer.
// 2. _LoadNextAudioChunkIfNeededAndAssignStartTime() adds Padding to
// the chunk buffer, too, like it is done in
// _LoadNextVideoChunkIfNeededAndAssignStartTime() at the moment.
if (fTempPacket.size > 0)
return B_OK;
const void* chunkBuffer = NULL;
size_t chunkBufferSize = 0;
// In the case that GetNextChunk() returns an error fChunkBufferSize
// should be left untouched.
media_header chunkMediaHeader;
status_t getNextChunkStatus = GetNextChunk(&chunkBuffer, &chunkBufferSize,
&chunkMediaHeader);
if (getNextChunkStatus != B_OK)
return getNextChunkStatus;
fChunkBuffer = chunkBuffer;
fChunkBufferSize = chunkBufferSize;
fTempPacket.data
= static_cast<uint8_t*>(const_cast<void*>(fChunkBuffer));
fTempPacket.size = fChunkBufferSize;
fTempPacket.dts = chunkMediaHeader.start_time;
// Let FFMPEG handle the correct relationship between start_time and
// decoded audio frames. By doing so we are merely following the way it
// is done for the video path.
// see _LoadNextVideoChunkIfNeededAndAssignStartTime()
//
// FIXME: Research how to establish a meaningful relationship
// between start_time and decoded audio frame when the received chunk
// buffer contains partial video frames. Maybe some data formats
// contain time stamps (ake pts / dts fields) that can be evaluated by
// FFMPEG. But as long as I don't have such audio data to test it, it
// makes no sense to implement it.
return B_OK;
}
/*! \brief Tries to decode at least one audio frame and store it in the /*! \brief Tries to decode at least one audio frame and store it in the
fDecodedDataBuffer. fDecodedDataBuffer.
@ -1185,11 +1124,9 @@ AVCodecDecoder::_DecodeNextVideoFrame()
while (true) { while (true) {
status_t loadingChunkStatus status_t loadingChunkStatus
= _LoadNextVideoChunkIfNeededAndAssignStartTime(); = _LoadNextChunkIfNeededAndAssignStartTime();
if (loadingChunkStatus == B_LAST_BUFFER_ERROR) if (loadingChunkStatus == B_LAST_BUFFER_ERROR)
return _FlushOneVideoFrameFromDecoderBuffer(); return _FlushOneVideoFrameFromDecoderBuffer();
if (loadingChunkStatus != B_OK) { if (loadingChunkStatus != B_OK) {
TRACE("AVCodecDecoder::_DecodeNextVideoFrame(): error from " TRACE("AVCodecDecoder::_DecodeNextVideoFrame(): error from "
"GetNextChunk(): %s\n", strerror(loadingChunkStatus)); "GetNextChunk(): %s\n", strerror(loadingChunkStatus));
@ -1301,25 +1238,21 @@ AVCodecDecoder::_ApplyEssentialVideoContainerPropertiesToContext()
} }
/*! \brief Loads the next video chunk into fVideoChunkBuffer and assigns it /*! \brief Loads the next chunk into fChunkBuffer and assigns it (including
(including the start time) to fTempPacket accordingly only if the start time) to fTempPacket but only if fTempPacket is empty.
fTempPacket is empty.
\returns B_OK \returns B_OK
1. meaning: Next video chunk is loaded. 1. meaning: Next chunk is loaded.
2. meaning: No need to load and assign anything. Proceed as usual. 2. meaning: No need to load and assign anything. Proceed as usual.
\returns B_LAST_BUFFER_ERROR No more video chunks available. \returns B_LAST_BUFFER_ERROR No more chunks available. fChunkBuffer and
fVideoChunkBuffer and fTempPacket are left untouched. fTempPacket are left untouched.
\returns Other errors Caller should bail out because fVideoChunkBuffer and \returns Other errors Caller should bail out because fChunkBuffer and
fTempPacket are in unknown states. Normal operation cannot be fTempPacket are in unknown states. Normal operation cannot be
guaranteed. guaranteed.
*/ */
status_t status_t
AVCodecDecoder::_LoadNextVideoChunkIfNeededAndAssignStartTime() AVCodecDecoder::_LoadNextChunkIfNeededAndAssignStartTime()
{ {
// TODO: Rename fVideoChunkBuffer to fChunkBuffer, once the audio path is
// responsible for releasing the chunk buffer, too.
if (fTempPacket.size > 0) if (fTempPacket.size > 0)
return B_OK; return B_OK;
@ -1335,25 +1268,25 @@ AVCodecDecoder::_LoadNextVideoChunkIfNeededAndAssignStartTime()
return getNextChunkStatus; return getNextChunkStatus;
status_t chunkBufferPaddingStatus status_t chunkBufferPaddingStatus
= _CopyChunkToVideoChunkBufferAndAddPadding(chunkBuffer, = _CopyChunkToChunkBufferAndAddPadding(chunkBuffer, chunkBufferSize);
chunkBufferSize);
if (chunkBufferPaddingStatus != B_OK) if (chunkBufferPaddingStatus != B_OK)
return chunkBufferPaddingStatus; return chunkBufferPaddingStatus;
fTempPacket.data = fVideoChunkBuffer; fTempPacket.data = fChunkBuffer;
fTempPacket.size = fChunkBufferSize; fTempPacket.size = fChunkBufferSize;
fTempPacket.dts = chunkMediaHeader.start_time; fTempPacket.dts = chunkMediaHeader.start_time;
// Let FFMPEG handle the correct relationship between start_time and // Let FFMPEG handle the correct relationship between start_time and
// decoded video frame. By doing so we are simply copying the way how // decoded a/v frame. By doing so we are simply copying the way how it
// it is implemented in ffplay.c // is implemented in ffplay.c for video frames (for audio frames it
// works, too, but isn't used by ffplay.c).
// \see http://git.videolan.org/?p=ffmpeg.git;a=blob;f=ffplay.c;h=09623db374e5289ed20b7cc28c262c4375a8b2e4;hb=9153b33a742c4e2a85ff6230aea0e75f5a8b26c2#l1502 // \see http://git.videolan.org/?p=ffmpeg.git;a=blob;f=ffplay.c;h=09623db374e5289ed20b7cc28c262c4375a8b2e4;hb=9153b33a742c4e2a85ff6230aea0e75f5a8b26c2#l1502
// //
// FIXME: Research how to establish a meaningful relationship // FIXME: Research how to establish a meaningful relationship between
// between start_time and decoded video frame when the received chunk // start_time and decoded a/v frame when the received chunk buffer
// buffer contains partial video frames. Maybe some data formats // contains partial a/v frames. Maybe some data formats do contain time
// contain time stamps (ake pts / dts fields) that can be evaluated by // stamps (ake pts / dts fields) that can be evaluated by FFMPEG. But
// FFMPEG. But as long as I don't have such video data to test it, it // as long as I don't have such video data to test it, it makes no
// makes no sense to implement it. // sense trying to implement it.
// //
// FIXME: Implement tracking start_time of video frames originating in // FIXME: Implement tracking start_time of video frames originating in
// data chunks that encode more than one video frame at a time. In that // data chunks that encode more than one video frame at a time. In that
@ -1363,19 +1296,20 @@ AVCodecDecoder::_LoadNextVideoChunkIfNeededAndAssignStartTime()
// to implement it. // to implement it.
#ifdef LOG_STREAM_TO_FILE #ifdef LOG_STREAM_TO_FILE
BFile* logFile = fIsAudio ? &sAudioStreamLogFile : &sVideoStreamLogFile;
if (sDumpedPackets < 100) { if (sDumpedPackets < 100) {
sStreamLogFile.Write(chunkBuffer, fChunkBufferSize); logFile->Write(chunkBuffer, fChunkBufferSize);
printf("wrote %ld bytes\n", fChunkBufferSize); printf("wrote %ld bytes\n", fChunkBufferSize);
sDumpedPackets++; sDumpedPackets++;
} else if (sDumpedPackets == 100) } else if (sDumpedPackets == 100)
sStreamLogFile.Unset(); logFile->Unset();
#endif #endif
return B_OK; return B_OK;
} }
/*! \brief Copies a chunk into fVideoChunkBuffer and adds a "safety net" of /*! \brief Copies a chunk into fChunkBuffer and adds a "safety net" of
additional memory as required by FFMPEG for input buffers to video additional memory as required by FFMPEG for input buffers to video
decoders. decoders.
@ -1385,11 +1319,11 @@ AVCodecDecoder::_LoadNextVideoChunkIfNeededAndAssignStartTime()
The additional memory has a size of FF_INPUT_BUFFER_PADDING_SIZE as defined The additional memory has a size of FF_INPUT_BUFFER_PADDING_SIZE as defined
in avcodec.h. in avcodec.h.
Ownership of fVideoChunkBuffer memory is with the class so it needs to be Ownership of fChunkBuffer memory is with the class so it needs to be freed
freed at the right times (on destruction, on seeking). at the right times (on destruction, on seeking).
Also update fChunkBufferSize to reflect the size of the contained video Also update fChunkBufferSize to reflect the size of the contained data
data (leaving out the padding). (leaving out the padding).
\param chunk The chunk to copy. \param chunk The chunk to copy.
\param chunkSize Size of the chunk in bytes \param chunkSize Size of the chunk in bytes
@ -1397,25 +1331,22 @@ AVCodecDecoder::_LoadNextVideoChunkIfNeededAndAssignStartTime()
\returns B_OK Padding was successful. You are responsible for releasing the \returns B_OK Padding was successful. You are responsible for releasing the
allocated memory. fChunkBufferSize is set to chunkSize. allocated memory. fChunkBufferSize is set to chunkSize.
\returns B_NO_MEMORY Padding failed. \returns B_NO_MEMORY Padding failed.
fVideoChunkBuffer is set to NULL making it safe to call free() on it. fChunkBuffer is set to NULL making it safe to call free() on it.
fChunkBufferSize is set to 0 to reflect the size of fVideoChunkBuffer. fChunkBufferSize is set to 0 to reflect the size of fChunkBuffer.
*/ */
status_t status_t
AVCodecDecoder::_CopyChunkToVideoChunkBufferAndAddPadding(const void* chunk, AVCodecDecoder::_CopyChunkToChunkBufferAndAddPadding(const void* chunk,
size_t chunkSize) size_t chunkSize)
{ {
// TODO: Rename fVideoChunkBuffer to fChunkBuffer, once the audio path is fChunkBuffer = static_cast<uint8_t*>(realloc(fChunkBuffer,
// responsible for releasing the chunk buffer, too.
fVideoChunkBuffer = static_cast<uint8_t*>(realloc(fVideoChunkBuffer,
chunkSize + FF_INPUT_BUFFER_PADDING_SIZE)); chunkSize + FF_INPUT_BUFFER_PADDING_SIZE));
if (fVideoChunkBuffer == NULL) { if (fChunkBuffer == NULL) {
fChunkBufferSize = 0; fChunkBufferSize = 0;
return B_NO_MEMORY; return B_NO_MEMORY;
} }
memcpy(fVideoChunkBuffer, chunk, chunkSize); memcpy(fChunkBuffer, chunk, chunkSize);
memset(fVideoChunkBuffer + chunkSize, 0, FF_INPUT_BUFFER_PADDING_SIZE); memset(fChunkBuffer + chunkSize, 0, FF_INPUT_BUFFER_PADDING_SIZE);
// Establish safety net, by zero'ing the padding area. // Establish safety net, by zero'ing the padding area.
fChunkBufferSize = chunkSize; fChunkBufferSize = chunkSize;

View File

@ -67,21 +67,14 @@ private:
void _CheckAndFixConditionsThatHintAtBrokenAudioCodeBelow(); void _CheckAndFixConditionsThatHintAtBrokenAudioCodeBelow();
void _MoveAudioFramesToRawDecodedAudioAndUpdateStartTimes(); void _MoveAudioFramesToRawDecodedAudioAndUpdateStartTimes();
status_t _DecodeNextAudioFrameChunk(); status_t _DecodeNextAudioFrameChunk();
status_t _LoadNextAudioChunkIfNeededAndAssignStartTime();
status_t _DecodeSomeAudioFramesIntoEmptyDecodedDataBuffer(); status_t _DecodeSomeAudioFramesIntoEmptyDecodedDataBuffer();
void _UpdateMediaHeaderForAudioFrame(); void _UpdateMediaHeaderForAudioFrame();
status_t _DecodeNextVideoFrame(); status_t _DecodeNextVideoFrame();
void _ApplyEssentialVideoContainerPropertiesToContext(); void _ApplyEssentialVideoContainerPropertiesToContext();
status_t _LoadNextVideoChunkIfNeededAndAssignStartTime(); status_t _LoadNextChunkIfNeededAndAssignStartTime();
// TODO: Remove the "Video" word once status_t _CopyChunkToChunkBufferAndAddPadding(const void* chunk,
// the audio path is responsible for size_t chunkSize);
// freeing the chunk buffer, too.
status_t _CopyChunkToVideoChunkBufferAndAddPadding(
const void* chunk, size_t chunkSize);
// TODO: Remove the "Video" word once
// the audio path is responsible for
// freeing the chunk buffer, too.
void _HandleNewVideoFrameAndUpdateSystemState(); void _HandleNewVideoFrameAndUpdateSystemState();
status_t _FlushOneVideoFrameFromDecoderBuffer(); status_t _FlushOneVideoFrameFromDecoderBuffer();
void _UpdateMediaHeaderForVideoFrame(); void _UpdateMediaHeaderForVideoFrame();
@ -121,12 +114,7 @@ private:
int fOutputFrameSize; int fOutputFrameSize;
// sample size * channel count // sample size * channel count
const void* fChunkBuffer; uint8_t* fChunkBuffer;
uint8_t* fVideoChunkBuffer;
// TODO: Remove and use fChunkBuffer again
// (with type uint8_t*) once the audio path
// is responsible for freeing the chunk
// buffer, too.
size_t fChunkBufferSize; size_t fChunkBufferSize;
bool fAudioDecodeError; bool fAudioDecodeError;