Visual Basic

22-Sep-09 5:41

The code below is the working part of the program which is fed image filenames in order to create thumbnail images. It is fine up to around 70 or so images and then I get an out of memory exception. If you wait around a two minutes or so and continue, it will complete. I am assuming that it is a garbage collection problem and I have tried sleeping the thread etc to no avail. Does any one have any ideas?

Friend Function CreateThumbNail(ByVal ImagePathName As String, ByVal ThumbNailWidth As Integer, ByVal ValueIsPercentage As Boolean) As System.Drawing.Image
    Dim imgWidth As Integer = 0
    Dim imgHeight As Integer = 0
    Dim thumbFactor As Double = 0
    Dim intChangeFactor As Integer = 150

    Try

        Dim sourceBitmap As New Bitmap(Image.FromFile(ImagePathName))
        Dim destBitmap As Image

        If ValueIsPercentage = False Then
            If sourceBitmap.Width > sourceBitmap.Height Then
                'based on width
                If ThumbNailWidth = 0 Then
                    thumbFactor = 125 / sourceBitmap.Width
                Else
                    thumbFactor = ThumbNailWidth / sourceBitmap.Width
                End If
            Else
                'based on height
                If ThumbNailWidth = 0 Then
                    thumbFactor = 125 / sourceBitmap.Height
                Else
                    thumbFactor = ThumbNailWidth / sourceBitmap.Height
                End If
            End If
            imgWidth = CInt(sourceBitmap.Width * thumbFactor)
            imgHeight = CInt(sourceBitmap.Height * thumbFactor)
        Else
            imgWidth = sourceBitmap.Width * (ThumbNailWidth / 100)
            imgHeight = sourceBitmap.Height * (ThumbNailWidth / 100)
        End If

        If imgWidth > intChangeFactor Or imgHeight > intChangeFactor Then
            destBitmap = New Bitmap(imgWidth, imgHeight)
            Dim destGraphic As Graphics = Graphics.FromImage(destBitmap)
            destGraphic.DrawImage(sourceBitmap, 0, 0, destBitmap.Width + 1, destBitmap.Height + 1)
            destGraphic.Dispose()
            'Force GC?
            destGraphic = Nothing
        Else
            destBitmap = sourceBitmap.GetThumbnailImage(imgWidth, imgHeight, Nothing, New IntPtr())
        End If
        sourceBitmap.Dispose()
        sourceBitmap = Nothing

        Thread.Sleep(500)
        Return destBitmap

    Catch ex As Exception

        Return Nothing

    End Try
End Function

Luc Pattyn22-Sep-09 6:24

22-Sep-09 6:24

Hi,

some comments

1.

TheComputerMan wrote:
Catch ex As Exception
Return Nothing
End Try

This code is stupid: whatever goes wrong you choose to just ignore it; and then wonder what is going wrong and post a question here?
The minimum action you should take consists of:
- logging ex.ToString() to a log file, to a listbox, whatever (don't annoy your app's users with unnecessary MessageBoxes though)
- disposing of sourceBitmap

2.

TheComputerMan wrote:
Thread.Sleep(500)

that is useless. if it were to help in any way, all that indicates is that something else is wrong.

3.

TheComputerMan wrote:
sourceBitmap = Nothing

that is useless too, as the variable is local and the method is coming to an end right away (well after idling for 500 msec).

4.
as sourceBitmap.Dispose() should be executed no matter what, you could add a Finally block and put it there (even when the try block ends on return!)

Smile | :)

Luc Pattyn

Have a look at my entry for the lean-and-mean competition; please provide comments, feedback, discussion, and don’t forget to vote for it! Thank you.

Local announcement (Antwerp region): Lange Wapper? Neen!

TheComputerMan22-Sep-09 7:16

22-Sep-09 7:16

Hi Luc,

<quote>TheComputerMan wrote:
Catch ex As Exception
Return Nothing
End Try

This code is stupid: whatever goes wrong you choose to just ignore it; and then wonder what is going wrong and post a question here?

1. A completely erroneous assumption. First of all you assume that I do not know what the error is which I do, second you assume that it is ignored - it is not as the graphic = nothing triggers code which places the problem filename in List(of String) which is then displayed to the user in a list box. The post was NOT because I do not know what is going on. Credit me with a bit more common sense that that.

2. Thread.Sleep was just experimental to see if it would improve things - and I said that in the original post.

3. Is it useless??? Are you sure about that. I have experience of it kicking off the garbage collector in another app which was not kicking off after a dispose was performed and was causing similar problems.

