Last Comment Bug 783101 - Record mediaconduit test data in WAV instead of raw pcm
: Record mediaconduit test data in WAV instead of raw pcm
Status: RESOLVED FIXED
[qa-]
:
Product: Core
Classification: Components
Component: WebRTC: Audio/Video (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: ---
Assigned To: Ralph Giles (:rillian) needinfo me
: Jason Smith [:jsmith]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-15 14:41 PDT by Ralph Giles (:rillian) needinfo me
Modified: 2012-08-19 22:25 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
proposed fix for recorded.wav (7.53 KB, patch)
2012-08-15 14:48 PDT, Ralph Giles (:rillian) needinfo me
snandaku: feedback+
Details | Diff | Review
fix for recorded.wav (7.73 KB, patch)
2012-08-16 14:01 PDT, Ralph Giles (:rillian) needinfo me
giles: review+
snandaku: feedback+
Details | Diff | Review

Description Ralph Giles (:rillian) needinfo me 2012-08-15 14:41:50 PDT
Write a quick-and-dirty wav header on our recorded data so it's easier to verify.

I just spent several minutes trying to remember how to play raw pcm files with sox, and then guessing at the correct parameters, so I could verify recorded.pcm from mediaconduit_unittests. That's ridiculous when WAV is a straightforward format to write.

Fortunately, VoEFile handles WAV just fine, so no modification of webrtc_standalone_test is necessary beyond the filename change.

WAVE header code is from Robert O'Callahan's patch in bug 727697.
Comment 1 Ralph Giles (:rillian) needinfo me 2012-08-15 14:48:37 PDT
Created attachment 652242 [details] [diff] [review]
proposed fix for recorded.wav
Comment 2 Suhas 2012-08-15 23:35:30 PDT
Comment on attachment 652242 [details] [diff] [review]
proposed fix for recorded.wav

Review of attachment 652242 [details] [diff] [review]:
-----------------------------------------------------------------

We can get rid of errno for now and commit the code. feedback+ from my side.

::: media/webrtc/signaling/test/mediaconduit_unittests.cpp
@@ +195,5 @@
> +  // Update the header
> +  unsigned char size[4];
> +  int err = fseek(outFile, 40, SEEK_SET);
> +  if (err < 0) {
> +    cerr << "Couldn't seek to WAV header: " << strerror(errno) << endl;

We should be good without the errno.

@@ +208,5 @@
> +
> +  // Return to the end
> +  err = fseek(outFile, 0, SEEK_END);
> +  if (err < 0) {
> +    cerr << "Couldn't seek to WAV file end: " << strerror(errno) << endl;

We should be good without the errno .
Comment 3 Ralph Giles (:rillian) needinfo me 2012-08-16 14:01:49 PDT
Created attachment 652557 [details] [diff] [review]
fix for recorded.wav
Comment 4 Ralph Giles (:rillian) needinfo me 2012-08-16 14:02:34 PDT
Comment on attachment 652557 [details] [diff] [review]
fix for recorded.wav

Carrying forward earlier positive review. Suhas, you can just change the ? to + if you approve of a patch.
Comment 5 Ralph Giles (:rillian) needinfo me 2012-08-16 14:20:03 PDT
http://hg.mozilla.org/projects/alder/rev/644903e22112

Note You need to log in before you can comment on or make changes to this bug.