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

[Michael Rickmann <mrickma gwdg de>]

I was stuck testing the latest changes to Ekiga since about 10 days. 
Once a call to 500 ekiga net had been established windows were piling up 
 and rendered WinXP and Win7 unusable. Today I found the reason: commit 
23036 to opal trunk which coresponds to  23028 opal v3_6 and is 
contained in the archives needed by Ekiga 3.2.5.
Attached patch fixes this for ekiga head - it just reverses the opal 
commit - no final solution. I am really no opal expert and a bit lost 
here. Why has only Win32 been hit here?
Good news is that "Split ekiga into an exec and helper libs" - it 
happend at about the same time as the opal change - seems to work for Win32.
Regards
Michael

diff -ur opal-23036/include/lids/lidep.h opal-23035/include/lids/lidep.h
--- opal-23036/include/lids/lidep.h	2009-07-12 20:23:54.000000000 +0200
+++ opal-23035/include/lids/lidep.h	2009-07-12 20:22:07.000000000 +0200
@@ -592,8 +592,7 @@
        The default behaviour does nothing.
       */
     virtual PBoolean SetDataSize(
-      PINDEX dataSize,  ///< New data size (in total)
-      PINDEX frameSize  ///< Individual frame size (if applicable)
+      PINDEX dataSize  ///<  New data size
     );
 
     /**Indicate if the media stream is synchronous.
diff -ur opal-23036/include/opal/mediastrm.h opal-23035/include/opal/mediastrm.h
--- opal-23036/include/opal/mediastrm.h	2009-07-12 20:23:54.000000000 +0200
+++ opal-23035/include/opal/mediastrm.h	2009-07-12 20:22:07.000000000 +0200
@@ -228,8 +228,7 @@
        The default behaviour does nothing.
       */
     virtual PBoolean SetDataSize(
-      PINDEX dataSize,  ///< New data size (in total)
-      PINDEX frameSize  ///< Individual frame size (if applicable)
+      PINDEX dataSize  ///<  New data size
     );
 
     /**Get the data size in bytes that is expected to be used. Some media
@@ -513,8 +512,7 @@
     /**Set the data size in bytes that is expected to be used.
       */
     virtual PBoolean SetDataSize(
-      PINDEX dataSize,  ///< New data size (in total)
-      PINDEX frameSize  ///< Individual frame size (if applicable)
+      PINDEX dataSize  ///<  New data size
     );
 
     /**Indicate if the media stream is synchronous.
@@ -734,8 +732,7 @@
        The defafault simply sets teh member variable defaultDataSize.
       */
     virtual PBoolean SetDataSize(
-      PINDEX dataSize,  ///< New data size (in total)
-      PINDEX frameSize  ///< Individual frame size (if applicable)
+      PINDEX dataSize  ///<  New data size
     );
 
     /**Indicate if the media stream is synchronous.
@@ -826,8 +823,7 @@
     /** Override size of frame header is included
       */
     virtual PBoolean SetDataSize(
-      PINDEX dataSize,  ///< New data size (in total)
-      PINDEX frameSize  ///< Individual frame size (if applicable)
+     PINDEX dataSize  ///<  New data size
     );
 
     /** Get the input device (e.g. for statistics)
diff -ur opal-23036/src/lids/lidep.cxx opal-23035/src/lids/lidep.cxx
--- opal-23036/src/lids/lidep.cxx	2009-07-12 20:23:54.000000000 +0200
+++ opal-23035/src/lids/lidep.cxx	2009-07-12 20:22:07.000000000 +0200
@@ -932,7 +932,7 @@
       return false;
   }
 
-  SetDataSize(GetDataSize(), GetDataSize());
+  SetDataSize(GetDataSize());
   PTRACE(3, "LineMedia\tStream opened for " << mediaFormat << ", using "
          << (notUsingRTP ? (useDeblocking ? "reblocked audio" : "audio frames") : "direct RTP"));
 
@@ -1101,13 +1101,13 @@
 }
 
 
-PBoolean OpalLineMediaStream::SetDataSize(PINDEX /*dataSize*/, PINDEX frameSize)
+PBoolean OpalLineMediaStream::SetDataSize(PINDEX dataSize)
 {
   if (notUsingRTP) {
     if (IsSource())
-      useDeblocking = !line.SetReadFrameSize(frameSize) || line.GetReadFrameSize() != frameSize;
+      useDeblocking = !line.SetReadFrameSize(dataSize) || line.GetReadFrameSize() != dataSize;
     else
-      useDeblocking = !line.SetWriteFrameSize(frameSize) || line.GetWriteFrameSize() != frameSize;
+      useDeblocking = !line.SetWriteFrameSize(dataSize) || line.GetWriteFrameSize() != dataSize;
 
     PTRACE(3, "LineMedia\tStream frame size: rd="
            << line.GetReadFrameSize() << " wr="
@@ -1115,8 +1115,7 @@
            << (useDeblocking ? "needs" : "no") << " reblocking.");
   }
 
-  // Yes we set BOTH to frameSize, and ignore the dataSize parameter
-  return OpalMediaStream::SetDataSize(frameSize, frameSize);
+  return OpalMediaStream::SetDataSize(dataSize);
 }
 
 
