softdevice audio problem. audio repacker issue?

  Reinhard Nissl schrieb:
> Hi,
> 
> Stefan Lucke wrote:
> 
>> Dumping the first 16 bytes of audio packets we get, shows the following:
>> ff fc a4 0d b6 64 88 55 33 65 56 54 44 21 33 33
>> ff fc a4 0d 54 6a 88 65 33 44 56 54 33 33 33 33
>> ff fc a4 0d 02 a4 88 55 33 54 66 44 43 33 33 33
>> 99 77 1b e6 34 b2 5f 41 e8 5b 90 a9 d2 04 24 5e
>> ff fc c4 04 7c c4 dd 44 44 66 57 55 55 55 33 33
>> ff fc c4 04 1d 61 dd 46 44 66 55 55 55 55 33 33
>> 68 6a 60 18 d8 52 92 4f 54 98 0b 93 27 22 4e 74
>> ff fc c4 04 94 01 dd 44 56 66 55 55 55 55 33 33
>>
>> Is audio repacker active even for old recording ?
>> When I deactivate audio repacker, sound is garbled allways.
> 
> cAudioRepacker was introduced after VDR-1.3.26 and it is only active in
> transfer mode or while recording. It is not active while replaying a
> recording, so it has no influence on recordings taken with VDR-1.2.1.
> 
> But when cAudioRepacker was not active, an audio PES packet may contain,
>  multiple audio frames and/or just a fragment of an audio frame at the
> beginning or at the end of the PES packet.
> 
> So if ffmpeg can only work on single and/or complete audio frames,
> you'll have to break the PES packet apart and assemble the fragments at
> the end and the beginning of the next PES packet. 

FFmpeg can handle handle packets with multiple audio frames well, one
just has to use the av_read_frame() interface instead of
av_read_packet() like the softdevice currently does. There has been a
change in the audio decode some time ago, before the decoder could also
split the packets, that is why it worked before...

I attached a patch with make the softdevice use av_read_frame(), it has
still some issues, but it solves the problems Stefan reports.

Is there a reason why the cAudioRepacker is used in transfer mode and
during recordings, but not while replaying?

Bye,

Martin
  

Hi,

Martin Wache wrote:

> Is there a reason why the cAudioRepacker is used in transfer mode and
> during recordings, but not while replaying?

Well, when cAudioRepacker was active while recording, then there is no
need for it when replaying such a recording. And old recordings will
vanish as time passes by.

Bye.
  

Hi,


Reinhard Nissl schrieb:
> Hi,
> 
> Martin Wache wrote:
> 
>> Is there a reason why the cAudioRepacker is used in transfer mode and
>> during recordings, but not while replaying?
> 
> Well, when cAudioRepacker was active while recording, then there is no
> need for it when replaying such a recording.

Wouldn't it make more sense to use the repacker only when replaying and
in transfer mode and not while recording?
Like it is now, it is possible that the device gets either repacked
audio frames (transfer mode and new recordings) or not repacked frames.
If the device needs repacked frames the device has to find out if it is
replaying an old recording to play and if so, split the packets. I guess
in most cases it is a lot simpler just to repack everything. Which means
in most cases to do the repacking twice...

> And old recordings will
> vanish as time passes by.
>
I don't buy this argument. I know that there are VDR users with large
archives of recordings, do you want to tell them that they can't use
them any more?

But anyway, actually I think the best would be only to repack audio and
video if the replaying device needs it. As far as I know FF cards don't
need it, the softdevice doesn't need it (ok, it depends on the ffmpeg
version one uses, but the patch I send fixes this).
So why not let the device choose if repacking is needed or not? It
should now if repacking is needed, and in most cases it should be able
to do it by itself.

Bye,
Martin
  

On Saturday 17 March 2007 23:11, Martin Wache wrote:
> Hi,
> 
> 
> Reinhard Nissl schrieb:


> 
> > And old recordings will
> > vanish as time passes by.
> >
> I don't buy this argument. I know that there are VDR users with large
> archives of recordings, do you want to tell them that they can't use
> them any more?

Second that. Lifetime 99.
Won't tell my son, that his favorit recordings of "Die Sendung mit der Maus"
won't be accessible after the next software upgrade :-) .
  

On Saturday 17 March 2007 22:05, Martin Wache wrote:
> I attached a patch with make the softdevice use av_read_frame(), it has
> still some issues, but it solves the problems Stefan reports.

Martin, thats really great.
Thank you.
This solves the issue I had with playback of some old recordings.

But there is still one drawback.
It is still not playable with softplay.

Neither audio nor video.
There are only some accustic fragments and no video frame.
  

Hi,

Martin Wache wrote:

