Add WAV content filter #982
Conversation
torusrxxx
force-pushed
the
WAVFilter
branch
from
e6d60c8
to
5bb6fff
Compare
|
That test needs to be at least a dozen tests, and none of them should be green if you remove all the code from the filter! 😄 |
| addMIMEType((short)623, "image/avif", "avif"); | ||
| addMIMEType((short)624, "image/heic", "heic"); | ||
| addMIMEType((short)625, "image/heif", "heif"); |
There was a problem hiding this comment.
These change appear to be unrelated to this PR.
| @@ -123,6 +124,11 @@ false, false, new M3UFilter(), false, false, false, false, false, false, | |||
| register(new FilterMIMEType("audio/mpeg", "mp3", new String[] {"audio/mp3", "audio/x-mp3", "audio/x-mpeg", "audio/mpeg3", "audio/x-mpeg3", "audio/mpg", "audio/x-mpg", "audio/mpegaudio"}, | |||
| new String[0], true, false, new MP3Filter(), true, true, false, true, false, false, | |||
| l10n("audioMP3ReadAdvice"), false, null, null, false)); | |||
|
|
|||
| // WAV - has a filter | |||
| register(new FilterMIMEType("audio/vnd.wave", "mp3", new String[] {"audio/vnd.wave", "audio/x-wav", "audio/wav", "audio/wave"}, | |||
There was a problem hiding this comment.
This mentions "mp3" even though it is about WAV.
| return new byte[] {'W', 'A', 'V', 'E'}; | ||
| } | ||
|
|
||
| class WAVFilterContext { |
There was a problem hiding this comment.
This class is not used elsewhere, please make it private.
Additionally, the fields in this class should not be declared public as they are not effectively accessible outside this class.
Either make them private, or of you prefer a more compact look, make them package-private (by just omitting public).
| } | ||
| } | ||
| return output; | ||
| } |
There was a problem hiding this comment.
Indentation is off in various places in this class. Please either use 4 spaces or 1 tab for indentation, and don't mix both styles.
| if((size & 1) != 0) // Add padding if necessary | ||
| output.writeByte(input.readByte()); |
There was a problem hiding this comment.
Please use braces around if/then/else blocks, even if they are otherwise one single line.
| if(ctx.hasfmt) { | ||
| throw new DataFilterException(l10n("invalidTitle"), l10n("invalidTitle"), "Unexpected fmt chunk was encountered"); | ||
| } | ||
| if(size != 16 && size != 18 && size != 40) { |
There was a problem hiding this comment.
What do these magic numbers represent? (please introduce constants with descriptive names)
| throw new DataFilterException(l10n("invalidTitle"), l10n("invalidTitle"), "Unexpected header chunk was encountered, instead of fmt chunk"); | ||
| } | ||
| if(ID[0] == 'd' && ID[1] == 'a' && ID[2] == 't' && ID[3] == 'a') { | ||
| if(ctx.format == WAVE_FORMAT_PCM || ctx.format == WAVE_FORMAT_IEEE_FLOAT || ctx.format == WAVE_FORMAT_ALAW || ctx.format == WAVE_FORMAT_MULAW) { |
There was a problem hiding this comment.
This condition will always evaluate to true (ctx.format is only written in line 51 and checked immediately after)
WAV PCM audio is such a simple and safe format. RIFF format is supported so why not WAV files?