4. You have a point there. This would only be a problem however if the error occurred, but would always be executed while it does not so since the error occurs and then the object does not get released this is not the cause of the problem.

It seems to me that you basically have made one good point and the rest is just unconstructive demeaning comment. This still leaves the problem however!

Luc Pattyn22-Sep-09 7:35

22-Sep-09 7:35

I looked at the code you have shown and gave you my ideas, as you requested.
If you don't like them, that is too bad. If you know better, good for you.
But then, who is having a problem?

Hmmm | :|

Luc Pattyn

Have a look at my entry for the lean-and-mean competition; please provide comments, feedback, discussion, and don’t forget to vote for it! Thank you.

Local announcement (Antwerp region): Lange Wapper? Neen!

TheComputerMan22-Sep-09 10:42

22-Sep-09 10:42

Luc, my apologies but I think I did not get what you were saying, and I did not see them as ideas merely harsh criticism.

Luc Pattyn22-Sep-09 11:47

22-Sep-09 11:47

Sorry, no harsh criticism intended; however I get upset when exceptions get ignored like that. They exist to signal a problem, so don't shut them out.

One more idea: Image.FromFile throws an OOM on a bad file, so I suggest you check which file fails; then try the sequence again, skipping the first half of the succeeding files.

Smile | :)

Luc Pattyn

Have a look at my entry for the lean-and-mean competition; please provide comments, feedback, discussion, and don’t forget to vote for it! Thank you.

Local announcement (Antwerp region): Lange Wapper? Neen!

TheComputerMan22-Sep-09 12:03

22-Sep-09 12:03

Thanks Luc, you may have a good point there. I will skip the first files and see if I get the error.

TheComputerMan22-Sep-09 12:06

22-Sep-09 12:06

Nope, it went straight through and did the lot (40+ files) no problem. It definitely seems to stil at 66 files.

TheComputerMan22-Sep-09 12:36

22-Sep-09 12:36

Strange but true Luc, you sort of hit the nail!

I did this:

sr = New StreamReader(ImagePathName)
'sourceBitmap = New Bitmap(Image.FromFile(ImagePathName))
sourceBitmap = New Bitmap(Image.FromStream(sr.BaseStream))

Reading it into a stream first works - no memory errors!

Thanks for you assistance.(and tutoring Smile | :)

)

Luc Pattyn22-Sep-09 12:50

22-Sep-09 12:50

you're welcome.

sorry, having information in two threads got me confused.

It still is too expensive; I don't see why you copy the image, as
sourceBitmap = New Bitmap(Image.FromStream(sr.BaseStream))
creates two identical images, the first one is the result of FromStream and is a real Image (and also a Bitmap upcast to an Image), the second the result of your Bitmap constructor is a real Bitmap. And once more, the first never gets disposed of.

A simple Image.FromStream(sr.BaseStream) As Bitmap should do (or whatever the VB syntax is for casting from Image to Bitmap). This should halve your CPU and memory load.

FYI: the person you reply to normally gets an e-mail notification; as you seem to be replying a lot to yourself, you got lots of e-mails, and I only saw your replies by watching the forum. So I suggest you make sure you reply to (the last message of) the person you want to get the reply and notification...

Smile | :)

Luc Pattyn

Have a look at my entry for the lean-and-mean competition; please provide comments, feedback, discussion, and don’t forget to vote for it! Thank you.

Local announcement (Antwerp region): Lange Wapper? Neen!

Ian Shlasko22-Sep-09 7:58

Ian Shlasko

22-Sep-09 7:58

Though I agree Luc could have been a little nicer, he's right on all four points.

1) If you know what the exception is, and are intentionally ignoring it (Not ideal, but sometimes unavoidable), you should at least catch the specific exception being triggered... For example, "Catch ex as FileNotFoundException", so you won't miss an unexpected one.

2) As he said, sleeping the thread won't help, but you realized that.

3) Yes, setting a local variable to nothing is meaningless, since it will go out of scope as soon as the method ends. This was needed back in VB6 when you didn't have a Dispose() method, but no longer.

4) Definitely put all disposes in Finally blocks, otherwise they'll be skipped if an exception occurs. If you want to keep things simple, define both disposables at the beginning of the method (Initialized to null), and only try to dispose if they're non-null. That way you don't need an extra try loop just for the graphics object.

Other suggestions:

5) Don't use "new IntPtr()"... Use IntPtr.Zero

