Export to GitHub

mp4v2 - issue #50

Extremely Poor Performace with Native Win32 - Std Fileprovider


Posted on Jan 11, 2010 by Swift Camel

What steps will reproduce the problem? 1. Open file for modification 2. copy samples to single file and close 3. Repeat this 20 to 50 times (or more)

I upgraded my libmp4v2 library from a trunk version dated 2008-12-18.

The modifications for cross-platform abstraction for file I/O and the switch to Native windows function (CreateFileA ) creates serious file read write performance degradation.

Reading about 40 audio files (MP4Modify, Read/write samples, MP4Close ... do again for next file and so on) and combining them in a single m4a file takes 50 secs using the old file routines. The new routines take over 5 minutes and nearly max out /hang the cpu.

I reverted all the file routines back to before the R296 change. Performance was restored.

What is the expected output? What do you see instead? reasonable read/write performance for multiple open/close of files.

What version of the product are you using? On what operating system? Version 1.9, 1.9.1 and trunk R355 all exhibit the problem. Tried on WinXP SP3 and Win7

Please provide any additional information below.

Comment #1

Posted on Feb 8, 2010 by Massive Cat

I have a fix for this, but I really can't figure out how to commit changes. The problem appears to be that for some reason which I can't figure out, using the Win32 API doesn't buffer, so each of the tiny reads goes back to the disk. What I did was incorporate a 4K buffer in the file_win32.cpp file. It makes it much, much faster in both reading and writing. I've attached my file. Trying building with it and see if it fixes the performance problem.

Dan

Attachments

Comment #2

Posted on Sep 22, 2010 by Happy Lion

jeffreyloden,

Could you try this again with the most recent trunk? And is there some way you could give me your test code so I could try and repeat it? There were some more changes to the native windows functions (mostly to deal with unicode), but I'd like to know if you still have the same issues.

One thought I had is to improve the file provider abstraction so users can more easily create their own read/write routines (in particular, I'd like to write one using Qt).

Thanks, kidjan

Comment #3

Posted on Mar 18, 2012 by Massive Cat

Just to see, I tried this with the most current rev, and the problem still exists. Not sure if there are any developers that run on Windows, but if so, my fix greatly speeds up reads and writes. I would be glad to work with anyone that would be interested in getting this into the sources.

Very easy to test, on Windows run a read and a write with and without the changes and notice the speedup. I've got thousands of users using my version and I've never had a problem with it.

Dan

Comment #4

Posted on Mar 19, 2012 by Happy Lion

What happens if you change the flags used? For example, see:

http://msdn.microsoft.com/en-us/library/aa363858.aspx#caching_behavior

...in particular,

"Specifying the FILE_FLAG_SEQUENTIAL_SCAN flag can increase performance for applications that read large files using sequential access. Performance gains can be even more noticeable for applications that read large files mostly sequentially, but occasionally skip forward over small ranges of bytes. If an application moves the file pointer for random access, optimum caching performance most likely will not occur. However, correct operation is still guaranteed."

Looks like the code is currently using FILE_ATTRIBUTE_NORMAL (see http://code.google.com/p/mp4v2/issues/attachmentText?id=50&aid=-802417126500239040&name=File_win32.cpp&token=4idgdzVy-fM3qodZaF02-OC04Xs%3A1332118380013#46 ); might be interesting to see what happens if you put in FILE_FLAG_SEQUENTIAL_SCAN since most MP4 file reading probably "read(s) large files mostly sequentially, but occasionally skip forward over small ranges of bytes"

Not opposed to the patch, but I'd prefer a solution that leveraged the existing file API rather than caching in user land.

Comment #5

Posted on Mar 19, 2012 by Happy Lion

Also, this sort of sheds some light on this API:

http://blogs.msdn.com/b/oldnewthing/archive/2012/01/20/10258690.aspx

So, the weird thing is if an application passes in neither FILE_FLAG_SEQUENTIAL_SCAN or FILE_FLAG_RANDOM_ACCESS, then the underlying API goes into some sort of heuristic mode where it tries to optimize performance based on the calls coming in. Also, it seems that this behavior may not be consistent across Windows OSs.

It has to be said: what a shit API this is. The documentation alone really shows you how convoluted it is. I wouldn't be opposed to going back to the more portable way (fopen/fread/fseek) and doing away with this windows specific file provider all together.

Comment #6

Posted on Mar 19, 2012 by Quick Kangaroo

Going back to fopen/fread/fseek has some pretty big drawbacks too: can't read filenames with non-ascii characters and can't read filenames that live in long paths.

I've been super delinquent in not trying Dan's patch for ages. There may be some light at the end of the tunnel for me but given my past performance, is there someone else who can take a look at his patch and work through any changes (if there are any) with him and get it committed?

Thanks much.

-DB

Comment #7

Posted on Mar 19, 2012 by Swift Lion

Before we do any patches, somebody needs to at least try FILE_FLAG_SEQUENTIAL_SCAN to see if that resolves the performance issues.

Comment #8

Posted on May 21, 2012 by Happy Lion

@dbyron,

I was under the impression that fopen accepts UTF8? If that's true, seems like you could read filenames with non-ascii characters, but maybe I'm misinformed about this.

Comment #9

Posted on May 21, 2012 by Quick Kangaroo

I don't think fopen accepts UTF-8. See the attached test program. I created three text files corresponding to the three files it tries to open. fopen opens the one with the ascii name, but not the ascii ones.

Attachments

Comment #10

Posted on Jul 10, 2013 by Helpful Kangaroo

Hi,

I use _wfopen() to handle Unicode filenames. _wfopen() can't handle very long paths (longer than MAX_PATH characters), but I

think it's acceptable because Explorer can not also handle such long paths.

Ken Takata

Attachments

Comment #11

Posted on Aug 5, 2013 by Massive Cat

I just tested out using _wfopen, and it has as good a performance as my fix, and since it's an OS API instead of my home rolled buffering, it seems like the right answer. Can someone put this in the next release?

Dan

Comment #12

Posted on Aug 16, 2013 by Happy Lion

happily, if I can get it as a diff?

Comment #13

Posted on Aug 16, 2013 by Helpful Kangaroo

OK, I have attached the diff.

Ken Takata

Attachments

Comment #14

Posted on Jan 13, 2015 by Helpful Kangaroo

Hi,

I have updated the patch for R507.

Ken Takata

Attachments

Status: New

Labels:
Type-Defect Priority-Medium