我发现我的过去的版本太有限了。他们需要记录一定的时间范围,然后停止。这对于许多现实世界的应用程序来说不太实用,因此我重写了所有内容,使计算机始终处于侦听状态,只记录必要的数据。
我想回顾一下:
内存消耗:显然,一个巨大的潜在问题是在录制音频并将音频保存到文件时会消耗大量空间。我是在浪费空间吗?每一点都很重要。注意:文件存储容器必须为FLAC。
速度:我必须实时处理数据。我什么都挂不着,否则我可能会丢失宝贵的音频数据。
语法/样式:我的代码看起来如何?怎么样才能使其看起来更好?我在语法上做错了吗?
也可以随意复习其他内容,我希望复习重点放在上面。请记住,我已经离开C语言了一段时间了,所以我可能已经忘记了一些更简单的东西;)
#include <stdio.h>
#include <stdlib.h>
#include <math.h>
#include <string.h>
#include <time.h>
#include <portaudio.h>
#include <sndfile.h>
#define FRAMES_PER_BUFFER 1024
typedef struct
{
uint16_t formatType;
uint8_t numberOfChannels;
uint32_t sampleRate;
size_t size;
float *recordedSamples;
} AudioData;
typedef struct
{
float *snippet;
size_t size;
} AudioSnippet;
AudioData initAudioData(uint32_t sampleRate, uint16_t channels, int type)
{
AudioData data;
data.formatType = type;
data.numberOfChannels = channels;
data.sampleRate = sampleRate;
data.size = 0;
data.recordedSamples = NULL;
return data;
}
float avg(float *data, size_t length)
{
float sum = 0;
for (size_t i = 0; i < length; i++)
{
sum += fabs(*(data + i));
}
return (float) sum / length;
}
long storeFLAC(AudioData *data, const char *fileName)
{
uint8_t err = SF_ERR_NO_ERROR;
SF_INFO sfinfo =
{
.channels = data->numberOfChannels,
.samplerate = data->sampleRate,
.format = SF_FORMAT_FLAC | SF_FORMAT_PCM_16
};
SNDFILE *outfile = sf_open(fileName, SFM_WRITE, &sfinfo);
if (!outfile) return -1;
// Write the entire buffer to the file
long wr = sf_writef_float(outfile, data->recordedSamples, data->size / sizeof(float));
err = data->size - wr;
// Force write to disk and close file
sf_write_sync(outfile);
sf_close(outfile);
puts("Wrote to file!!!!");
return err;
}
int main(void)
{
PaError err = paNoError;
if((err = Pa_Initialize())) goto done;
const PaDeviceInfo *info = Pa_GetDeviceInfo(Pa_GetDefaultInputDevice());
AudioData data = initAudioData(44100, info->maxInputChannels, paFloat32);
AudioSnippet sampleBlock =
{
.snippet = NULL,
.size = FRAMES_PER_BUFFER * sizeof(float) * data.numberOfChannels
};
PaStream *stream = NULL;
sampleBlock.snippet = malloc(sampleBlock.size);
time_t talking = 0;
time_t silence = 0;
PaStreamParameters inputParameters =
{
.device = Pa_GetDefaultInputDevice(),
.channelCount = data.numberOfChannels,
.sampleFormat = data.formatType,
.suggestedLatency = info->defaultHighInputLatency,
.hostApiSpecificStreamInfo = NULL
};
if((err = Pa_OpenStream(&stream, &inputParameters, NULL, data.sampleRate, FRAMES_PER_BUFFER, paClipOff, NULL, NULL))) goto done;
if((err = Pa_StartStream(stream))) goto done;
for(int i = 0;;)
{
err = Pa_ReadStream(stream, sampleBlock.snippet, FRAMES_PER_BUFFER);
if (err) goto done;
else if(avg(sampleBlock.snippet, FRAMES_PER_BUFFER) > 0.000550) // talking
{
printf("You're talking! %d\n", i);
i++;
time(&talking);
data.recordedSamples = realloc(data.recordedSamples, sampleBlock.size * i);
data.size = sampleBlock.size * i;
if (data.recordedSamples) memcpy((char*)data.recordedSamples + ((i - 1) * sampleBlock.size), sampleBlock.snippet, sampleBlock.size);
else
{
free(data.recordedSamples);
data.recordedSamples = NULL;
data.size = 0;
}
}
else //silence
{
double test = difftime(time(&silence), talking);
if (test >= 1.5 && test <= 10)
{
char buffer[100];
snprintf(buffer, 100, "file:%d.flac", i);
storeFLAC(&data, buffer);
talking = 0;
free(data.recordedSamples);
data.recordedSamples = NULL;
data.size = 0;
}
}
}
done:
free(sampleBlock.snippet);
Pa_Terminate();
return err;
}
编译为:
gcc main.c -lsndfile -lportaudio
(假设您已经安装了那些库)#1 楼
确实,您一定已经在这个项目上工作了一段时间,因为一段时间前我回答了与您的一个旧版本有关的SO问题。到目前为止,至少我仍然得到了一些反馈。代码和样式注释>
问题已经在这里开始。不使用
uint32_t
时,会大量使用#include <stdint.h>
和其他此类类型。我认为这是可行的,因为您可能仍在Mac OS X上,并且这也可以解释为什么您的GCC在没有-std=gnu99
的情况下不会抱怨C99循环的初始声明。使用指针算法,这使得代码难以阅读。我绝对喜欢@nhgrif的平均值版本;我发现sum += fabs(data[i]);
的使用非常方便。avg()
功能至少还有其他一些问题。首先,如果您想计算某人说话的时间,即使使用fabs()
这样的gi头,我也不会进行平均。我会改用RMS(均方根),然后从RMS值中减去常规平均值(无fabs()
);这将计算缓冲区中的音频“功率”,同时忽略任何直流偏置(非零平均值),您的代码对其不强健。通过与用来“扩音”来自麦克风的输入相同的幂定律(可以是A律或U律,或者其他),可以将其直接连接到样本中的分贝数。 br />第二,和相关;在1024帧和44100Hz的采样率下,缓冲区的时间仅为23毫秒;换句话说,只有一个完整的频率为43 Hz的声音循环。我不知道@ rob0t的声音有多低,但是如果您要处理低频内容,则可能要增加该数量。在计算RMS和DC(平均值)时,只有准确捕获所需频率的整数个周期,您才能获得准确的结果。但是,如果您的缓冲区跨越了您感兴趣的所有频率的足够的周期,那么平均而言,部分周期所引入的任何误差都会被完整的平均周期所淹没。累加器是float
。我怀疑您在这里不太关心精度,但是您可能想知道,因为IEEE浮点算术与实际数学并不完全一样,所以添加浮点值的顺序可能会产生很小的差异。特别是,如果您要合计1024个相同的值,则在最后一次累加时,您将失去24位精度中的大约10位。这是因为累加器将增长到累加器大小的2 ^ 10倍,添加到累加器中将仅使用14个MSB,同时舍入10个LSB。除非已向我证明double
是性能瓶颈,并且矢量化已经完成了所有可能的工作,否则我将在这里使用avg()
。 /> return (float) sum / length;
完全是多余的:
sum
已经是浮点数,您将其再次转换为float
。如果您想使用浮点数学进行除法,则已经可以保证;在C语言中,在隐式转换中,浮点值比任何整数都高。请记住,强制类型转换不适用于操作(/
),但不适用于其操作数(此处为sum
)。storeFLAC()
不会在发生错误的情况下进行日志记录,因此我没有立即解释为什么基于Linux的代码只能成功创建零长度文件。返回的错误代码将被完全忽略。#include <stdio.h>
#include <stdlib.h>
#include <math.h>
#include <string.h>
#include <time.h>
#include <portaudio.h>
#include <sndfile.h>
由于早期
return
语句周围没有括号,因此审阅者更难添加他们自己的调试代码,因为在调试printf()
之上添加花括号的工作量很大。非常感谢您的审阅者,包括您将来的自我。main()
迫切需要重构,尤其是为每个主要步骤提取一些功能。您编写的代码对我不“说话”;我必须付出一些努力来了解它的作用。至少大约是一个筛选,在我看来这是一个函数的不错选择。这是对
goto
的严重误用:虽然我确实赞成goto
用于错误处理,但是您在这里犯下了跳过局部变量的初始化,然后又费心使用它们的罪过。不能保证在尝试sampleBlock.snippet
时会初始化free()
:如果PortAudio未能初始化,您甚至都没有完成sampleBlock
的初始化! Clang的-Wall -Wextra
标志要求进行许多有用的诊断。此错误报告为SNDFILE *outfile = sf_open(fileName, SFM_WRITE, &sfinfo);
if (!outfile) return -1;
// Write the entire buffer to the file
。但是请注意,仅在启用优化后才会显示此警告;这是因为分析通过了GCC的要求,才能发现未初始化的用途仅在
-O1
和更高版本上运行。因此,可能有必要定期在高优化级别上进行构建,在此过程中,GCC会投入更多的精力进行分析,并且可以作为副作用发现潜在的错误。 /> int main(void)
{
PaError err = paNoError;
if((err = Pa_Initialize())) goto done;
const PaDeviceInfo *info = Pa_GetDeviceInfo(Pa_GetDefaultInputDevice());
AudioData data = initAudioData(44100, info->maxInputChannels, paFloat32);
AudioSnippet sampleBlock =
{
.snippet = NULL,
.size = FRAMES_PER_BUFFER * sizeof(float) * data.numberOfChannels
};
...
done:
free(sampleBlock.snippet);
Pa_Terminate();
return err;
}
很明显,如果您需要向右滚动,则大大超出了我的80列软限制。您也没有在语句中花括号,并且在这里进行不错的调试
printf
会很有帮助。我当然想知道我的流是否无法打开。对于带有大量带有长名称的参数的函数,我倾向于将它们放下一行。不要害怕在上面花几行。
恭喜您使用
difftime()
方便,安全地确定时差。我学到了一些东西。但是,通常将time_t
定义为自大纪元以来的整数秒数。由于您将以每秒43次的速度调用time()
,因此许多连续的调用会给出相同的时间,因此相差0
s,而有些调用却相差1
s,尽管实际上只是相隔23
ms。 br /> 这里最好使用分辨率更高的壁钟计时器。对于这种类型的计时,我建议
gettimeofday()
;对于极端分辨率,我建议在Linux上使用clock_gettime()
,在Mac OS X上建议使用更容易使用且开销较小的mach_absolute_time()
。这很疯狂:
portaudio.c: In function ‘main’:
portaudio.c:135:6: warning: ‘sampleBlock.snippet’ may be used uninitialized in this function [-Wmaybe-uninitialized]
free(sampleBlock.snippet);
^
首先,这里的一行再次过长。您在这里使用了臭名昭著的
x = realloc(x, ...)
惯用语,这很糟糕,因为如果realloc()
失败,则会泄漏内存。在memcpy()
内部的任何memcpy()
的顶部。撇开每23毫秒的语音,您可能会realloc()
并复制到目前为止收集的整个数据数组,这一事实使我严重质疑甚至根本不需要移动任何数据。您应该有可能自己构造一个双缓冲区,或更普遍地说是一个循环缓冲区,并完全避免任何复制。然后,最重要的是,再次查看您的条件语句。如果
realloc()
确实是data.recordedSamples
(因此,您已经失去了对该内存块的最后一个引用,就已经泄漏了内存),为什么您要在NULL
指向free()
指针,然后再次将指向NULL
的指针设置为NULL
?这是完全无效的。设计注释
您的代码中通常缺少文档。了解某些事物的优点会有所帮助。例如,我可以很快地认为您仅以
NULL
格式进行记录,因为您未记录的结构仅使用float
;但是,根据您或PortAudio的说法,样本,摘要或框架到底是什么?信道的交织方案是什么(如果有)? float*
和AudioSnippet
之间的根本区别是什么迫使您为它们两者制作不同的结构?鉴于您强调一点都不放弃任何音频数据,我建议您使此应用程序成为线程。一个线程会将数据读入循环缓冲区,并以可忽略的计算复杂度执行任务(假定RMS将花费很少的时间以至于永远不会成为瓶颈)。如果此线程检测到语音,则可以向工作线程发送消息,以指定的偏移量开始读取此缓冲区并将其编码为FLAC文件。当然,如果工作线程太慢,则最终读取线程将耗尽循环缓冲区,并且必须阻塞,可能会丢帧。
评论
\ $ \ begingroup \ $
欢迎使用代码审查!我很高兴您在网上跟踪我,将您引到了这里;)。
\ $ \ endgroup \ $
–syb0rg
15年3月21日在4:44
\ $ \ begingroup \ $
好的第一答案,因为它是最完整的答案,所以它也应该获得更多好评。
\ $ \ endgroup \ $
–edmz
15年3月21日在16:53
\ $ \ begingroup \ $
我赞成这一点,因为我在传递“热门文章”侧栏时看到了它,并且它通常包含大量有用的设计信息。好东西。
\ $ \ endgroup \ $
–凯尔·巴兰(Kyle Baran)
15年3月21日在22:01
\ $ \ begingroup \ $
@ syb0rg我回到了这个答案,并添加了一些技巧来使您的信号处理更加可靠。他们在3.以下,是您的avg()的讨论。
\ $ \ endgroup \ $
– Iwillnotexist Idonotexist
2015年3月25日在21:31
#2 楼
如果它走路像数组,说话像数组,请像数组一样使用它。为什么我们不把它当作下标并使用下标来访问它?是一个data
)。不要欺骗无括号的1行
return
语句。float avg(float *data, size_t length)
{
float sum = 0;
for (size_t i = 0; i < length; i++)
{
sum += fabs(*(data + i));
}
return (float) sum / length;
}
虽然我可以非常简单的单行语句在同一行上且没有括号,这很荒谬。这是一行,因为您欺骗和内联了太多东西。
float avg(float *data, size_t length)
{
float sum = 0;
for (size_t i = 0; i < length; ++i)
{
sum += fabs(data[i]);
}
return (float) sum / length;
}
这更好(尽管我选择的变量名可能没有意义)。
叫
sum
而不是float
我们已经把
if
塞满了。最终,这使我们进入了罕见的场景,其中main()
实际上是在函数内组织代码的一种非常简洁的方法。我再说一遍:这实际上是使用a的一种相对简洁的方法
everything()
语句。立即选择的方法是深度嵌套的main()
语句,这实际上根本不是干净的。但是有一种更好的方法可以使代码更整洁,并且具有消除所有
goto
的副作用:不要塞满goto
变得非常重要!再增加一些功能。调出您的功能。并且让您的函数拥有较早的
if
。函数可帮助读者比较代码块。您在
goto
功能上做得很好。 “ AVG”是平均的常见缩写。我可以仅凭函数名称来猜测它的作用,而我不必通读6条简单的逻辑线就能知道它在做什么。我所要做的只是看到您正在拨打main()
,而我知道发生了什么。因此,找到了更多方法来将代码分解和划分为更多易于消化的块,以使读取操作可以仅查看函数名称并了解此处发生的事情的本质,而无需查看每一行可执行代码。
评论
\ $ \ begingroup \ $
Re“为什么我们不将其像对待的那样使用下标进行访问?”因为对于我们中的某些人来说,指针和下标一样自然,并且通常更容易理解?
\ $ \ endgroup \ $
–jamesqf
15年3月20日在5:03
\ $ \ begingroup \ $
我必须同意...虽然我可以很容易地阅读*(ptr + i),但是变量名并不能像这样传达它,因此作为代码审阅者,我必须去寻找它的声明和赋值。确保...而buf [i]在初次读取时是清楚的。最好只做* ptr ++,直到ptr达到其初始值+长度为止。
\ $ \ endgroup \ $
–科技龙
15年3月20日在5:10
\ $ \ begingroup \ $
甚至不要欺骗一个衬里上的牙套。 “任何情况下都需要{}”。现在,拥有大括号可以防止将来在更改条件时出错。我也相信它可以帮助代码有条件地脱颖而出。
\ $ \ endgroup \ $
– AShelly
15年3月20日在19:38
#3 楼
设计审查连续重新分配
data.recordedSamples
听起来不正确。最终可能会失败,因为您不仅需要大量内存,而且需要大量连续内存。考虑至少重用data.recordedSamples
而不是释放它。这将只需要一个计数器(它当前可以容纳多少个块)并显着减少重新分配的数量。在一个电话中。我认为减少写调用的数量不会提高性能。大部分时间都花在数据传输上,并且与呼叫次数无关。实际上,大写操作使执行配置文件不太流畅:您的程序有时不会花费很多时间,而是会花费很多时间。错过重要音频事件的绝好机会。解决此类问题的标准方法是双重缓冲(我认为写缓冲区要比记录缓冲区快),大致如下:
while (keep_going) {
read_buffer(buf1);
swap(buf1, buf2);
write_nonblock(buf2);
}
代码审查
fwrite
是一个看起来很奇怪的魔术数字。 0.000550
也许?无论如何,该值都需要一些基本原理。您记录了一些导出沉默。我还建议记录一些导入。否则,您可能会在事件开始时就错过一些“交谈”样本。
const float talking_threshold
是加速的绝佳选择。考虑硬件加速。avg
绝对不应该。评论
\ $ \ begingroup \ $
如果没有推荐的清洁剂替代品,我完全不同意goto是“绝对没有必要的”。我可以想到一种更好的方法,但是仅消除goto会导致大量重复或深度嵌套。
\ $ \ endgroup \ $
– nhgrif
15年3月20日在1:24
\ $ \ begingroup \ $
完全同意不需要重新分配,并且要使用触发器或循环缓冲区。 API正在以可管理的块为您提供音频。只需将这些块直接交给文件编写器即可。无需自己将它们粘贴在一起,让文件系统执行此操作。
\ $ \ endgroup \ $
– AShelly
15年3月20日在20:55
\ $ \ begingroup \ $
@ nhgrif,goto在某些深层嵌套中可能很有用,但是在这种情况下,可以将其直接替换为完全相同行为的break。
\ $ \ endgroup \ $
– AShelly
15年3月20日在20:57
\ $ \ begingroup \ $
@AShelly在for循环之前的两个goto语句呢?
\ $ \ endgroup \ $
– nhgrif
2015年3月20日在22:28
\ $ \ begingroup \ $
将第一部分重构到返回错误的StreamSetup函数中,并放入;!err;。作为for循环的测试。
\ $ \ endgroup \ $
– AShelly
15年3月21日在0:44
#4 楼
Goto条件经常被认为是不当行为,这是有原因的。在几乎所有使用goto的情况下,这都是因为您在某种方法上做得太多。考虑一下您的主要方法:int main(void)
{
PaError err = paNoError;
if((err = Pa_Initialize())) goto done;
.....
AudioData data = initAudioData(44100, info->maxInputChannels, paFloat32);
AudioSnippet sampleBlock =
{
.snippet = NULL,
.size = FRAMES_PER_BUFFER * sizeof(float) * data.numberOfChannels
};
PaStream *stream = NULL;
sampleBlock.snippet = malloc(sampleBlock.size);
.....
if((err = Pa_OpenStream(&stream, &inputParameters, NULL, data.sampleRate, FRAMES_PER_BUFFER, paClipOff, NULL, NULL))) goto done;
if((err = Pa_StartStream(stream))) goto done;
for(int i = 0;;)
{
err = Pa_ReadStream(stream, sampleBlock.snippet, FRAMES_PER_BUFFER);
if (err) goto done;
........
}
done:
free(sampleBlock.snippet);
Pa_Terminate();
return err;
}
整个事情可以简单地完成,例如:
int main(void)
{
time_t talking = 0;
time_t silence = 0;
PaStream *stream = NULL;
PaError err = setup_stream(stream);
while (!err) {
err = process_stream(stream);
}
free_stream(stream);
return err;
}
然后,如果有问题,您的各种方法都可以简单地在其中进行操作:设置相同数据的过程是有问题的。完成...
#5 楼
首先,您不会在单行if
语句周围使用括号: /> 为什么要这样做?
if (!outfile) return -1;
那不是
static const int FRAMES_PER_BUFFER = 1024;
吗?问题对此进行了讨论。
正如评论中提到的@RubberDuck,您在
main()
中有很多逻辑。这可能应该委托给一个单独的方法或多个方法。 main()
不应包含一堆逻辑,而应更像是程序的入口。除这些要点外,这是一些最干净,最整洁的代码'已经看到。您在运算符周围使用了空格,并且代码通常看起来很整洁。
评论
\ $ \ begingroup \ $
如果在同一行上的语句很简单,则省略if块周围的花括号并不是很糟糕。如果在if条件周围加上多余的括号,则会禁止编译器发出有关=分配而不是==相等比较的警告。
\ $ \ endgroup \ $
– 200_success
15年3月20日在1:17
评论
我想我将来会装备这个吗?@ rob0t是的,是的。
我不会尝试回答,但是您的Main例程中似乎有很多逻辑。您可能要考虑提取一些子例程。即使唯一的好处就是更容易阅读Main。
@ syb0rg您是否已固定代码?看到它会很有趣。