[haiku-commits] haiku: hrev47629 - src/add-ons/media/plugins/ffmpeg
- From: coling@xxxxxx
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Wed, 6 Aug 2014 14:35:52 +0200 (CEST)
hrev47629 adds 3 changesets to branch 'master'
old head: cb721c596392cbbaa63137ea292dc928a07992af
new head: 6063c02ef91c615f2fa28468f16c40b9b8ba1164
overview: http://cgit.haiku-os.org/haiku/log/?qt=range&q=6063c02+%5Ecb721c5
----------------------------------------------------------------------------
2d83b84: FFMPEG Plugin: Implement video input buffer padding.
- Padding is required by FFMPEG for correct operation of all video 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.
- Note: Padding is required for audio decoders, too. I will tackle this in
some
later commits. For the time being we have a degradation in code reuse, due
to
different memory ownership of chunk buffers in audio and video decoder path.
Audio path must not care about freeing chunk buffers whereas video path
must.
- Fix coding style and some typos.
- Update documentation accordingly.
6defcb6: FFMPEG Plugin: Refactor out loading next video chunk.
- Main reason for refactoring was to increase readability of
_DecodeNextVideoFrame() by simplifying it. Refactoring was tested
successfully for no functional change with mpeg2_decoder_test.
- Reindented the method definition in the header file so that the new method
_LoadNextVideoChunkIfNeededAndUpdateStartTime() fits into 80 chars per line.
Reindentdation is applied to methods only as the member variables have no
space left for reindentation.
- Update documentation accordingly.
- Fix wording of audio part to audio path.
- No functional change intended.
6063c02: FFMPEG Plugin: Fix bug and refactor input buffer padding.
- Fixes a bug using realloc with a memory area that is declared const which
lead to a crash in MediaPlayer playing big_buck_bunny_720p_stereo.ogg.
- The refactoring introduces a strict separation between const memory areas
(chunk data read from GetNextChunk()) and mutable memory areas
(fVideoChunkBuffer) by using a copy operation instead of a casted
assignment operation.
- Updated documentation accordingly.
- Besides fixing the bug, there is no functional change intended.
[ Colin Günther <coling@xxxxxx> ]
----------------------------------------------------------------------------
2 files changed, 203 insertions(+), 99 deletions(-)
.../media/plugins/ffmpeg/AVCodecDecoder.cpp | 227 +++++++++++++------
.../media/plugins/ffmpeg/AVCodecDecoder.h | 75 +++---
############################################################################
Commit: 2d83b8419cb16ba1733ebf42432ea5b953b5eee7
URL: http://cgit.haiku-os.org/haiku/commit/?id=2d83b84
Author: Colin Günther <coling@xxxxxx>
Date: Mon Aug 4 22:04:27 2014 UTC
FFMPEG Plugin: Implement video input buffer padding.
- Padding is required by FFMPEG for correct operation of all video 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.
- Note: Padding is required for audio decoders, too. I will tackle this in some
later commits. For the time being we have a degradation in code reuse, due to
different memory ownership of chunk buffers in audio and video decoder path.
Audio path must not care about freeing chunk buffers whereas video path must.
- Fix coding style and some typos.
- Update documentation accordingly.
----------------------------------------------------------------------------
diff --git a/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.cpp
b/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.cpp
index bef2586..13618eb 100644
--- a/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.cpp
+++ b/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.cpp
@@ -106,6 +106,7 @@ AVCodecDecoder::AVCodecDecoder()
fOutputFrameSize(0),
fChunkBuffer(NULL),
+ fVideoChunkBuffer(NULL),
fChunkBufferOffset(0),
fChunkBufferSize(0),
fAudioDecodeError(false),
@@ -140,6 +141,9 @@ AVCodecDecoder::~AVCodecDecoder()
if (fCodecInitDone)
avcodec_close(fContext);
+ free(fVideoChunkBuffer);
+ // TODO: Replace with fChunkBuffer, once audio part is
+ // responsible for freeing the chunk buffer, too.
free(fDecodedData);
av_free(fPostProcessedDecodedPicture);
@@ -260,6 +264,9 @@ AVCodecDecoder::SeekedTo(int64 frame, bigtime_t time)
}
// Flush internal buffers as well.
+ free(fVideoChunkBuffer);
+ // TODO: Replace with fChunkBuffer, once audio part is
+ // responsible for freeing the chunk buffer, too.
fChunkBuffer = NULL;
fChunkBufferOffset = 0;
fChunkBufferSize = 0;
@@ -721,7 +728,10 @@ AVCodecDecoder::_DecodeVideo(void* outBuffer, int64*
outFrameCount,
On first call, the member variables fSwsContext /
fFormatConversionFunc
are initialized.
- \return B_OK, when we successfully decoded one video frame
+ \returns B_OK when we successfully decoded one video frame
+ \returns B_LAST_BUFFER_ERROR when there are no more video frames
available.
+ \returns B_NO_MEMORY when we have no memory left for correct operation.
+ \returns Other Errors
*/
status_t
AVCodecDecoder::_DecodeNextVideoFrame()
@@ -733,7 +743,8 @@ AVCodecDecoder::_DecodeNextVideoFrame()
if (fTempPacket.size == 0) {
// Our packet buffer is empty, so fill it now.
- status_t getNextChunkStatus =
GetNextChunk(&fChunkBuffer,
+ const void* chunkBuffer = NULL;
+ status_t getNextChunkStatus = GetNextChunk(&chunkBuffer,
&fChunkBufferSize, &chunkMediaHeader);
switch (getNextChunkStatus) {
case B_OK:
@@ -743,17 +754,31 @@ AVCodecDecoder::_DecodeNextVideoFrame()
return
_FlushOneVideoFrameFromDecoderBuffer();
default:
-
TRACE("AVCodecDecoder::_DecodeNextVideoFrame(): error from "
- "GetNextChunk(): %s\n",
strerror(err));
+
TRACE("AVCodecDecoder::_DecodeNextVideoFrame(): error from"
+ " GetNextChunk(): %s\n",
strerror(err));
return getNextChunkStatus;
}
- fTempPacket.data =
static_cast<uint8_t*>(const_cast<void*>(
- fChunkBuffer));
+ free(fVideoChunkBuffer);
+ // Free any previously used chunk buffer first
+ // TODO: Replace with fChunkBuffer, once audio
part is
+ // responsible for freeing the chunk buffer,
too.
+ fVideoChunkBuffer
+ =
static_cast<uint8_t*>(const_cast<void*>(chunkBuffer));
+ // TODO: Replace fVideoChunkBuffer with
fChunkBuffer, once
+ // audio part is responsible for freeing the
chunk buffer, too.
+ status_t chunkBufferPaddingStatus
+ = _AddInputBufferPaddingToVideoChunkBuffer();
+ if (chunkBufferPaddingStatus != B_OK)
+ return chunkBufferPaddingStatus;
+
+ fTempPacket.data = fVideoChunkBuffer;
+ // TODO: Replace with fChunkBuffer, once audio
part is
+ // responsible for freeing the chunk buffer,
too.
fTempPacket.size = fChunkBufferSize;
fContext->reordered_opaque =
chunkMediaHeader.start_time;
- // Let ffmpeg handle the relationship between
start_time and
+ // Let FFMPEG handle the relationship between
start_time and
// decoded video frame.
//
// Explanation:
@@ -779,7 +804,7 @@ AVCodecDecoder::_DecodeNextVideoFrame()
#ifdef LOG_STREAM_TO_FILE
if (sDumpedPackets < 100) {
- sStreamLogFile.Write(fChunkBuffer,
fChunkBufferSize);
+ sStreamLogFile.Write(chunkBuffer,
fChunkBufferSize);
printf("wrote %ld bytes\n", fChunkBufferSize);
sDumpedPackets++;
} else if (sDumpedPackets == 100)
@@ -857,6 +882,39 @@ AVCodecDecoder::_DecodeNextVideoFrame()
}
+/*! \brief Adds a "safety net" of additional memory to fVideoChunkBuffer as
+ required by FFMPEG for input buffers to video decoders.
+
+ This is needed so that some decoders can read safely a predefined
number of
+ bytes at a time for performance optimization purposes.
+
+ The additonal memory has a size of FF_INPUT_BUFFER_PADDING_SIZE as
defined
+ in avcodec.h.
+
+ \returns B_OK Padding was successful. You are responsible for releasing
the
+ allocated memory.
+ \returns B_NO_MEMORY Padding failed. The memory in fVideoChunkBuffer is
not
+ freed. NULL is assigned to fVideoChunkBuffer.
+*/
+status_t
+AVCodecDecoder::_AddInputBufferPaddingToVideoChunkBuffer()
+{
+ // TODO: Rename fVideoChunkBuffer to fChunkBuffer, once the audio part
is
+ // responsible for releasing the chunk buffer, too.
+ fVideoChunkBuffer
+ =
static_cast<uint8_t*>(realloc(static_cast<void*>(fVideoChunkBuffer),
+ fChunkBufferSize + FF_INPUT_BUFFER_PADDING_SIZE));
+ if (fVideoChunkBuffer == NULL)
+ return B_NO_MEMORY;
+
+ memset(fVideoChunkBuffer + fChunkBufferSize, 0,
+ FF_INPUT_BUFFER_PADDING_SIZE);
+ // Establish safety net, by zero'ing the padding area.
+
+ return B_OK;
+}
+
+
/*! \brief Executes all steps needed for a freshly decoded video frame.
\see _UpdateMediaHeaderForVideoFrame() and
diff --git a/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.h
b/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.h
index 13e0bc2..c9a8b11 100644
--- a/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.h
+++ b/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.h
@@ -66,6 +66,10 @@ private:
media_header* mediaHeader,
media_decode_info* info);
status_t _DecodeNextVideoFrame();
+ status_t
_AddInputBufferPaddingToVideoChunkBuffer();
+ //
TODO: Remove the "Video" word once
+ // the
audio part is responsible for
+ //
freeing the chunk buffer, too.
void
_HandleNewVideoFrameAndUpdateSystemState();
status_t
_FlushOneVideoFrameFromDecoderBuffer();
void
_UpdateMediaHeaderForVideoFrame();
@@ -106,6 +110,11 @@ private:
//
sample size * channel count
const void* fChunkBuffer;
+ uint8_t* fVideoChunkBuffer;
+ //
TODO: Remove and use fChunkBuffer again
+ //
(with type uint8_t*) once the audio part is
+ //
responsible for freeing the chunk buffer,
+ // too.
int32 fChunkBufferOffset;
size_t fChunkBufferSize;
bool fAudioDecodeError;
############################################################################
Commit: 6defcb6c6dcc4175fd38477777c369416c0ca498
URL: http://cgit.haiku-os.org/haiku/commit/?id=6defcb6
Author: Colin Günther <coling@xxxxxx>
Date: Tue Aug 5 15:24:11 2014 UTC
FFMPEG Plugin: Refactor out loading next video chunk.
- Main reason for refactoring was to increase readability of
_DecodeNextVideoFrame() by simplifying it. Refactoring was tested
successfully for no functional change with mpeg2_decoder_test.
- Reindented the method definition in the header file so that the new method
_LoadNextVideoChunkIfNeededAndUpdateStartTime() fits into 80 chars per line.
Reindentdation is applied to methods only as the member variables have no
space left for reindentation.
- Update documentation accordingly.
- Fix wording of audio part to audio path.
- No functional change intended.
----------------------------------------------------------------------------
diff --git a/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.cpp
b/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.cpp
index 13618eb..ebac43c 100644
--- a/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.cpp
+++ b/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.cpp
@@ -142,7 +142,7 @@ AVCodecDecoder::~AVCodecDecoder()
avcodec_close(fContext);
free(fVideoChunkBuffer);
- // TODO: Replace with fChunkBuffer, once audio part is
+ // TODO: Replace with fChunkBuffer, once audio path is
// responsible for freeing the chunk buffer, too.
free(fDecodedData);
@@ -265,7 +265,7 @@ AVCodecDecoder::SeekedTo(int64 frame, bigtime_t time)
// Flush internal buffers as well.
free(fVideoChunkBuffer);
- // TODO: Replace with fChunkBuffer, once audio part is
+ // TODO: Replace with fChunkBuffer, once audio path is
// responsible for freeing the chunk buffer, too.
fChunkBuffer = NULL;
fChunkBufferOffset = 0;
@@ -687,7 +687,7 @@ AVCodecDecoder::_DecodeVideo(void* outBuffer, int64*
outFrameCount,
}
-/*! \brief Decode next video frame
+/*! \brief Decodes next video frame.
We decode exactly one video frame into fDecodedData. To achieve this goal,
we might need to request several chunks of encoded data resulting in a
@@ -702,7 +702,7 @@ AVCodecDecoder::_DecodeVideo(void* outBuffer, int64*
outFrameCount,
To every decoded video frame there is a media_header populated in
fHeader, containing the corresponding video frame properties.
-
+
Normally every decoded video frame has a start_time field populated in
the
associated fHeader, that determines the presentation time of the frame.
This relationship will only hold true, when each data chunk that is
@@ -725,8 +725,8 @@ AVCodecDecoder::_DecodeVideo(void* outBuffer, int64*
outFrameCount,
More over the fOutputFrameRate variable is updated for every decoded video
frame.
- On first call, the member variables fSwsContext /
fFormatConversionFunc
- are initialized.
+ On first call the member variables fSwsContext / fFormatConversionFunc
are
+ initialized.
\returns B_OK when we successfully decoded one video frame
\returns B_LAST_BUFFER_ERROR when there are no more video frames
available.
@@ -739,77 +739,16 @@ AVCodecDecoder::_DecodeNextVideoFrame()
assert(fTempPacket.size >= 0);
while (true) {
- media_header chunkMediaHeader;
-
- if (fTempPacket.size == 0) {
- // Our packet buffer is empty, so fill it now.
- const void* chunkBuffer = NULL;
- status_t getNextChunkStatus = GetNextChunk(&chunkBuffer,
- &fChunkBufferSize, &chunkMediaHeader);
- switch (getNextChunkStatus) {
- case B_OK:
- break;
-
- case B_LAST_BUFFER_ERROR:
- return
_FlushOneVideoFrameFromDecoderBuffer();
-
- default:
-
TRACE("AVCodecDecoder::_DecodeNextVideoFrame(): error from"
- " GetNextChunk(): %s\n",
strerror(err));
- return getNextChunkStatus;
- }
+ status_t loadingChunkStatus
+ = _LoadNextVideoChunkIfNeededAndUpdateStartTime();
- free(fVideoChunkBuffer);
- // Free any previously used chunk buffer first
- // TODO: Replace with fChunkBuffer, once audio
part is
- // responsible for freeing the chunk buffer,
too.
- fVideoChunkBuffer
- =
static_cast<uint8_t*>(const_cast<void*>(chunkBuffer));
- // TODO: Replace fVideoChunkBuffer with
fChunkBuffer, once
- // audio part is responsible for freeing the
chunk buffer, too.
- status_t chunkBufferPaddingStatus
- = _AddInputBufferPaddingToVideoChunkBuffer();
- if (chunkBufferPaddingStatus != B_OK)
- return chunkBufferPaddingStatus;
-
- fTempPacket.data = fVideoChunkBuffer;
- // TODO: Replace with fChunkBuffer, once audio
part is
- // responsible for freeing the chunk buffer,
too.
- fTempPacket.size = fChunkBufferSize;
-
- fContext->reordered_opaque =
chunkMediaHeader.start_time;
- // Let FFMPEG handle the relationship between
start_time and
- // decoded video frame.
- //
- // Explanation:
- // The received chunk buffer may not contain
the next video
- // frame to be decoded, due to frame reordering
(e.g. MPEG1/2
- // provides encoded video frames in a different
order than the
- // decoded video frame).
- //
- // FIXME: Research how to establish a
meaningful relationship
- // between start_time and decoded video 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
- // video data to test it, it makes no sense to
implement it.
- //
- // FIXME: Implement tracking start_time of
video frames
- // originating in data chunks that encode more
than one video
- // frame at a time. In that case on would
increment the
- // start_time for each consecutive frame of
such a data chunk
- // (like it is done for audio frame decoding).
But as long as
- // I don't have such video data to test it, it
makes no sense
- // to implement it.
+ if (loadingChunkStatus == B_LAST_BUFFER_ERROR)
+ return _FlushOneVideoFrameFromDecoderBuffer();
-#ifdef LOG_STREAM_TO_FILE
- if (sDumpedPackets < 100) {
- sStreamLogFile.Write(chunkBuffer,
fChunkBufferSize);
- printf("wrote %ld bytes\n", fChunkBufferSize);
- sDumpedPackets++;
- } else if (sDumpedPackets == 100)
- sStreamLogFile.Unset();
-#endif
+ if (loadingChunkStatus != B_OK) {
+ TRACE("AVCodecDecoder::_DecodeNextVideoFrame(): error
from "
+ "GetNextChunk(): %s\n", strerror(err));
+ return loadingChunkStatus;
}
#if DO_PROFILING
@@ -822,9 +761,9 @@ AVCodecDecoder::_DecodeNextVideoFrame()
// packet buffer is allowed to contain incomplete frames so we
are
// required to buffer the packets between different calls to
// _DecodeNextVideoFrame().
- int gotPicture = 0;
+ int gotVideoFrame = 0;
int decodedDataSizeInBytes = avcodec_decode_video2(fContext,
- fRawDecodedPicture, &gotPicture, &fTempPacket);
+ fRawDecodedPicture, &gotVideoFrame, &fTempPacket);
if (decodedDataSizeInBytes < 0) {
TRACE("[v] AVCodecDecoder: ignoring error in decoding
frame %lld:"
" %d\n", fFrame, len);
@@ -839,8 +778,8 @@ AVCodecDecoder::_DecodeNextVideoFrame()
fTempPacket.size -= decodedDataSizeInBytes;
fTempPacket.data += decodedDataSizeInBytes;
- bool gotNoPictureYet = gotPicture == 0;
- if (gotNoPictureYet) {
+ bool gotNoVideoFrame = gotVideoFrame == 0;
+ if (gotNoVideoFrame) {
TRACE("frame %lld - no picture yet,
decodedDataSizeInBytes: %d, "
"chunk size: %ld\n", fFrame,
decodedDataSizeInBytes,
fChunkBufferSize);
@@ -882,6 +821,91 @@ AVCodecDecoder::_DecodeNextVideoFrame()
}
+/*! \brief Loads the next video chunk into fVideoChunkBuffer and assigns it to
+ fTempPacket accordingly only if fTempPacket is empty. Updates
fContext
+ with the start time of the new data chunk.
+
+ \returns B_OK
+ 1. meaning: Next video chunk is loaded and fContext is updated.
+ 2. meaning: No need to load and update anything. Proceed as
usual.
+ \returns B_LAST_BUFFER_ERROR No more video chunks available.
+ fVideoChunkBuffer, fTempPacket and fContext are left untouched.
+ \returns Other errors Caller should bail out because fVideoChunkBuffer,
+ fTempPacket and fContext are in unknown states. Normal
operation cannot
+ be guaranteed.
+*/
+status_t
+AVCodecDecoder::_LoadNextVideoChunkIfNeededAndUpdateStartTime()
+{
+ // TODO: Rename fVideoChunkBuffer to fChunkBuffer, once the audio path
is
+ // responsible for releasing the chunk buffer, too.
+
+ 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;
+
+ fChunkBufferSize = chunkBufferSize;
+
+ free(fVideoChunkBuffer);
+ // Free any previously used chunk buffer first
+ fVideoChunkBuffer
+ = static_cast<uint8_t*>(const_cast<void*>(chunkBuffer));
+ status_t chunkBufferPaddingStatus
+ = _AddInputBufferPaddingToVideoChunkBuffer();
+ if (chunkBufferPaddingStatus != B_OK)
+ return chunkBufferPaddingStatus;
+
+ fTempPacket.data = fVideoChunkBuffer;
+ fTempPacket.size = fChunkBufferSize;
+
+ fContext->reordered_opaque = chunkMediaHeader.start_time;
+ // Let FFMPEG handle the relationship between start_time and
+ // decoded video frame.
+ //
+ // Explanation:
+ // The received chunk buffer may not contain the next video
+ // frame to be decoded, due to frame reordering (e.g. MPEG1/2
+ // provides encoded video frames in a different order than the
+ // decoded video frame).
+ //
+ // FIXME: Research how to establish a meaningful relationship
+ // between start_time and decoded video 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
+ // video data to test it, it makes no sense to implement it.
+ //
+ // FIXME: Implement tracking start_time of video frames
+ // originating in data chunks that encode more than one video
+ // frame at a time. In that case on would increment the
+ // start_time for each consecutive frame of such a data chunk
+ // (like it is done for audio frame decoding). But as long as
+ // I don't have such video data to test it, it makes no sense
+ // to implement it.
+
+#ifdef LOG_STREAM_TO_FILE
+ if (sDumpedPackets < 100) {
+ sStreamLogFile.Write(chunkBuffer, fChunkBufferSize);
+ printf("wrote %ld bytes\n", fChunkBufferSize);
+ sDumpedPackets++;
+ } else if (sDumpedPackets == 100)
+ sStreamLogFile.Unset();
+#endif
+
+ return B_OK;
+}
+
+
/*! \brief Adds a "safety net" of additional memory to fVideoChunkBuffer as
required by FFMPEG for input buffers to video decoders.
@@ -899,7 +923,7 @@ AVCodecDecoder::_DecodeNextVideoFrame()
status_t
AVCodecDecoder::_AddInputBufferPaddingToVideoChunkBuffer()
{
- // TODO: Rename fVideoChunkBuffer to fChunkBuffer, once the audio part
is
+ // TODO: Rename fVideoChunkBuffer to fChunkBuffer, once the audio path
is
// responsible for releasing the chunk buffer, too.
fVideoChunkBuffer
=
static_cast<uint8_t*>(realloc(static_cast<void*>(fVideoChunkBuffer),
@@ -961,13 +985,14 @@ AVCodecDecoder::_FlushOneVideoFrameFromDecoderBuffer()
fTempPacket.data = NULL;
fTempPacket.size = 0;
- int gotPicture = 0;
- avcodec_decode_video2(fContext, fRawDecodedPicture, &gotPicture,
+ int gotVideoFrame = 0;
+ avcodec_decode_video2(fContext, fRawDecodedPicture, &gotVideoFrame,
&fTempPacket);
// We are only interested in complete frames now, so ignore the
return
// value.
- if (gotPicture == 0) {
+ bool gotNoVideoFrame = gotVideoFrame == 0;
+ if (gotNoVideoFrame) {
// video buffer is flushed successfully
return B_LAST_BUFFER_ERROR;
}
diff --git a/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.h
b/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.h
index c9a8b11..6441a09 100644
--- a/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.h
+++ b/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.h
@@ -28,52 +28,51 @@ extern "C" {
class AVCodecDecoder : public Decoder {
public:
-
AVCodecDecoder();
+ AVCodecDecoder();
- virtual ~AVCodecDecoder();
+ virtual ~AVCodecDecoder();
- virtual void GetCodecInfo(media_codec_info*
mci);
+ virtual void GetCodecInfo(media_codec_info* mci);
- virtual status_t Setup(media_format*
ioEncodedFormat,
- const void*
infoBuffer, size_t infoSize);
+ virtual status_t Setup(media_format* ioEncodedFormat,
+ const void* infoBuffer,
size_t infoSize);
- virtual status_t NegotiateOutputFormat(
-
media_format* inOutFormat);
+ virtual status_t NegotiateOutputFormat(media_format*
inOutFormat);
- virtual status_t Decode(void* outBuffer, int64*
outFrameCount,
-
media_header* mediaHeader,
-
media_decode_info* info);
+ virtual status_t Decode(void* outBuffer, int64* outFrameCount,
+ media_header*
mediaHeader,
+ media_decode_info*
info);
- virtual status_t SeekedTo(int64 trame, bigtime_t
time);
+ virtual status_t SeekedTo(int64 trame, bigtime_t time);
private:
- void _ResetTempPacket();
-
- status_t
_NegotiateAudioOutputFormat(
-
media_format* inOutFormat);
-
- status_t
_NegotiateVideoOutputFormat(
-
media_format* inOutFormat);
-
- status_t _DecodeAudio(void*
outBuffer,
- int64*
outFrameCount,
-
media_header* mediaHeader,
-
media_decode_info* info);
-
- status_t _DecodeVideo(void*
outBuffer,
- int64*
outFrameCount,
-
media_header* mediaHeader,
-
media_decode_info* info);
- status_t _DecodeNextVideoFrame();
- status_t
_AddInputBufferPaddingToVideoChunkBuffer();
- //
TODO: Remove the "Video" word once
- // the
audio part is responsible for
- //
freeing the chunk buffer, too.
- void
_HandleNewVideoFrameAndUpdateSystemState();
- status_t
_FlushOneVideoFrameFromDecoderBuffer();
- void
_UpdateMediaHeaderForVideoFrame();
- void
_DeinterlaceAndColorConvertVideoFrame();
+ void _ResetTempPacket();
+
+ status_t
_NegotiateAudioOutputFormat(media_format* inOutFormat);
+
+ status_t
_NegotiateVideoOutputFormat(media_format* inOutFormat);
+
+ status_t _DecodeAudio(void* outBuffer, int64*
outFrameCount,
+ media_header*
mediaHeader,
+ media_decode_info*
info);
+
+ status_t _DecodeVideo(void* outBuffer, int64*
outFrameCount,
+ media_header*
mediaHeader,
+ media_decode_info*
info);
+ status_t _DecodeNextVideoFrame();
+ status_t
_LoadNextVideoChunkIfNeededAndUpdateStartTime();
+ // TODO: Remove the
"Video" word once
+ // the audio path is
responsible for
+ // freeing the chunk
buffer, too.
+ status_t
_AddInputBufferPaddingToVideoChunkBuffer();
+ // TODO: Remove the
"Video" word once
+ // the audio path is
responsible for
+ // freeing the chunk
buffer, too.
+ void
_HandleNewVideoFrameAndUpdateSystemState();
+ status_t _FlushOneVideoFrameFromDecoderBuffer();
+ void _UpdateMediaHeaderForVideoFrame();
+ void _DeinterlaceAndColorConvertVideoFrame();
media_header fHeader;
@@ -112,7 +111,7 @@ private:
const void* fChunkBuffer;
uint8_t* fVideoChunkBuffer;
//
TODO: Remove and use fChunkBuffer again
- //
(with type uint8_t*) once the audio part is
+ //
(with type uint8_t*) once the audio path is
//
responsible for freeing the chunk buffer,
// too.
int32 fChunkBufferOffset;
############################################################################
Revision: hrev47629
Commit: 6063c02ef91c615f2fa28468f16c40b9b8ba1164
URL: http://cgit.haiku-os.org/haiku/commit/?id=6063c02
Author: Colin Günther <coling@xxxxxx>
Date: Tue Aug 5 22:25:37 2014 UTC
FFMPEG Plugin: Fix bug and refactor input buffer padding.
- Fixes a bug using realloc with a memory area that is declared const which
lead to a crash in MediaPlayer playing big_buck_bunny_720p_stereo.ogg.
- The refactoring introduces a strict separation between const memory areas
(chunk data read from GetNextChunk()) and mutable memory areas
(fVideoChunkBuffer) by using a copy operation instead of a casted
assignment operation.
- Updated documentation accordingly.
- Besides fixing the bug, there is no functional change intended.
----------------------------------------------------------------------------
diff --git a/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.cpp
b/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.cpp
index ebac43c..a7ff620 100644
--- a/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.cpp
+++ b/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.cpp
@@ -267,6 +267,7 @@ AVCodecDecoder::SeekedTo(int64 frame, bigtime_t time)
free(fVideoChunkBuffer);
// TODO: Replace with fChunkBuffer, once audio path is
// responsible for freeing the chunk buffer, too.
+ fVideoChunkBuffer = NULL;
fChunkBuffer = NULL;
fChunkBufferOffset = 0;
fChunkBufferSize = 0;
@@ -854,14 +855,9 @@
AVCodecDecoder::_LoadNextVideoChunkIfNeededAndUpdateStartTime()
if (getNextChunkStatus != B_OK)
return getNextChunkStatus;
- fChunkBufferSize = chunkBufferSize;
-
- free(fVideoChunkBuffer);
- // Free any previously used chunk buffer first
- fVideoChunkBuffer
- = static_cast<uint8_t*>(const_cast<void*>(chunkBuffer));
status_t chunkBufferPaddingStatus
- = _AddInputBufferPaddingToVideoChunkBuffer();
+ = _CopyChunkToVideoChunkBufferAndAddPadding(chunkBuffer,
+ chunkBufferSize);
if (chunkBufferPaddingStatus != B_OK)
return chunkBufferPaddingStatus;
@@ -906,35 +902,51 @@
AVCodecDecoder::_LoadNextVideoChunkIfNeededAndUpdateStartTime()
}
-/*! \brief Adds a "safety net" of additional memory to fVideoChunkBuffer as
- required by FFMPEG for input buffers to video decoders.
+/*! \brief Copies a chunk into fVideoChunkBuffer and adds a "safety net" of
+ additional memory as required by FFMPEG for input buffers to
video
+ decoders.
This is needed so that some decoders can read safely a predefined
number of
bytes at a time for performance optimization purposes.
- The additonal 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.
+ Ownership of fVideoChunkBuffer memory is with the class so it needs to
be
+ freed at the right times (on destruction, on seeking).
+
+ Also update fChunkBufferSize to reflect the size of the contained video
+ data (leaving out the padding).
+
+ \param chunk The chunk to copy.
+ \param chunkSize Size of the chunk in bytes
+
\returns B_OK Padding was successful. You are responsible for releasing
the
- allocated memory.
- \returns B_NO_MEMORY Padding failed. The memory in fVideoChunkBuffer is
not
- freed. NULL is assigned to fVideoChunkBuffer.
+ allocated memory. fChunkBufferSize is set to chunkSize.
+ \returns B_NO_MEMORY Padding failed.
+ fVideoChunkBuffer is set to NULL making it safe to call free()
on it.
+ fChunkBufferSize is set to 0 to reflect the size of
fVideoChunkBuffer.
*/
status_t
-AVCodecDecoder::_AddInputBufferPaddingToVideoChunkBuffer()
+AVCodecDecoder::_CopyChunkToVideoChunkBufferAndAddPadding(const void* chunk,
+ size_t chunkSize)
{
// TODO: Rename fVideoChunkBuffer to fChunkBuffer, once the audio path
is
// responsible for releasing the chunk buffer, too.
- fVideoChunkBuffer
- =
static_cast<uint8_t*>(realloc(static_cast<void*>(fVideoChunkBuffer),
- fChunkBufferSize + FF_INPUT_BUFFER_PADDING_SIZE));
- if (fVideoChunkBuffer == NULL)
+
+ fVideoChunkBuffer = static_cast<uint8_t*>(realloc(fVideoChunkBuffer,
+ chunkSize + FF_INPUT_BUFFER_PADDING_SIZE));
+ if (fVideoChunkBuffer == NULL) {
+ fChunkBufferSize = 0;
return B_NO_MEMORY;
+ }
- memset(fVideoChunkBuffer + fChunkBufferSize, 0,
- FF_INPUT_BUFFER_PADDING_SIZE);
+ memcpy(fVideoChunkBuffer, chunk, chunkSize);
+ memset(fVideoChunkBuffer + chunkSize, 0, FF_INPUT_BUFFER_PADDING_SIZE);
// Establish safety net, by zero'ing the padding area.
+ fChunkBufferSize = chunkSize;
+
return B_OK;
}
diff --git a/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.h
b/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.h
index 6441a09..63ee4f9 100644
--- a/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.h
+++ b/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.h
@@ -65,7 +65,8 @@ private:
// TODO: Remove the
"Video" word once
// the audio path is
responsible for
// freeing the chunk
buffer, too.
- status_t
_AddInputBufferPaddingToVideoChunkBuffer();
+ 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.
Other related posts:
- » [haiku-commits] haiku: hrev47629 - src/add-ons/media/plugins/ffmpeg - coling
