DeaDBeeF-Player / deadbeef

DeaDBeeF Player
https://deadbeef.sourceforge.io/
Other
1.63k stars 177 forks source link

Converter WAV issues (endianness, rounding, etc.) #1139

Closed Oleksiy-Yakovenko closed 9 years ago

Oleksiy-Yakovenko commented 9 years ago

Original issue 1237 created by Alexey-Yakovenko on 2014-11-23T15:12:49.000Z:

The RIFF WAV header constructed by the converter appears to be in native-endian format. It should always be in little-endian format. Does anyone have a big-endian machine that can look at this? I can't imagine it would work well. Big-endian RIFF files are theoretically possible, but should have a magic word of FFIR or RIFX. I don't think WAV headers can be big-endian ever.

The file length placed in the RIFF header is an approximation when a pipe is used, since it is calculated in advance rather than being an actual file size. It is often wrong for Flac files because their playitems don't contain an endsample except from cuesheets (should they, it looks easy?), but also for output through DSP where samplerate changes can cause rounding errors. This doesn't cause problems for any encoders I've tested with, but they have to be set to ignore header sizes anyway. With a temporary file, the chunk size is written in after completion and is exact, but the RIFF file size is still left as an approximation which means RIFF WAV output files are sometimes technically invalid, although I haven't found a player that refuses to play them. Some tools do report header problems.

When encoding through a pipe, four spurious bytes are written to the pipe at the end of the audio data. This is because the converter tries to fill in the correct chunk size in the temporary file, but seeking in a pipe fails and the bytes are just appended. I imagine this data will be discarded in most cases since it won't form a complete sample (Flac warns about this extra data if it is given the chunk size and the --ignore-chunk-size parameter is not set), but for 8 bit or mono it would look like a sample and since the chunk size is zero the encoder cannot know it is garbage.

Oleksiy-Yakovenko commented 9 years ago

Comment #1 originally posted by Alexey-Yakovenko on 2014-11-23T16:16:09.000Z:

The WAVE extensible header should also strictly be used for any file which has more than 16 bits per sample or more than 2 channels, but is only used in the converter for files containing float samples. I haven't found a player that refuses to play these files with the short header, but haven't done much testing with multiple channels. Deadbeef plays them OK :)

Oleksiy-Yakovenko commented 9 years ago

Comment #2 originally posted by Alexey-Yakovenko on 2014-11-23T20:16:43.000Z:

yeah, there are quite a lot of places in deadbeef which would break on big endian. I had a big endian machine when I started making deadbeef (ps3linux), but I don't have it anymore, so on one hand I can fix the issue, on the other hand big-endian machines are pretty much extinct, so nobody cares.

Oleksiy-Yakovenko commented 9 years ago

Comment #3 originally posted by Alexey-Yakovenko on 2014-12-09T19:09:31.000Z:

Turns out it is fairly easy to support big-endian machines with a few macros so there is no impact on little-endian. I have no way to properly test it though.

wtermini commented 9 years ago

I have a Big-Endian PPC iMac G4 running Debian, I just tried to play a FLAC file on it and all I hear is hissing. If you can fix this I would be able to test it.

Oleksiy-Yakovenko commented 9 years ago

@T3RM1NU5 could you try if any other file format plays correctly?

If not, most likely the code in ALSA plugin is wrong, but can be fixed quite easily.

I don't have access to big-engian hardware anymore, so can't test locally, but I can make a patch for you to try out.

wtermini commented 9 years ago

@Alexey-Yakovenko I tried some some files ogg ,ape and chiptunes with the game music emu library. All are working fine.

The only file type I have the issue with is FLAC. Ill be happy to test a patch.

Oleksiy-Yakovenko commented 9 years ago

ok, if it's flac only - the problem is not what I thought. It makes sense to file a separate bug about it.

Oleksiy-Yakovenko commented 9 years ago

closing this issue for split into multiple bugs