Re: [Ekiga-devel-list] Win32 Opal datasize, please help

[Michael Rickmann <mrickma gwdg de>]

Damien Sandras schrieb:
> Le mercredi 15 juillet 2009 à 08:52 +0200, Michael Rickmann a écrit :
> > Am Dienstag, den 14.07.2009, 23:09 +0200 schrieb Damien Sandras:
> > > Le mardi 14 juillet 2009 à 18:06 +0200, Michael Rickmann a écrit :
> > > > Damien Sandras schrieb:
> > > > > Le lundi 13 juillet 2009 à 23:32 +0200, Michael Rickmann a écrit :
> > > > > > Damien Sandras schrieb:
> > > > > > > Le dimanche 12 juillet 2009 à 22:40 +0200, Michael Rickmann a écrit :
> > snip
> >
> > > > > > http://wwwuser.gwdg.de/~mrickma/ekiga/data-size-logs.tar.gz. 
> > > > > > 500 ekiganet-ekiga-killed-stderr txt is when calling 500 ekiga net  As 
> > > > > > soon as the call was accepted empty windows were piling up and I had to 
> > > > > > kill Ekiga. fritzbox-nosound-ekiga-stderr.txt is when calling my 
> > > > > > audio-only SIP-phone-box at home. Ekiga told me that it was unable to 
> > > > > > open the audio driver, exit gracefully. The programs which I used can be 
> > > > > > found at http://wwwuser.gwdg.de/~mrickma :
> > > > > > buggy-ekiga-setup-3.2.5-release.exe fails, ekiga-setup-3.2.5-release.exe 
> > > > > > is ok with patch reversed, ekiga_build-3.2.5.tgz documents how I built 
> > > > > > the stuff.
> > > > > Can you try with the following patch ?
> > > > > The error will stay, but we will have more information and you won't
> > > > > have windows popping up again and again.
> > > > >
> > > > > You will also have an indication of what the problem can be.
> > > > >
> > > > > Robert thinks the problem could be in Ekiga, but due to the API change.
> > > > I applied the patch, the Windows went and Win32 Ekiga is better than it 
> > > > ever was. The large audio delay (> 1.5 sec) is nearly gone, must have 
> > > > been the changes to Opal. However, as you said, the error stayed - 
> > > > somehow I like it, audio is next to perfect. In the logs we get now 
> > > > lines saying
> > > > Media Patch:3096	GMAudioInputManager_ptlib	Encountered error while 
> > > > trying to read data, read 160 instead of 320
> > > > The complete -d 4 log you find at 
> > > > http://wwwuser.gwdg.de/~mrickma/ekiga/ekiga-stderr.txt.gz .
> > > > Attached backtrace shows what happens when calling 500 ekiga net and 
> > > > where the request for 320 comes from.
> > > > Thanks for the help so far.
> > > > Michael
> > > The purpose of that fix (from Robert) was to improve the audio quality.
> > >
> > > Now, even if it works, we need to determine where those 320 and 160
> > > values come from and which one is correct (I suppose 160 is correct as
> > > it works).
> > > --
> > >  _     Damien Sandras
> > Yes,, I also think 160 is correct. From the -d 4 log:
> > Pool:2872	Media	Clamping audio stream frame size from 16 to minimum 160
> > Pool:2872	Media	Audio source data size set to 160 bytes and 20 buffers.
> > Pool:2872	Media	Set data size from 16 to 320
> > Pool:2872	AudioInputCore	Setting stream buffer size 20/160
> > Pool:2872	GMAudioInputManager_ptlib	Setting buffer size to 160/20
> > Pool:2872	WinSnd	Setting sounds buffers to 20 x 160
> > Pool:2872	PCSS	Adding filters to patch
> >
> > This evening, I will look where this "Media Set data size from 16 to
> > 320" comes from, I guess Opal. Ekiga does not set any data sizes, does
> > it?
>
> I don't think so, but I'm not 100% sure. I have read through the code
> yesterday too.

Well it all condenses down to line opal-3.6.4/src/opal/patch.cxx:217 
which is
source.SetDataSize(sink->primaryCodec->GetOptimalDataFrameSize(true), 
sourceFormat.GetFrameSize());
Opal's ***"dataSize"***, which we are called with in 
"ekiga-3.2.5/lib/engine/components/ptlib/audioinput-manager-ptlib.cpp :
GMAudioInputManager_ptlib::get_frame_data : size" is derived from the 
GetOptimalDataFrameSize(true) part.
In our GMAudioInputManager_ptlib::set_buffer_size : buffer_size we are 
confronted with Opal's ***"frameSize"*** which is derived from 
sourceFormat.GetFrameSize() (see above). Opal makes shure that 
***"dataSize"*** is a multiple of ***"frameSize"***.
As Ekiga must not know about Opal's internals in 
GMAudioInputManager_ptlib::get_frame_data we are supposed to read
as many buffers as needed for size  (see GetOptimalDataFrameSize(true) 
above).

