On 12/15/2014 11:24 AM, Carl Eugen Hoyos wrote:
Thomas Volkert <silvo <at> gmx.net> writes:
+#include <netinet/in.h>
This will hopefully be unneeded.
Okay, this will simplify the patch.
if (size >= 18) { /* We're obviously dealing with WAVEFORMATEX */
+ if (big_endian)
+ avpriv_report_missing_feature(codec,
"WAVEFORMATEX support for RIFX files\n");
Is this sufficient, no further error handling needed?
I do not have an example file in RIFX format with WAVEFORMATEX. So, I
concentrated on the usual file format.
The sent patch does not influence the usual RIFF decoder but it extends
it by a first support for big endian values.
Maybe someone else can extend my patch with support for WAVEFORMATEX in
the future.
- if (!memcmp(p->buf, "RIFF", 4))
+ if ((!memcmp(p->buf, "RIFF", 4)) || (!memcmp(p->buf, "RIFX", 4)))
Maybe I misread but this looks like too many parentheses.
The compiler would not accept the following construction:
"if (!memcmp(p->buf, "RIFF", 4)) || (!memcmp(p->buf, "RIFX", 4))"
- int rf64;
+ int rf64 = 0;
Should be unneeded.
In the previous version of the code, the variable rf64 was initialized
in every case by the following:
"rf64 = tag == MKTAG('R', 'F', '6', '4');"
I simplified the code and replaced this with a default value, which gets
overwritten, if the file header has RF64 format
- /* check RIFF header */
- tag = avio_rl32(pb);
nit: You could not remove the variable and do a switch (tag)
below to make the patch smaller.
(Smaller patch == easier review, both now and in the future)
The variable "tag" is initialized by "size = next_tag(pb, &tag,
wav->rifx);" before the following switch-case.
+ wav->rifx = 0;
Should be unneeded.
Okay, we can rely on the zero-initialization of the WAVDemuxContext
structure.
Best regards,
Thomas.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
