VDR-1.3.26-31: BUG in cVideo/AudioRepacker while syncing
Commit Message
Hi,
while looking at the code of cRepacker::Reset() for adding a comment why
I was initializing variable "packetTodo" with the maximum value for
MPEG2, I've discovered a bug in cVideo/AudioRepacker::Repack():
When resyncing took longer than packetTodo bytes then a result packet
could have been generated with the data collected while syncing, which
actually should have been omited.
The other changes address the BreatAt() methods which shall do nothing
while initially syncing to allow the packet buffer to fill to it's
maximum size before it is repacked.
As a result there is nolonger the need to initialize "packetTodo" to a
certain value.
I sincerely apologize for causing such difficulties and hope that the
code is stable now.
Bye.
Comments
Am 28.08.2005 um 23:02 schrieb Reinhard Nissl:
> MPEG2, I've discovered a bug in cVideo/AudioRepacker::Repack():
With VDR 1.3.31 i have these lines in the log sometimes when doing
recordings:
Aug 29 17:14:45 linvdr user.err vdr[1501]: cAudioRepacker: skipped 59
bytes to sync on next audio frame
Aug 29 17:14:45 linvdr user.err vdr[1501]: cAudioRepacker: skipped 91
bytes to sync on next audio frame
Aug 29 17:14:45 linvdr user.err vdr[1501]: cAudioRepacker: skipped
124 bytes to sync on next audio frame
What does this mean? Are the recordings now corrupted? What went wrong?
Ciao, Dominique
Hi,
Dominique Simon wrote:
>> MPEG2, I've discovered a bug in cVideo/AudioRepacker::Repack():
>
> With VDR 1.3.31 i have these lines in the log sometimes when doing
> recordings:
>
> Aug 29 17:14:45 linvdr user.err vdr[1501]: cAudioRepacker: skipped 59
> bytes to sync on next audio frame
> Aug 29 17:14:45 linvdr user.err vdr[1501]: cAudioRepacker: skipped 91
> bytes to sync on next audio frame
> Aug 29 17:14:45 linvdr user.err vdr[1501]: cAudioRepacker: skipped 124
> bytes to sync on next audio frame
>
> What does this mean? Are the recordings now corrupted? What went wrong?
Well, let's first assume that there is no further bug left in
cAudioRepacker.
Then this means, that there was a little distortion of the received data
which had an influence on 3 audio frames. These broken frames didn't
make it into the recording, as no decoder would be able to play them.
So, your recording is not currupted, but it misses 3 (non decodeable)
audio frames. A decoder will be going to fill these gaps (depending on
bit and sample rate, each about 24 ms) either with silence or by
repeating the previous audio frame.
Please keep in mind, that broadcasting is a one way data transmission.
When an error happens on the transmission channel which cannot be fixed
by the correction logic, then you'll be going to record it. The only
difference to VDR-1.3.30 is that you now get informed about this issue.
Bye.
@@ -142,7 +142,7 @@ void cCommonRepacker::Reset(void)
{
cRepacker::Reset();
skippedBytes = 0;
- packetTodo = maxPacketSize - 6 - 3;
+ packetTodo = 0;
fragmentLen = 0;
pesHeaderLen = 0;
pesHeaderBackupLen = 0;
@@ -361,7 +361,7 @@ void cVideoRepacker::Repack(cRingBufferL
done++;
todo--;
// do we have to start a new packet as there is no more space left?
- if (--packetTodo <= 0) {
+ if (state != syncing && --packetTodo <= 0) {
// we connot start a new packet here if the current might end in a start
// code and this start code shall possibly be put in the next packet. So
// overfill the current packet until we can safely detect that we won't
@@ -468,6 +468,9 @@ void cVideoRepacker::Repack(cRingBufferL
int cVideoRepacker::BreakAt(const uchar *Data, int Count)
{
+ if (initiallySyncing)
+ return -1; // fill the packet buffer completely until we have synced once
+
int PesPayloadOffset = 0;
if (AnalyzePesHeader(Data, Count, PesPayloadOffset) <= phInvalid)
@@ -732,7 +735,7 @@ void cAudioRepacker::Repack(cRingBufferL
done++;
todo--;
// do we have to start a new packet as there is no more space left?
- if (--packetTodo <= 0) {
+ if (state != syncing && --packetTodo <= 0) {
// We connot start a new packet here if the current might end in an audio
// frame header and this header shall possibly be put in the next packet. So
// overfill the current packet until we can safely detect that we won't
@@ -836,6 +839,9 @@ void cAudioRepacker::Repack(cRingBufferL
int cAudioRepacker::BreakAt(const uchar *Data, int Count)
{
+ if (initiallySyncing)
+ return -1; // fill the packet buffer completely until we have synced once
+
int PesPayloadOffset = 0;
ePesHeader MpegLevel = AnalyzePesHeader(Data, Count, PesPayloadOffset);
@@ -1189,6 +1195,9 @@ void cDolbyRepacker::Repack(cRingBufferL
int cDolbyRepacker::BreakAt(const uchar *Data, int Count)
{
+ if (initiallySyncing)
+ return -1; // fill the packet buffer completely until we have synced once
+
// enough data for test?
if (Count < 6 + 3)
return -1;