Last Comment Bug 800538 - Add MOZ_ASSERT for mVoiceEngine in MediaEngineWebRTCAudioSource/etc constructors
: Add MOZ_ASSERT for mVoiceEngine in MediaEngineWebRTCAudioSource/etc constructors
Status: RESOLVED FIXED
[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]
: Maire Reavy [:mreavy] Please needinfo me
Mentors:
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:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
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 User image Randell Jesup [:jesup] 2012-10-11 13:54:46 PDT
Belt-and-suspenders safety
Comment 1 User image 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 User image 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 User image 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 User image 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 User image Randell Jesup [:jesup] 2012-11-28 19:54:47 PST
 https://tbpl.mozilla.org/?tree=Try&rev=9ee1b88c7b7f
Comment 7 User image Ryan VanderMeulen [:RyanVM] 2012-11-29 16:35:21 PST
https://hg.mozilla.org/mozilla-central/rev/3d26dff1f3e9

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