diff -ur opal-23036/src/opal/mediastrm.cxx opal-23035/src/opal/mediastrm.cxx
--- opal-23036/src/opal/mediastrm.cxx	2009-07-12 20:23:54.000000000 +0200
+++ opal-23035/src/opal/mediastrm.cxx	2009-07-12 20:22:07.000000000 +0200
@@ -389,15 +389,11 @@
 }
 
 
-PBoolean OpalMediaStream::SetDataSize(PINDEX dataSize, PINDEX frameSize)
+PBoolean OpalMediaStream::SetDataSize(PINDEX dataSize)
 {
   if (dataSize <= 0)
     return false;
 
-  // Quantise data size
-  if (frameSize > 1)
-    dataSize = (dataSize+frameSize-1)/frameSize * frameSize;
-
   PTRACE_IF(4, defaultDataSize != dataSize, "Media\tSet data size from " << defaultDataSize << " to " << dataSize);
   defaultDataSize = dataSize;
   return true;
@@ -665,8 +661,11 @@
 }
 
 
-PBoolean OpalRTPMediaStream::SetDataSize(PINDEX PTRACE_PARAM(dataSize), PINDEX /*frameSize*/)
+PBoolean OpalRTPMediaStream::SetDataSize(PINDEX dataSize)
 {
+  if (dataSize <= 0)
+    return false;
+
   PTRACE(3, "Media\tRTP data size cannot be changed to " << dataSize << ", fixed at " << defaultDataSize);
   return true;
 }
@@ -813,7 +812,7 @@
   m_channel = chan;
   m_autoDelete = autoDelete;
 
-  SetDataSize(GetDataSize(), sizeof(short));
+  SetDataSize(GetDataSize());
 
   m_channelMutex.Signal();
 
@@ -959,16 +958,16 @@
 }
 
 
-PBoolean OpalAudioMediaStream::SetDataSize(PINDEX dataSize, PINDEX frameSize)
+PBoolean OpalAudioMediaStream::SetDataSize(PINDEX dataSize)
 {
   /* For efficiency reasons we will not accept a packet size that is too small.
      We move it up to the next even multiple, which has a danger of the remote not
      sending an even number of our multiplier. */
-  const unsigned MinTimeMilliseconds = 10;
-  PINDEX minSize = mediaFormat.GetClockRate()/(1000/MinTimeMilliseconds)*sizeof(short);
-  if (frameSize < minSize) {
-    PTRACE(1, "Media\tClamping audio stream frame size from " << frameSize << " to minimum " << minSize);
-    frameSize = minSize;
+  const unsigned MinTime = 10;
+  PINDEX minSize = mediaFormat.GetClockRate()/(1000/MinTime)*sizeof(short);
+  if (dataSize < minSize) {
+    PTRACE(1, "Media\tClamping audio stream data size from " << dataSize << " to minimum " << minSize);
+    dataSize = ((minSize+dataSize-1)/dataSize)*dataSize;
   }
 
   unsigned dataTime = ((dataSize/sizeof(short))*1000)/mediaFormat.GetClockRate();
@@ -985,9 +984,9 @@
   }
 
   PTRACE(3, "Media\tAudio " << (IsSource() ? "source" : "sink") << " data size set to "
-         << frameSize << " bytes and " << soundChannelBuffers << " buffers.");
-  return OpalMediaStream::SetDataSize(dataSize, frameSize) &&
-         ((PSoundChannel *)m_channel)->SetBuffers(frameSize, soundChannelBuffers);
+         << dataSize << " bytes and " << soundChannelBuffers << " buffers.");
+  return OpalMediaStream::SetDataSize(dataSize) &&
+         ((PSoundChannel *)m_channel)->SetBuffers(dataSize, soundChannelBuffers);
 }
 
 
