817488 - Verify Audio Transmit and Receive - PC Unit Tests

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect, P2)

Comment 1

[Suhas]

Attachment: Audio Send and Recv support for PC tests

Comment 2

[Suhas]

Attachment: Audio Send and Recv support for PC tests

Comment 3

[Suhas]

Attachment: Audio Send and Recv support for PC tests

Comment 4

[Suhas]

Attachment: Audio Send and Recv support for PC tests

Comment 5

[Eric Rescorla (:ekr)]

Comment on attachment 688708 [details] [diff] [review]
Audio Send and Recv support for PC tests

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

These look good to me with the following comments.

::: media/webrtc/signaling/test/FakeMediaStreams.h
@@ +125,5 @@
> +        numReadSamples++;
> +        if(!memcmp(buf,zeroBuffer,chunk.mDuration)) {
> +          //sample received was  filled with zeroes.
> +          numZeroSamples++;
> +        }

OK, now that I have seen this code, I gave you terrible advice here about the memcmp.

Let's just iterate over the buffer to see if the values are nonzero.But don't bother rescaling.

@@ +129,5 @@
> +        }
> +        //process next chunk
> +        iter.Next();
> +      }
> +      if(numZeroSamples != numReadSamples) {

Wouldn't this code be easier if you just did bool nonzero=false; at the beginning and then set it to true if you ever saw a nonzero sample? Isn't that the same logic?

::: media/webrtc/signaling/test/FakeMediaStreamsImpl.h
@@ +43,5 @@
>  
>  void Fake_SourceMediaStream::Periodic() {
> +  //Pull more audio-samples iff pulling is enabled
> +  // and we are not asked by the signaling agent to stop
> +  //pulling data.

space after //

@@ +74,5 @@
>  // Fake_AudioStreamSource
>  void Fake_AudioStreamSource::Periodic() {
> +  //Are we asked to stop pumping audio samples ?
> +  if(mStop) {
> +    std::cerr <<"AudioStream Stopped !!!" << std::endl;

Don't think we need this.

@@ +80,5 @@
> +  }
> +
> +  //Generate Signed 16 Bit Audio samples
> +  nsRefPtr<mozilla::SharedBuffer> samples =
> +          mozilla::SharedBuffer::Create(1600 * 2 * sizeof(int16_t));

Can we break these integers out into constants.

Comment 6

[Suhas]

Attachment: Audio Send and Recv support for PC tests

Comment 7

[Suhas]

Attachment: Audio Send and Recv support for PC tests

Comment 8

[Suhas]

Attachment: Audio Send and Recv support for PC tests

Comment 9

[Suhas]

Attachment: Audio Send and Recv support for PC tests

Comment 10

[Ethan Hugg [:ehugg]]

Comment on attachment 688943 [details] [diff] [review]
Audio Send and Recv support for PC tests


https://hg.mozilla.org/integration/mozilla-inbound/rev/77f529436f2b

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/77f529436f2b