6) From the MSDN article for GetThumbnailImage, in reference to the callback parameter: "In GDI+ version 1.0, the delegate is not used. Even so, you must create a delegate and pass a reference to that delegate in this parameter"... As we don't know how this is implemented (By Microsoft), this may or may not be the cause.

If none of these solve your problem, it may be elsewhere in your program... Just because this method does the heavy lifting, doesn't necessarily mean it's the only one that can leak.

Proud to have finally moved to the A-Ark. Which one are you in?
Developer, Author (Guardians of Xen)

TheComputerMan22-Sep-09 11:04

22-Sep-09 11:04

Ian, many thanks for your response. I understand what was being said now and yes the code was not the best! Blush | :O

I have made all the changes that you and Luc suggested, but it still gives an Out of memory error after about 65 or so iterations.

I have had to leave Nothing in the callback as both Microsoft's example and the other example given in MSDN do not work for me. The Microsoft one gives the message "'System.Drawing.Image.GetThumbnailImageAbort' is a delegate type and requires a single 'addressof' expression as the only argument to the constructor." ?? What does that mean. I copied the code which was in c# and converted it to VB on Developer Fusion's web page and that was the result. Using the other example I get 'Value of type 'Boolean' cannot be converted to 'System.Drawing.Image.GetThumbnailImageAbort'

Nothing does not seem to cause any problems!!

Ian Shlasko22-Sep-09 11:13

Ian Shlasko

22-Sep-09 11:13

No problems on the surface, but maybe a memory leak internally... Graphics calls tend to have issues like that...

Basically it wants a delegate of a particular type, in this case a Image.GetThumbnailImageAbort delegate. As per MSDN, you need to create a dummy function, such as:

Private Function ThumbnailDummyFunc() as Boolean
End Function

'And in the GetThumbnailImage call, you would pass this as the argument:
new Image.GetThumbnailImageAbort(AddressOf ThumbnailDummyFunc)

(Sorry if my syntax is off... Been programming in C# all day, so it's tricky to switch mental modes)

Proud to have finally moved to the A-Ark. Which one are you in?
Developer, Author (Guardians of Xen)

TheComputerMan22-Sep-09 11:50

22-Sep-09 11:50

Hi Ian, I think I sussed out the delegate thing. I ended up with this:

Dim callback As New Image.GetThumbnailImageAbort(AddressOf ThumbnailCallback)

and

Public Function ThumbnailCallback() As Boolean
    Return False
End Function

so my getthumbnailimage call looks like this now - but the error still occurs.

destBitmap = sourceBitmap.GetThumbnailImage(imgWidth, imgHeight, callback, IntPtr.Zero)

Looking at the list of errors there is one 'out of memory', then two 'Parameter is not valid' and then the rest are 'Out of Memory'.

I don't know where this 'Parameter is not valid' is coming from so I shall try and single step the whole thing (after the first error)

TheComputerMan22-Sep-09 12:00

22-Sep-09 12:00

It is this line

sourceBitmap = New Bitmap(Image.FromFile(ImagePathName))

This causes the Out Of Memory errors and the Parameter is not valid. When I get the parameter error, the file name is a valid existing file name and is a graphic. If I wait two minutes and then complete the list it works, in other words the file that seemed to cause an error, and all the subsequent files in the list, go through OK. Confused | :confused:

Luc Pattyn22-Sep-09 12:37

22-Sep-09 12:37

TheComputerMan wrote:
sourceBitmap = New Bitmap(Image.FromFile(ImagePathName))

that is bad code! Image has a Dispose() method, so you should call it; by having two operations in one line, you don't have the first image reference and can't call Dispose. Split it into two statements, and dispose.

BTW: If the copy is there to avoid file lock problems, the cheaper solution is to use Image.FromStream, like so (using C# syntax):

stream=File.OpenRead(ImagePathName));
Bitmap bm=Image.FromStream(stream);
stream.Dispose();

Luc Pattyn

Have a look at my entry for the lean-and-mean competition; please provide comments, feedback, discussion, and don’t forget to vote for it! Thank you.

Local announcement (Antwerp region): Lange Wapper? Neen!

TheComputerMan22-Sep-09 12:39

22-Sep-09 12:39

Hi Luc, yes the stream thingy sorted it. I replied this to you on the other bit of the thread. Thanks for all your help.

TheComputerMan22-Sep-09 12:37

22-Sep-09 12:37

Hi Ian, thanks for your assistance. I resolved it!!!! Smile | :)

See my reply to Luc