>>> Is there a reason why the cAudioRepacker is used in transfer mode and
>>> during recordings, but not while replaying?
>>
>> Well, when cAudioRepacker was active while recording, then there is no
>> need for it when replaying such a recording.
> 
> Wouldn't it make more sense to use the repacker only when replaying and
> in transfer mode and not while recording?
> Like it is now, it is possible that the device gets either repacked
> audio frames (transfer mode and new recordings) or not repacked frames.
> If the device needs repacked frames the device has to find out if it is
> replaying an old recording to play and if so, split the packets. I guess
> in most cases it is a lot simpler just to repack everything. Which means
> in most cases to do the repacking twice...

Fiddling around with repacking at that late stage is quite a mess
especially in trickspeed mode. And cVideoRepacker causes "high" CPU load
because there is no length information in the video stream so the whole
stream has to be scanned for the 00 00 01 pattern. Furthermore, consider
the memcpying involved in this process. That's why we have chosen the
earliest stage possible (cRemux) to do this complex stuff just once.

>> And old recordings will vanish as time passes by.
>
> I don't buy this argument. I know that there are VDR users with large
> archives of recordings, do you want to tell them that they can't use
> them any more?

No, I don't, as there is no need to. VDR's device interface
specification doesn't state any relationship between PES packets and
their content, so a device implementation shouldn't rely on that.

> But anyway, actually I think the best would be only to repack audio and
> video if the replaying device needs it. As far as I know FF cards don't
> need it, the softdevice doesn't need it (ok, it depends on the ffmpeg
> version one uses, but the patch I send fixes this).
> So why not let the device choose if repacking is needed or not? It
> should now if repacking is needed, and in most cases it should be able
> to do it by itself.

Well, it isn't just a matter of the output device. cVideoRepacker takes
care that VDR's index file contains valid entries. Before
cVideoRepacker, the index file could address incomplete frames or even
miss some frames.

Bye.
  

Stefan Lucke schrieb:
> On Saturday 17 March 2007 22:05, Martin Wache wrote:
>> I attached a patch with make the softdevice use av_read_frame(), it has
>> still some issues, but it solves the problems Stefan reports.
> 
> Martin, thats really great.
> Thank you.
> This solves the issue I had with playback of some old recordings.
> 
> But there is still one drawback.
> It is still not playable with softplay.
> 
> Neither audio nor video.
> There are only some accustic fragments and no video frame.
> 

For me softplay works fine with vdr recordings. Did you make sure that
you compiled softplay with the same ffmpeg version like the softdevice?

Bye,
Martin
  

Reinhard Nissl schrieb:
> Hi,
> 
> Martin Wache wrote:
> 
>>>> Is there a reason why the cAudioRepacker is used in transfer mode and
>>>> during recordings, but not while replaying?
>>> Well, when cAudioRepacker was active while recording, then there is no
>>> need for it when replaying such a recording.
>> Wouldn't it make more sense to use the repacker only when replaying and
>> in transfer mode and not while recording?
>> Like it is now, it is possible that the device gets either repacked
>> audio frames (transfer mode and new recordings) or not repacked frames.
>> If the device needs repacked frames the device has to find out if it is
>> replaying an old recording to play and if so, split the packets. I guess
>> in most cases it is a lot simpler just to repack everything. Which means
>> in most cases to do the repacking twice...
> 
> Fiddling around with repacking at that late stage is quite a mess
> especially in trickspeed mode.

You have to do it anyway when parsing the content of the packets to
decode them. And you can do it even better, because you know what should
be inside. I never had any problem implementing trickspeed modes for the
softdevice, also when there was no repacker in the vdr. FFmpeg does a
good job parsing those packets, and I guess ffmpeg is much better tested
and faster than cAudio/VideoRepacker.

> And cVideoRepacker causes "high" CPU load
> because there is no length information in the video stream so the whole
> stream has to be scanned for the 00 00 01 pattern. Furthermore, consider
> the memcpying involved in this process. 

In my opinion that is a reason not to do the repacking at in the vdr.

> That's why we have chosen the
> earliest stage possible (cRemux) to do this complex stuff just once.
>
No, as I said earlier, you don't guarantee that the repacking is done,
so one have to check that the packet are fine in any case. And checking
them is not more work than completely parsing the packets. So in fact
because  the repacking is only done sometimes one hast to do the work twice.

And during the decoding one has to parse the packets anyway, so it is
much simpler _only_ to parse them properly in the decoder.

>>> And old recordings will vanish as time passes by.
>> I don't buy this argument. I know that there are VDR users with large
>> archives of recordings, do you want to tell them that they can't use
>> them any more?
> 
> No, I don't, as there is no need to. VDR's device interface
> specification doesn't state any relationship between PES packets and
> their content, so a device implementation shouldn't rely on that.
> 
So you are saying that we have to repack in any case... which means that
the packets are completely parsed twice. Didn't you say something about
"high cpu load" and "complex stuff" above?