Could you make sure whether the interpretation of the new API to read as 
many buffers to fullfill the size arggument of "PBoolean 
OpalRawMediaStream::ReadData(BYTE * buffer, PINDEX size, PINDEX & 
length)" is correct?

If so I would suggest something like attached patch - I have only tested 
it under Win7-RC. The shortening of Win32 audio latency presumably 
achieved by the change in Opal stays. Eventually, I think Linux and 
Win32 could share the code at this place.
Michael



diff -ur ekiga-3.2.5.orig/lib/engine/components/ptlib/audioinput-manager-ptlib.cpp ekiga-3.2.5/lib/engine/components/ptlib/audioinput-manager-ptlib.cpp
--- ekiga-3.2.5.orig/lib/engine/components/ptlib/audioinput-manager-ptlib.cpp	2009-07-15 18:37:09.000000000 +0200
+++ ekiga-3.2.5/lib/engine/components/ptlib/audioinput-manager-ptlib.cpp	2009-07-15 19:37:57.000000000 +0200
@@ -47,7 +47,7 @@
 {
   current_state.opened = false;
   input_device = NULL;
-  expectedFrameSize = 0;
+  internalFrameSize = 0;
 }
 
 GMAudioInputManager_ptlib::~GMAudioInputManager_ptlib ()
@@ -160,8 +160,10 @@
 {
   PTRACE(4, "GMAudioInputManager_ptlib\tSetting buffer size to " << buffer_size << "/" <<  num_buffers);
 
-  if (input_device)
+  if (input_device) {
     input_device->SetBuffers (buffer_size, num_buffers);
+    internalFrameSize = buffer_size;
+  }
 }
 
 
@@ -169,6 +171,7 @@
                                                 unsigned size,
 						unsigned & bytes_read)
 {
+  unsigned chunk_read;
   bool ret = false;
   bytes_read = 0;
 
@@ -177,14 +180,19 @@
     return false;
   }
 
-  if (input_device) {
-    ret = input_device->Read ((void*)data, size);
-    if (ret) {
-      bytes_read = input_device->GetLastReadCount();
-    }
-    if (bytes_read != size) {
-      PTRACE(1, "GMAudioInputManager_ptlib\tEncountered error while trying to read data");
-      Ekiga::Runtime::run_in_main (sigc::bind (sigc::mem_fun (this, &GMAudioInputManager_ptlib::device_error_in_main), current_state.device, Ekiga::AI_ERROR_READ));
+  if (input_device && internalFrameSize) {
+    ret = true;
+    while (ret && (size - bytes_read >= internalFrameSize)) {
+      ret = input_device->Read ((void*)(data + bytes_read), internalFrameSize);
+      if (ret) {
+        chunk_read = input_device->GetLastReadCount();
+        bytes_read += chunk_read;
+      }
+      if (chunk_read != internalFrameSize) {
+        ret = false;
+        PTRACE(1, "GMAudioInputManager_ptlib\tEncountered error while trying to read data, read " << chunk_read << " instead of " << internalFrameSize);
+        Ekiga::Runtime::run_in_main (sigc::bind (sigc::mem_fun (this, &GMAudioInputManager_ptlib::device_error_in_main), current_state.device, Ekiga::AI_ERROR_READ));
+      }
     }
   }
 
diff -ur ekiga-3.2.5.orig/lib/engine/components/ptlib/audioinput-manager-ptlib.h ekiga-3.2.5/lib/engine/components/ptlib/audioinput-manager-ptlib.h
--- ekiga-3.2.5.orig/lib/engine/components/ptlib/audioinput-manager-ptlib.h	2009-07-15 18:37:09.000000000 +0200
+++ ekiga-3.2.5/lib/engine/components/ptlib/audioinput-manager-ptlib.h	2009-07-15 18:42:19.000000000 +0200
@@ -78,7 +78,7 @@
 
   protected:
       Ekiga::ServiceCore & core;
-      unsigned expectedFrameSize;
+      unsigned internalFrameSize;
 
       PSoundChannel *input_device;