@@ -1028,7 +1027,7 @@
 }
 
 
-PBoolean OpalVideoMediaStream::SetDataSize(PINDEX dataSize, PINDEX frameSize)
+PBoolean OpalVideoMediaStream::SetDataSize(PINDEX dataSize)
 {
   if (inputDevice != NULL) {
     PINDEX minDataSize = inputDevice->GetMaxFrameBytes();
@@ -1041,7 +1040,7 @@
       dataSize = minDataSize;
   }
 
-  return OpalMediaStream::SetDataSize(sizeof(PluginCodec_Video_FrameHeader) + dataSize, frameSize);
+  return OpalMediaStream::SetDataSize(sizeof(PluginCodec_Video_FrameHeader) + dataSize); 
 }
 
 
@@ -1080,7 +1079,7 @@
     }
   }
 
-  SetDataSize(1, 1); // Gets set to minimum of device buffer requirements
+  SetDataSize(0); // Gets set to minimum of device buffer requirements
 
   return OpalMediaStream::Open();
 }
diff -ur opal-23036/src/opal/patch.cxx opal-23035/src/opal/patch.cxx
--- opal-23036/src/opal/patch.cxx	2009-07-12 20:23:54.000000000 +0200
+++ opal-23035/src/opal/patch.cxx	2009-07-12 20:22:07.000000000 +0200
@@ -180,7 +180,7 @@
   if (sink->primaryCodec != NULL) {
     PTRACE(4, "Patch\tCreated primary codec " << sourceFormat << "->" << destinationFormat << " with ID " << id);
 
-    if (!sinkStream->SetDataSize(sink->primaryCodec->GetOptimalDataFrameSize(false), destinationFormat.GetFrameSize())) {
+    if (!sinkStream->SetDataSize(sink->primaryCodec->GetOptimalDataFrameSize(false))) {
       PTRACE(1, "Patch\tSink stream " << *sinkStream << " cannot support data size "
               << sink->primaryCodec->GetOptimalDataFrameSize(PFalse));
       return false;
@@ -203,7 +203,7 @@
 
     PTRACE(4, "Patch\tCreated two stage codec " << sourceFormat << "/" << intermediateFormat << "/" << destinationFormat << " with ID " << id);
 
-    if (!sinkStream->SetDataSize(sink->secondaryCodec->GetOptimalDataFrameSize(false), destinationFormat.GetFrameSize())) {
+    if (!sinkStream->SetDataSize(sink->secondaryCodec->GetOptimalDataFrameSize(PFalse))) {
       PTRACE(1, "Patch\tSink stream " << *sinkStream << " cannot support data size "
               << sink->secondaryCodec->GetOptimalDataFrameSize(PFalse));
       return false;
@@ -215,7 +215,7 @@
            << " and " << *sink->secondaryCodec << ", data size=" << sinkStream->GetDataSize());
   }
 
-  source.SetDataSize(sink->primaryCodec->GetOptimalDataFrameSize(true), sourceFormat.GetFrameSize());
+  source.SetDataSize(sink->primaryCodec->GetOptimalDataFrameSize(true));
   return true;
 }
 
@@ -464,22 +464,11 @@
   bool asynchronous = OnPatchStart();
   PTimeInterval lastTick;
 
-  /* Note the RTP frame is outside loop so that a) it is more efficient
-     for memory usage, the buffer is only ever increased and not allocated
-     on the heap ever time, and b) the timestamp value embedded into the
-     sourceFrame is needed for correct operation of the jitter buffer and
-     silence frames. It is adjusted by DispatchFrame (really Sink::WriteFrame)
-     each time and passed back in to source.Read() (and eventually the JB) so
-     it knows where it is up to in extracting data from the JB. */
-  RTP_DataFrame sourceFrame(0);
-
   while (source.IsOpen()) {
-    sourceFrame.SetPayloadType(source.GetMediaFormat().GetPayloadType());
 
-    // We do the following to make sure that the buffer size is large enough,
-    // in case something in previous loop adjusted it
-    sourceFrame.SetPayloadSize(source.GetDataSize());
-    sourceFrame.SetPayloadSize(0);
+    RTP_DataFrame sourceFrame(source.GetDataSize());
+    sourceFrame.SetPayloadType(source.GetMediaFormat().GetPayloadType());
+    sourceFrame.SetPayloadSize(0); 
 
     if (!source.ReadPacket(sourceFrame)) {
       PTRACE(4, "Patch\tThread ended because source read failed");