For soft devices that actually should not matter too much, because of
the strong cpus, but the people who suffer are the ones which are using
hardware decoders and weak cpus. And as far as I know they don't need
repacking...

>> But anyway, actually I think the best would be only to repack audio and
>> video if the replaying device needs it. As far as I know FF cards don't
>> need it, the softdevice doesn't need it (ok, it depends on the ffmpeg
>> version one uses, but the patch I send fixes this).
>> So why not let the device choose if repacking is needed or not? It
>> should now if repacking is needed, and in most cases it should be able
>> to do it by itself.
> 
> Well, it isn't just a matter of the output device. cVideoRepacker takes
> care that VDR's index file contains valid entries. Before
> cVideoRepacker, the index file could address incomplete frames or even
> miss some frames.
> 

I agree that it is cleaner to have an index file which points to
complete frames, but I never experienced any problems without repacking.

I switched the repacking of in my vdr... I don't think that it is necessary.

Bye,
Martin
  

Hi,

Martin Wache wrote:

> I agree that it is cleaner to have an index file which points to
> complete frames, but I never experienced any problems without repacking.

So it seems to me that only xine's libmpeg2 has the problem to drop
incomplete frames. This was annoying when moving cutting marks and I
first solved this issue by patching GetNextIFrame() to deliver the
missing tail too. But Klaus didn't like this hack and asked for a
cleaner approach. The solution sounds quite simple: just start a new PES
packet for each video frame and everything is fine. But the actual
solution is quite complex: cVideoRepacker.

> I switched the repacking off in my vdr... I don't think that it is necessary.

Maybe you are right. The benefit of avoiding memcpys when repacking is
done already during the TS to PES transformation is void as long as one
cannot rely on getting repacked packets all the time.

Bye.
  

On Sun, Mar 18, 2007 at 01:26:03PM +0100, Reinhard Nissl wrote:
 
> > I switched the repacking off in my vdr... I don't think that it is necessary.
> 
> Maybe you are right. The benefit of avoiding memcpys when repacking is
> done already during the TS to PES transformation is void as long as one
> cannot rely on getting repacked packets all the time.

Due to the limited power of the Geode with 300MHz of the Reelbox, I've
analyzed the TS-path in the vdr with gprof/oprofile and re-wrote the
repacker from scratch and changed some of the related TS code in
device/dvbdevice. It has a lot of optimizations:

- The ringbuffer works with packet granularity (no single bytes), no extra
sync checks are needed. Multiple memcpy of the same data is avoided as much
as possible, especially for the video part.

- The MPEG-sequence code search (ScanVideoPacket) is optimized for "simple"
CPUs where the raw number of memory accesses and instructions is important
(no memchr, that's slower on the Geode)

- The video packer detects if the PES-flag in the TS header actually starts
a new PES packet with I, P and B-frames at the TS beginning. After this
detection, the search for start-of-picture is no longer done and the
PES-flag only determines the separation. Since a lot of broadcasters are
using the flag this way (especially the hingh bandwidth ones like ARD and
ZDF in Germany) this saves a lot of CPU time. Services which don't allow
this usually have low bandwidth anyway and are IMO not that important anyway
(CNBC, Gods Channel, etc ;-) )...

- There's one ringbuffer (and thus one data copying) eliminated in the way
from the DVB-device to the TS receivers. Multiple ringbuffers in a row only
give the illusion of better buffering...

- The TS-dispatcher in cDevice::Action looks for bursts of TS-packets
with the same PID and only does one Lock() and one Receive()-call for this
burst instead for each single packet. Video data usually comes in bursts, so
this saves a lot of unnecessary overhead. As the receiver function allows
multiple packets (length parameter) this is perfectly legal. Unfortunately,
some plugins don't respect the length parameter (older femon comes to mind,
I don't know about the latest version). But that is easily fixed...

The new repacker may have some issues with AC3 or some audio-only channels,
but maybe it's worth to have a look at its code...

svn co svn://anonymous@reelbox.org/testing/src/vdr-1.4/
  

