Last Comment Bug 800538 - Add MOZ_ASSERT for mVoiceEngine in MediaEngineWebRTCAudioSource/etc constructors
: Add MOZ_ASSERT for mVoiceEngine in MediaEngineWebRTCAudioSource/etc constructors
[WebRTC], [blocking-webrtc-] [qa-]
Product: Core
Classification: Components
Component: WebRTC (show other bugs)
: Trunk
: All All
: P3 normal (vote)
: mozilla20
Assigned To: Jan-Ivar Bruaroey [:jib]
: Jason Smith [:jsmith]
Depends on:
Blocks: 800579
  Show dependency treegraph
Reported: 2012-10-11 13:54 PDT by Randell Jesup [:jesup]
Modified: 2012-12-09 00:49 PST (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Adds two MOZ_ASSERTs (2.21 KB, patch)
2012-11-28 18:49 PST, Jan-Ivar Bruaroey [:jib]
rjesup: review+
Details | Diff | Splinter Review
Adds two MOZ_ASSERTs (1.97 KB, patch)
2012-11-28 19:37 PST, Jan-Ivar Bruaroey [:jib]
jib: review+
rjesup: checkin+
Details | Diff | Splinter Review

Description Randell Jesup [:jesup] 2012-10-11 13:54:46 PDT
Belt-and-suspenders safety
Comment 1 Jan-Ivar Bruaroey [:jib] 2012-11-28 18:49:36 PST
Created attachment 686386 [details] [diff] [review]
Adds two MOZ_ASSERTs

Note that I took the "etc" in the comment as license to add an equivalent MOZ_ASSERT to the mVideoEngine in MediaEngineWebRTCVideoSource as well.
Comment 2 Randell Jesup [:jesup] 2012-11-28 19:09:47 PST
Comment on attachment 686386 [details] [diff] [review]
Adds two MOZ_ASSERTs

Review of attachment 686386 [details] [diff] [review]:

::: content/media/webrtc/MediaEngineWebRTC.h
@@ +62,5 @@
>    // ViEExternalRenderer.
>    virtual int FrameSizeChange(unsigned int, unsigned int, unsigned int);
>    virtual int DeliverFrame(unsigned char*, int, uint32_t, int64_t);
> +  MediaEngineWebRTCVideoSource(webrtc::VideoEngine* videoEngine,

Mozilla prefers aFoo for parameters, mFoo for member vars, foo for locals.  Put this back to aVideoEnginePtr (or aVideoEngine might be better), and change the AudioSource from voiceEngine to aVoiceEngine
Comment 3 Jan-Ivar Bruaroey [:jib] 2012-11-28 19:12:07 PST
I tried to make it symmetrical with the Audio code. Should I change the Audio code to the preferred syntax or leave both alone?
Comment 4 Jan-Ivar Bruaroey [:jib] 2012-11-28 19:37:40 PST
Created attachment 686409 [details] [diff] [review]
Adds two MOZ_ASSERTs

Addressing comments from review, carrying forward r=jesup.
Comment 5 Randell Jesup [:jesup] 2012-11-28 19:54:47 PST
Comment 7 Ryan VanderMeulen [:RyanVM] 2012-11-29 16:35:21 PST

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