Hi!
The newest firmware for FF cards did not completely fix the AV desync problems for me. According to information from Werner the problem happens when small video frames fill the decoder buffer with over 2 seconds of data. So I made this patch for dvbplayer.c to stop it from uploading more PES frames to decoder when STC/PTS difference is more than 2 seconds. This seems to fix the remaining problems for me, but I have not tested it much. The PTS/STC-code has been mostly taken from the dvb-subtitles plugin. Comments, please
Ville
--- dvbplayer.c.orig 2007-01-31 18:21:42.000000000 +0200 +++ dvbplayer.c 2007-01-31 18:35:36.000000000 +0200 @@ -495,6 +495,51 @@ } } if (p) { + if(playMode == pmPlay && pc > 13) + { + int64_t stc = -1; + int64_t pts = -1; + + if ( p[0] == 0x00 && p[1] == 0x00 && p[2] == 0x01) + { + if(p[7] & 0x80) + { + switch( p[3] ) + { + case 0xE0 ... 0xEF: // video + case 0xC0 ... 0xDF: // audio + pts = (int64_t) (p[ 9] & 0x0E) << 29 ; + pts |= (int64_t) p[ 10] << 22 ; + pts |= (int64_t) (p[ 11] & 0xFE) << 14 ; + pts |= (int64_t) p[ 12] << 7 ; + pts |= (int64_t) (p[ 13] & 0xFE) >> 1 ; + } + } + } + if(pts != -1) + { + cDevice *pd = cDevice::PrimaryDevice(); + if(pd) + { + stc = pd->GetSTC(); + if(stc != 0) + { + if(pts & (int64_t)1<<32) + { + stc |= (int64_t)1<<32; + } + int64_t timeDiff = (pts-stc); + if (pts<stc) + { + timeDiff += (int64_t)1<<33; + } + timeDiff /= 90; + if(timeDiff > 2000) + cCondWait::SleepMs(timeDiff - 2000); + } + } + } + } int w = PlayPes(p, pc, playMode != pmPlay); if (w > 0) { p += w;
Ville Rannikko wrote:
Hi!
The newest firmware for FF cards did not completely fix the AV desync problems for me. According to information from Werner the problem happens when small video frames fill the decoder buffer with over 2 seconds of data. So I made this patch for dvbplayer.c to stop it from uploading more PES frames to decoder when STC/PTS difference is more than 2 seconds. This seems to fix the remaining problems for me, but I have not tested it much. The PTS/STC-code has been mostly taken from the dvb-subtitles plugin. Comments, please
While this may actually help in your case, I'm not particularly fond of this. The cDvbPlayer shouldn't have to worry about this. It takes care that data is sent to the player device fast enough to avoid underruns, but that's all it should have to care about. It's the device's (driver and firmware) job to play the data correctly.
Klaus
On 1/31/07, Klaus Schmidinger Klaus.Schmidinger@cadsoft.de wrote:
Ville Rannikko wrote:
Hi!
The newest firmware for FF cards did not completely fix the AV desync problems for me. According to information from Werner the problem happens when small video frames fill the decoder buffer with over 2 seconds of data. So I made this patch for dvbplayer.c to stop it from uploading more PES frames to decoder when STC/PTS difference is more than 2 seconds. This seems to fix the remaining problems for me, but I have not tested it much. The PTS/STC-code has been mostly taken from the dvb-subtitles plugin. Comments, please
While this may actually help in your case, I'm not particularly fond of this. The cDvbPlayer shouldn't have to worry about this. It takes care that data is sent to the player device fast enough to avoid underruns, but that's all it should have to care about. It's the device's (driver and firmware) job to play the data correctly.
From what he's saying, the problem is buffer overrun's, not underrun's. Too
much data is being sent and the device isn't able to keep up. If that's the case then it would make sense for vdr to have a user setting to limit how many seconds (or milliseconds perhaps?) worth of data is sent to the buffer. I can't think of any reason not to add such a feature if it means better usability for the end-user.
That's my opinion anyways.
VDR User wrote:
On 1/31/07, *Klaus Schmidinger* <Klaus.Schmidinger@cadsoft.de mailto:Klaus.Schmidinger@cadsoft.de> wrote:
Ville Rannikko wrote: > Hi! > > The newest firmware for FF cards did not completely fix the AV desync > problems for me. According to information from Werner the problem > happens when small video frames fill the decoder buffer with over 2 > seconds of data. So I made this patch for dvbplayer.c to stop it from > uploading more PES frames to decoder when STC/PTS difference is more > than 2 seconds. This seems to fix the remaining problems for me, but I > have not tested it much. The PTS/STC-code has been mostly taken from the > dvb-subtitles plugin. Comments, please While this may actually help in your case, I'm not particularly fond of this. The cDvbPlayer shouldn't have to worry about this. It takes care that data is sent to the player device fast enough to avoid underruns, but that's all it should have to care about. It's the device's (driver and firmware) job to play the data correctly.From what he's saying, the problem is buffer overrun's, not underrun's. Too much data is being sent and the device isn't able to keep up. If that's the case then it would make sense for vdr to have a user setting to limit how many seconds (or milliseconds perhaps?) worth of data is sent to the buffer. I can't think of any reason not to add such a feature if it means better usability for the end-user.
If the device can't take any more data, it should just refuse to accept it and return from the write() call without anything written.
Klaus
On 1/31/07, Klaus Schmidinger Klaus.Schmidinger@cadsoft.de wrote:
VDR User wrote:
From what he's saying, the problem is buffer overrun's, not underrun's. Too much data is being sent and the device isn't able to keep up. If that's the case then it would make sense for vdr to have a user setting to limit how many seconds (or milliseconds perhaps?) worth of data is sent to the buffer. I can't think of any reason not to add such a feature if it means better usability for the end-user.
If the device can't take any more data, it should just refuse to accept it and return from the write() call without anything written.
I disagree. Simply returning from write() implies the data was written and the device is ready for more. If this is not true then you risk making the sync even worse. I consulted with people familiar with this type of stuff and it was suggested the problem could (and should) be fixed at the driver level so I'm following up on that now. Hopefully we'll find a good (or at least resonable) solution to this last little bit of a/v sync frustration.
VDR User wrote:
On 1/31/07, *Klaus Schmidinger* <Klaus.Schmidinger@cadsoft.de mailto:Klaus.Schmidinger@cadsoft.de> wrote:
VDR User wrote: > From what he's saying, the problem is buffer overrun's, not underrun's. > Too much data is being sent and the device isn't able to keep up. If > that's the case then it would make sense for vdr to have a user setting > to limit how many seconds (or milliseconds perhaps?) worth of data is > sent to the buffer. I can't think of any reason not to add such a > feature if it means better usability for the end-user. If the device can't take any more data, it should just refuse to accept it and return from the write() call without anything written.I disagree. Simply returning from write() implies the data was written and the device is ready for more.
The write() function returns the number of bytes actually written, which is not necessarily the same as the number of bytes the caller wanted to write. So the device can chose not to accept all of the data. If the device is currently unable to accept any data (and the file handle is in O_NONBLOCK mode) it shall return EAGAIN to inform the caller that it is currently busy and that the caller should try again later.
If this is not true then you risk making the sync even worse. I consulted with people familiar with this type of stuff and it was suggested the problem could (and should) be fixed at the driver level so I'm following up on that now.
Well, that's exactly what I was suggesting. The write() function has everything it takes to block data from coming into the device if its buffer is full. No need for the caller to do any funny estimates here.
Klaus
Klaus Schmidinger wrote:
VDR User wrote:
On 1/31/07, *Klaus Schmidinger* <Klaus.Schmidinger@cadsoft.de mailto:Klaus.Schmidinger@cadsoft.de> wrote:
VDR User wrote: > From what he's saying, the problem is buffer overrun's, not underrun's.
Afaics this is just a guess. We need a sample recording and track down what happens in the driver/firmware.
> Too much data is being sent and the device isn't able to keep up. If > that's the case then it would make sense for vdr to have a user setting > to limit how many seconds (or milliseconds perhaps?) worth of data is > sent to the buffer. I can't think of any reason not to add such a > feature if it means better usability for the end-user. If the device can't take any more data, it should just refuse to accept it and return from the write() call without anything written.
Code from the driver: | ... | if (nonblock && !FREE_COND) | return -EWOULDBLOCK; | | while (todo > 0) { | if (!FREE_COND) { | if (nonblock) | return count - todo; | if (wait_event_interruptible(av7110->avout.queue, | FREE_COND)) | return count - todo; | } | ...
Looks ok to me. The driver does not accept more data than it can handle.
I disagree. Simply returning from write() implies the data was written and the device is ready for more.
The write() function returns the number of bytes actually written, which is not necessarily the same as the number of bytes the caller wanted to write. So the device can chose not to accept all of the data. If the device is currently unable to accept any data (and the file handle is in O_NONBLOCK mode) it shall return EAGAIN to inform the caller that it is currently busy and that the caller should try again later.
If this is not true then you risk making the sync even worse. I consulted with people familiar with this type of stuff and it was suggested the problem could (and should) be fixed at the driver level so I'm following up on that now.
Well, that's exactly what I was suggesting. The write() function has everything it takes to block data from coming into the device if its buffer is full. No need for the caller to do any funny estimates here.
Fullack. There is no reason to add workarounds like this to VDR.
@all: Don't panic. Provide samples. :-)
Oliver
On Wed, Jan 31, 2007 at 08:38:29PM +0100, Klaus Schmidinger wrote:
From what he's saying, the problem is buffer overrun's, not underrun's. Too much data is being sent and the device isn't able to keep up. If that's the case then it would make sense for vdr to have a user setting to limit how many seconds (or milliseconds perhaps?) worth of data is sent to the buffer. I can't think of any reason not to add such a feature if it means better usability for the end-user.
If the device can't take any more data, it should just refuse to accept it and return from the write() call without anything written.
I gather that in plugin-based playback, cDevice::PlayAudio() and cDevice::PlayVideo() are overridden and may return 0 to indicate that nothing was written.
I modified PlayAudio and PlayVideo in softdevice.c so that they return 0 when softdevice is in suspended state. That is, I replaced if (! packetMode) with if (!packetMode && !setupStore.shouldSuspend) in both methods, so that they would return 0.
This seemed to work for recordings, but I am not sure if it worked for video. If I suspended the playback for only a short while, it looked like the playback resumed from where it was stopped, and maybe some frames were dropped every now and then to reduce the lag. I'd prefer the live playback to resume without any lag, that is, to drop all live PES packets during the time the playback was suspended in the plugin. However, the PES packets from recordings should be blocked.
I believe that the easy fix would be to have these methods return 0 only when playing a recording. Can the plugin detect this somehow?
Marko
Hi,
Marko Mäkelä wrote:
I believe that the easy fix would be to have these methods return 0 only when playing a recording. Can the plugin detect this somehow?
For live TV, VDR runs a transfer thread and the replaying device may use cDevice::Transferring() to detect this (works at least for vdr-xine in cXineDevice::SetPlayMode() for PlayMode being != pmNone).
Bye.
On Thu, Feb 01, 2007 at 07:57:20PM +0100, Reinhard Nissl wrote:
Hi,
Marko Mäkelä wrote:
I believe that the easy fix would be to have these methods return 0 only when playing a recording. Can the plugin detect this somehow?
For live TV, VDR runs a transfer thread and the replaying device may use cDevice::Transferring() to detect this (works at least for vdr-xine in cXineDevice::SetPlayMode() for PlayMode being != pmNone).
Thanks, I knew there had to be some way. Softdevice used to blank the screen after a few seconds of missing video stream (tuned to an audio channel). A fix was introduced so that it wouldn't blank the screen when a recording was paused.
Marko
Ville Rannikko wrote:
Hi!
The newest firmware for FF cards did not completely fix the AV desync problems for me.
Can you provide a sample recording where A/V sync fails with the current firmware?
According to information from Werner the problem happens when small video frames fill the decoder buffer with over 2 seconds of data. So I made this patch for dvbplayer.c to stop it from uploading more PES frames to decoder when STC/PTS difference is more than 2 seconds. This seems to fix the remaining problems for me, but I have not tested it much. The PTS/STC-code has been mostly taken from the dvb-subtitles plugin. Comments, please
The driver accepts as much data as the driver buffers can hold: - max. 192 KByte video - max. 64 KByte audio
The firmware requests data from the driver. The driver does not push data to the firmware without request from the firmware.
Until now I do not understand how there could be any over/underflows during normal operation.
--- dvbplayer.c.orig 2007-01-31 18:21:42.000000000 +0200 +++ dvbplayer.c 2007-01-31 18:35:36.000000000 +0200 @@ -495,6 +495,51 @@ } } if (p) {
if(playMode == pmPlay && pc > 13){int64_t stc = -1;int64_t pts = -1;if ( p[0] == 0x00 && p[1] == 0x00 && p[2] == 0x01){if(p[7] & 0x80){switch( p[3] ){case 0xE0 ... 0xEF: // videocase 0xC0 ... 0xDF: // audiopts = (int64_t) (p[ 9] & 0x0E) << 29 ;pts |= (int64_t) p[ 10] << 22 ;pts |= (int64_t) (p[ 11] & 0xFE) << 14 ;pts |= (int64_t) p[ 12] << 7 ;pts |= (int64_t) (p[ 13] & 0xFE) >> 1 ;}}}if(pts != -1){cDevice *pd = cDevice::PrimaryDevice();if(pd){stc = pd->GetSTC();if(stc != 0){if(pts & (int64_t)1<<32){stc |= (int64_t)1<<32;}int64_t timeDiff = (pts-stc);if (pts<stc){timeDiff += (int64_t)1<<33;}timeDiff /= 90;if(timeDiff > 2000)cCondWait::SleepMs(timeDiff - 2000);}}}} int w = PlayPes(p, pc, playMode != pmPlay); if (w > 0) { p += w;
Hm, that patch does not look like a real bug fix to me. ;-) It is a workaround which limits the total buffer capacity to max 2s. I guess this works because it somehow triggers a resync in the firmware.
Anyway, fixes must be applied to the firmware or driver, not to VDR.
But let's analyse the problem first. We need a sample recording...
Oliver
-
Klaus Schmidinger -
Marko Mäkelä -
Oliver Endriss -
Reinhard Nissl -
VDR User -
Ville Rannikko