0

I want to write a routine that receives some jpeg frames from a server (Remote desktop - like), converts them to bitmap images and then displays them on a windows form. I am trying to make the routine as lighter as possible but perhaps I am doing it wrong as I receive always a System.OutOfMemoryException. My code follows:

EDIT: added a part that is related to this exception

private void WatcherRoutine()
    {
        Boolean lLoopEnd = false;
        Bitmap lCurrent = null;
        //Graphics lGraphics = null;
        Image lImg = null;
        BinaryReader lBRVideo = new BinaryReader(this._state.Video.GetStream());

        while (lLoopEnd == false)
        {
            try
            {
                // Reads frame type
                switch (lBRVideo.ReadByte())
                {
                    // Frame received is a big frame (ie a desktop screenshot)
                    case Constants.BIGFRAME:
                        {
                            // Reads frame size in bytes
                            Int32 lVideoLength = lBRVideo.ReadInt32();
                            if (lVideoLength > 0)
                            {
                                // Stores frame in a stream
                                MemoryStream ms = new MemoryStream(lBRVideo.ReadBytes(lVideoLength));
                                // Creates image from stream
                                lImg = Image.FromStream(ms);
                                ms.Dispose();
                                // Creates bitmap from image
                                lCurrent = new Bitmap(lImg);
                                lImg.Dispose();
                                // Passes image to windows form to display it
                                this.Invoke(this._state.dUpdateVideo, lCurrent);
                                    ////lGraphics = Graphics.FromImage(lImg);
                                    //lGraphics.Dispose();
                            }
                        }
                        break;
                    // Diff frame (ie a small part of desktop that has changed)
                    // Commenting this part makes the exception disappear :|
                    case Constants.DIFFFRAME:
                        {
                            Int16 lX = lBRVideo.ReadInt16(),
                                lY = lBRVideo.ReadInt16();
                            Int32 lVideoLength = lBRVideo.ReadInt32();
                            if (lVideoLength > 0)
                            {
                                //Byte[] lVideoImg = lBRVideo.ReadBytes(lVideoLength);
                                //Image lImgDiff = Image.FromStream(new MemoryStream(lVideoImg));
                                ////if(lGraphics != null)
                                //{
                                //    lGraphics.DrawImage(lImgDiff, lX, lY);
                                //    this.Invoke(this._state.dUpdateVideo, new Bitmap(lImg));
                                //}
                            }
                        }
                        break;
                    case Constants.CURSOR:
                        {
                            Int16 lX = lBRVideo.ReadInt16(),
                                lY = lBRVideo.ReadInt16();
                            // TODO
                        }
                        break;
                    default:
                        break;
                }
            }
            catch (Exception e)
            {
                if (this._state.WorkEnd == false)
                {
                    this._state.WorkEnd = true;
                    this.BeginInvoke(this._state.dDisconnect);
                }
                lLoopEnd = true;
                SmartDebug.DWL(e.Message);
            }
        }
    }

dUpdateVideo is a delegate that contains this small routine.. perhaps have I to free pBmp?

private void UpdateVideo(Bitmap pBmp)
    {
        this.VideoPictureBox.Image = pBmp;
    }
gc5
  • 9,468
  • 24
  • 90
  • 151
  • don't see if you release memory used by lCurrent – imaximchuk Dec 11 '11 at 16:55
  • pease provide the source of dUpdateVideo, it may also be a source of memory leak. check if you receive correct data in lVideoLength (it may be very big number due to some error) – imaximchuk Dec 11 '11 at 16:56
  • 1
    I think this is a problem with your binary protocol and video length is somehow messed up (see lBRVideo.ReadInt16 and ReadInt32 calls you comment out) – imaximchuk Dec 11 '11 at 16:59
  • now I'll check thx :) edit: I found a bug.. now I'll try to see if it is related.. – gc5 Dec 11 '11 at 17:07
  • it is related.. if you add an answer I'll give you another point, because it was a part of problem :) thank you – gc5 Dec 11 '11 at 17:20

3 Answers3

2

When you're using GDI+ based APIs (System.Drawing), an OutOfMemory exception doesn't necessarily mean that you're out of memory. It can also mean that parameters passed to GDI+ are invalid, or some other cause. GDI+ is pretty OutOfMemory happy.


You should also reuse your memory stream, if possible. That reduces GC pressure a lot. You're allocating many large objects, and the GC is pretty bad in that scenario.


Also I think you're never disposing lCurrent.


Then you violate the contract of Image.FromStream:

You must keep the stream open for the lifetime of the Image:

lImg = Image.FromStream(ms);
ms.Dispose();
lCurrent = new Bitmap(lImg);// `lImage` is used here, but `ms` is already disposed
lImg.Dispose();

The documentation for Image.FromStream states:

You must keep the stream open for the lifetime of the Image.

Move ms.Dispose() behind lImg.Dispose()

CodesInChaos
  • 106,488
  • 23
  • 218
  • 262
1

Once I wrote a program which processed lots of images loaded from files. I Disposed everything I could, as soon as possible, and let the rest to the GC. This wasn't enough, memory usage profiling showed clearly that GC was too slow relative to the image loading speed of my program. The solution was to call GC.Collect() manually each time when I finished processing a given number of images. Note that this is not a good practice, but sometimes helps. At least it worth trying.

Community
  • 1
  • 1
kol
  • 27,881
  • 12
  • 83
  • 120
  • I forgot that feature, thanks :) Yes it is not a good practice, but maybe here it is necessary – gc5 Dec 11 '11 at 17:06
1

The problem may be related to the binary protocol error (video length is somehow messed up, see lBRVideo.ReadInt16 and ReadInt32 calls you comment out)

imaximchuk
  • 748
  • 5
  • 13