Hi,
Is there a patch available with your modifications?
Thanks
Halim
On So, Mär 18, 2007 at 04:30:02 +0100, Georg 
Acher wrote:
> On Sun, Mar 18, 2007 at 01:26:03PM +0100, Reinhard Nissl wrote:
>  
> > > I switched the repacking off in my vdr... I don't think that it is necessary.
> > 
> > Maybe you are right. The benefit of avoiding memcpys when repacking is
> > done already during the TS to PES transformation is void as long as one
> > cannot rely on getting repacked packets all the time.
> 
> Due to the limited power of the Geode with 300MHz of the Reelbox, I've
> analyzed the TS-path in the vdr with gprof/oprofile and re-wrote the
> repacker from scratch and changed some of the related TS code in
> device/dvbdevice. It has a lot of optimizations:
> 
> - The ringbuffer works with packet granularity (no single bytes), no extra
> sync checks are needed. Multiple memcpy of the same data is avoided as much
> as possible, especially for the video part.
> 
> - The MPEG-sequence code search (ScanVideoPacket) is optimized for "simple"
> CPUs where the raw number of memory accesses and instructions is important
> (no memchr, that's slower on the Geode)
> 
> - The video packer detects if the PES-flag in the TS header actually starts
> a new PES packet with I, P and B-frames at the TS beginning. After this
> detection, the search for start-of-picture is no longer done and the
> PES-flag only determines the separation. Since a lot of broadcasters are
> using the flag this way (especially the hingh bandwidth ones like ARD and
> ZDF in Germany) this saves a lot of CPU time. Services which don't allow
> this usually have low bandwidth anyway and are IMO not that important anyway
> (CNBC, Gods Channel, etc ;-) )...
> 
> - There's one ringbuffer (and thus one data copying) eliminated in the way
> from the DVB-device to the TS receivers. Multiple ringbuffers in a row only
> give the illusion of better buffering...
> 
> - The TS-dispatcher in cDevice::Action looks for bursts of TS-packets
> with the same PID and only does one Lock() and one Receive()-call for this
> burst instead for each single packet. Video data usually comes in bursts, so
> this saves a lot of unnecessary overhead. As the receiver function allows
> multiple packets (length parameter) this is perfectly legal. Unfortunately,
> some plugins don't respect the length parameter (older femon comes to mind,
> I don't know about the latest version). But that is easily fixed...
> 
> The new repacker may have some issues with AC3 or some audio-only channels,
> but maybe it's worth to have a look at its code...
> 
> svn co svn://anonymous@reelbox.org/testing/src/vdr-1.4/
>  
> -- 
>          Georg Acher, acher@in.tum.de         
>          http://www.lrr.in.tum.de/~acher
>          "Oh no, not again !" The bowl of petunias          
> 
> _______________________________________________
> vdr mailing list
> vdr@linuxtv.org
> http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
  

On Sun, Mar 18, 2007 at 05:25:00PM +0100, Halim Sahin wrote:
> Hi,
> Is there a patch available with your modifications?

That's a bit difficult, since "our" vdr has no exact mainline equivalent.
It may contain parts from different vdr versions. But as I've heard, it
should work also on a full featured card without any modifications. Maybe
you need to tweak the makefile... The remuxer itself should be API
compatible, so exchanging the file and using tools.c/h from the svn should
also work.
  

Hi,

Georg Acher wrote:

> The new repacker may have some issues with AC3 or some audio-only channels,
> but maybe it's worth to have a look at its code...
> 
> svn co svn://anonymous@reelbox.org/testing/src/vdr-1.4/

I'll have a look at it the next days.

I must admit, that the repacker classes were designed to be integrated
into the existing TS/PES remux code at almost no changes (at least no
complex changes). I thought about rewriting the whole chain too but
didn't find time so far respectively didn't have the need to do it.
Thanks for your work in this area.

Bye.
  

On 3/18/07, Martin Wache <M.Wache@gmx.net> wrote:
>
>
> I switched the repacking of in my vdr... I don't think that it is
> necessary.
>
> Bye,
> Martin



 Hi,

I would like to disable the repacker in my VDR implementation as I think it
might be responsible for sync problems in transfer mode between my budget
Nova-T and FF DVB-S. Could you let me know how you turned it off and if
possible post a patch to do  it?

Kind Regards,

Morfsta
  

On 3/18/07, Georg Acher <acher@in.tum.de> wrote:
>
> That's a bit difficult, since "our" vdr has no exact mainline equivalent.
> It may contain parts from different vdr versions. But as I've heard, it
> should work also on a full featured card without any modifications. Maybe
> you need to tweak the makefile... The remuxer itself should be API
> compatible, so exchanging the file and using tools.c/h from the svn should
> also work.


I can confirm that this repacker code does integrate seamlessly into the
latest 1.4 version of VDR, I replaced remux.c and remux.h as well as tools.cand
tools.h from Georg's SVN tree and recompiled.

I've done some quick tests on radio channels in transfer mode and all seems
to work okay. My AC3 channels are provided directly from my FF card so might
not be subjected to the repacker but they also work fine in both live mode
and recordings.

I'll see if my sync problems in transfer mode improve with this new
implementation and report back. Georg / Reinhard if you would like me to do
any additional testing or debugging let me know.

Kind Regards,

Morfsta