Is *.wav loading broken?

Hi,
I have been using the Juce WAV loading code for some time - but I don’t remember when I last tested it - and now I can’t seem to get anything reasonable out of it.
Whatever I read, it seem broken or garbage. I don’t think it’s a casting issue between 32 bit int and 32 bit floats, since my unchanged code used to work.

Does anyone else see this in the latest tip? Or am I just crazy? Or both?

Thanks, Michael

Reverting to the older (5 days) version of juce_AudioDataConverters.h definitely puts things back in order again. Could you confirm something was up Jules?

The fail occur in both VS2012 and Xcode 4

Drat, sorry! Fixed now!

Thank you! I hope you didn’t have to sacrifice any of those compiler optimizations.
Edit. It would seem you reverted all changes. Sorry about that… Oups. :oops:

I’m actually not 100% sure which of those changes broke it, so rolling everything back was the easiest plan to get things going quickly. I might have another try at refactoring it some time, but the changes weren’t important, I was just playing around with some tricks I learned recently about how to write code that compilers can optimise.

[size=150]SHARE PLEASE!!![/size]

I’ll add some notes about this to the coding standards page when I get chance - just a few interesting tips that I picked up at the C++now conference.

http://cpp-next.com/archive/2009/08/want-speed-pass-by-value/ ?

This is mostly a hey boy there won’t be any aliasing to the compiler as well.

I don’t entirely agree with that article. A class method like so is pretty naive, of course;

std::vector<std::string> get_names();

But a direct const-reference is better since the names are likely to be a massive chunk of data (and, with a well designed class and a const-caring programmer [[size=70]ahem - not using const_cast() and std::remove_const[/size]], the data should stay immutable…)

It’s also less work to write, and is more user-friendly (and aesthetically pleasing, imho… no extra “out” variables) than RVO.

const std::vector<std::string>& get_names() const { return names; }

IMHO jules only changed the references to “pass by values” where the class data isn’t bigger than a reference (“pointer”) would be, like colour data-type…
This is one aspect…

The other thing, but completely different, is to help the compiler to use new c++ move semantics, so that in the end “std::vectorstd::string get_names();” has no additional costs than a reference.
(as long you didn’t define a extra move semantic constructor “rule of 5” which may uses a lot of extra cpu)

FYI I added a note about this on the coding standards page:

http://www.juce.com/documentation/coding-standards

The basic thing is that when you pass a reference to a function, the compiler must assume that the object might be modified by the function, which dramatically reduces its ability to optimise the calling code. There was a very good talk about this at C++Now (the video’s not available yet, but I recommend watching it when it is)

Will you begin to add first experimental c++11 features? (std::function etc…)
something like addFunctionOnClick() for a button would be cool…

[quote]FYI I added a note about this on the coding standards page:

http://www.juce.com/documentation/coding-standards

The basic thing is that when you pass a reference to a function, the compiler must assume that the object might be modified by the function, which dramatically reduces its ability to optimise the calling code. There was a very good talk about this at C++Now (the video’s not available yet, but I recommend watching it when it is)[/quote]

What happens to PODs created with double; such as juce::Range? Such is sizeof(double) * 2 * 8 bits; does it get optimised the same way as, say, an int or float would when copied? Or is that scenario worth passing by reference? (Though I guess I could check the generated assembly…)

No, I don’t think it’d ever be sensible to pass it by reference. On 64-bit machines the compiler would probably pass it in registers as two doubles anyway, which would be vastly better than a pointer to memory.

When I get chance I’ll probably also change all the Rectangles to be by-value too, though it’d be interesting to see some actual measurements about how this affects things.

[quote=“jules”]FYI I added a note about this on the coding standards page:
http://www.juce.com/documentation/coding-standards
[/quote]

I think you’d also better update the part where you say: “Object parameters should be passed as const references wherever possible. Only pass a parameter as a copy-by-value object if you really need to mutate a local copy inside the method, and if making a local copy inside the method would be difficult.”, in order to warn the users that it does not apply for small objects.

Ah yes, thanks for spotting that!

Augh! I was going crazy over this wondering what I broke. Thanks for the fix.