Resolved Merging multiple files into one using filestream

zackmark29

Active member
Joined
Apr 21, 2020
Messages
28
Programming Experience
Beginner
Hey guys could someone help me fix this codes?
I'm getting a little problem. The output isn't merging the input files



VB.NET:
 Private Sub CopyMyFiles()
        Dim inputFolder As String = txtSource.Text
        Dim outputFolder As String = txtOutputFolder.Text + "\" + txtOuputFilename.Text + ".ts"

        Dim enumFiles = Directory.EnumerateFiles(inputFolder, "*.ts").ToArray

        For Each files In enumFiles

            Dim input As FileStream = New FileStream(files, FileMode.Open, FileAccess.ReadWrite, FileShare.Read, FileOptions.SequentialScan)
            Dim output As FileStream = New FileStream(outputFolder, FileMode.Create, FileAccess.Write, FileShare.ReadWrite, FileOptions.SequentialScan)

            CopyStream(input, output)

        Next

    End Sub

    Public Shared Sub CopyStream(inputStream As Stream, outputStream As Stream)
        Dim buffer = New Byte(1025) {}
        Dim bytesRead As Integer
        bytesRead = inputStream.Read(buffer, 0, buffer.Length)
        While bytesRead > 0
            outputStream.Write(buffer, 0, bytesRead)
            bytesRead = inputStream.Read(buffer, 0, buffer.Length)
        End While
        outputStream.Flush()
        inputStream.Close()
        outputStream.Close()
    End Sub
 
Unless this is homework and you were required to write it, that `CopyStream` method is quite unnecessary. The Stream class already has a CopyTo method. If you are required to write your own method, part of the problem is that that method closes the Streams. The name of that method is CopyStream so that's what it should do and all it should do. What if you wanted to use that method but then use one or both of the Streams again afterwards? You can't, and that is a big part of your problem.

How does it make sense to copy all those input files to the same output file and to close and reopen the output file in between? It should be fairly obvious that opening the output file once at the start and closing it once at the end would be more efficient. This is linked to the issue you asked about. You are using FileMode.Create to open the output file, which is fine if you open it once. If you open it once for each input file then you are overwriting the existing file each time, so of course you're not combining anything.

Fix your CopyStream method first and then fix how you use it. Open and close the FileStreams in the same scope, which means that you can use Using blocks, and only open and close the output file once. I would also suggest that, if a variable is supposed to store the path of an output file, outputFolder is a very bad name for it.
 
I already fixed with this. I didn't notice the flush and close filestream

VB.NET:
Dim counter As Double = 1
Dim inputFolder As String = txtSource.Text
Dim outputFolder As String = txtOutputFolder.Text + "\" + txtOuputFilename.Text + ".ts"

Dim enumFiles = Directory.EnumerateFiles(inputFolder, "*.ts").ToArray
Dim output As FileStream = New FileStream(outputFolder, FileMode.Create, FileAccess.ReadWrite)

For Each files In enumFiles
    Dim input As FileStream = New FileStream(files, FileMode.Open, FileAccess.ReadWrite)
    CopyStream(input, output)
    input.Close()
Next

output.Flush()
output.Close()
 
That's far closer to what I would suggest but there are still improvements to be made.

  • A counter variable of type Double is wrong. Counting is, by definition, performed using integers. As such, that variable should be type Integer. It seems not to be being used anyway.
  • A variable intended to store an output file path should not be named outputFolder. outputFile or, even better, outputFilePath would be preferable.
  • Use the concatenation operator (&) to concatenate, not the addition operator (+).
  • Use the Path.Combine method to combine partial paths.
  • Calling EnumerateFiles and ToArray is nonsensical. If you want an array then call GetFiles. The whole point of EnumeratFiles is that is doesn't get all files and create an array before using them.
  • If you want to create and destroy a disposable object in the same scope, you should use a Using block.
  • While not essential, I would recommend using the File class to create your FileStreams.
With all that in mind, here's the code that I would use:
VB.NET:
Dim inputFolderPath = inputFolderPathTextBox.Text
Dim outputFolderPath = outputFolderPathTextBox.Text
Dim outputFilePath = Path.Combine(outputFolderPath, outputFileNameTextBox.Text & ".ts")
Dim inputFilePaths = Directory.GetFiles(inputFolderPath, "*.ts")

Using outputStream = File.Create(outputFilePath)
    For Each inputFilePath In inputFilePaths
        Using inputStream = File.OpenRead(inputFilePath)
            CopyStream(inputStream, outputStream)
        End Using
    Next
End Using
As you can see, I've changed a number of variable names to be more descriptive of EXACTLY what they represent. Names like enumFiles are also bad because that is the name you would give to a method that enumerates files, not a variable that refers to an array of input file paths.

Another point I meant to mention was this:
VB.NET:
Dim buffer = New Byte(1025) {}
You've got your wires crossed there. Not that the exact size is too important but it is common to use a power of 2 as the size for a Byte array like that. That would mean the Length should be 1024. When you create an array, you specify the upper bound rather than the length, 1 LESS than the length, not one MORE than the length. That should be:
VB.NET:
Dim buffer As New Byte(1023) {}
or, as many people prefer:
VB.NET:
Const BUFFER_LENGTH As Integer = 1024
Dim buffer As New Byte(BUFFER_LENGTH - 1) {}
You can then use that same constant later, instead of getting the Length of the array.
 

Sir could tell me what else I can improve with this codes?
I tried everything and change codes but still the process is slow on 1GB files

here's the code:

VB.NET:
 Private Sub BGW_DoWork(sender As Object, e As DoWorkEventArgs) Handles BGW.DoWork
        outputPath = Path.Combine(txtOutputfolder.Text, txtOutputFilename.Text & ".ts")
        Dim newWorker As BackgroundWorker = DirectCast(sender, BackgroundWorker)
        Dim AES As AesCryptoServiceProvider
        Dim IV(15) As Byte
        Dim KEY As Byte() = File.ReadAllBytes(txtKeyFile.Text)

        ifFileExists()

        Try
            Decrypting = True
            outputFilestream = New FileStream(outputPath, FileMode.Create, FileAccess.Write)
            For Each item As Object In ListView1.Items
                sourceItems = CType(item, SourceItem)
                Dim inputFiles As String = sourceItems.Filename
                Dim countFiles As Long = ListView1.Items.Count

                inputFilestream = New FileStream(inputFiles, FileMode.Open, FileAccess.Read)

                AES = New AesCryptoServiceProvider With {
                    .Key = KEY,
                    .IV = IV,
                    .Mode = CipherMode.CBC}

                Dim Decryptor As ICryptoTransform = AES.CreateDecryptor()
                cryptoStream = New CryptoStream(inputFilestream, Decryptor, CryptoStreamMode.Read)

                While inputFilestream.Position < countFiles
                    mReset.WaitOne() 'pause or resume process
                    cryptoStream.CopyTo(outputFilestream) 'main file process
                    newWorker.ReportProgress(CInt(counter / countFiles * 100)) 'progressbar
                    counter += 1

                    If pauseProcess Then
                        ss.Text = "Aborting process..."
                        mReset.Reset() 'pause the process

                        Dim r = MessageBox.Show("Do you to abort the process?", "Decypting in progress",
                                        MessageBoxButtons.YesNo, MessageBoxIcon.Exclamation)
                        If r = DialogResult.No Then
                            ss.Text = "Resuming"
                            pauseProcess = False
                            mReset.Set() 'resume the process
                        ElseIf r = DialogResult.Yes Then
                            If BGW.IsBusy Then
                                BGW.CancelAsync()
                                BGW.Dispose()
                                If BGW.CancellationPending = True Then
                                    e.Cancel = True
                                    Exit For
                                End If
                            End If
                            Decrypting = False
                            Return
                        End If
                    End If
                End While
            Next
            outputFilestream.Flush()
            outputFilestream.Close()
            outputFilestream.Dispose()
            cryptoStream.Flush()
            cryptoStream.Close()
            cryptoStream.Dispose()
        Catch ex As Exception
            MsgBox(ex.Message, vbCritical, "Error")
            erroDec = True
        Finally
            If inputFilestream IsNot Nothing Then
                inputFilestream.Close()
                inputFilestream.Dispose()
            End If
            If outputFilestream IsNot Nothing Then
                outputFilestream.Close()
                outputFilestream.Dispose()
            End If
        End Try
        Decrypting = False
    End Sub
 
I tried everything and change codes but still the process is slow on 1GB files
Is it though? Of course bigger files are going to take more time to process but it's not slow if it's a linear or close-to-linear increase. If increasing the file size ten times increases the time taken ten times then that's not slow; it's exactly the same speed. You're going to have to provide some numbers to demonstrate that it actually is slow.
 
Is it though? Of course bigger files are going to take more time to process but it's not slow if it's a linear or close-to-linear increase. If increasing the file size ten times increases the time taken ten times then that's not slow; it's exactly the same speed. You're going to have to provide some numbers to demonstrate that it actually is slow.

Ah alright so it's really normal. I thought there's wrong with using filestream.

Could you tell me about using block? as you mention in previous message
 
Back
Top