Add MOZ_ASSERT for mVoiceEngine in MediaEngineWebRTCAudioSource/etc constructors

RESOLVED FIXED in mozilla20

Status

()

Core
WebRTC
P3
normal
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: jesup, Assigned: jib)

Tracking

(Blocks: 1 bug)

Trunk
mozilla20
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [WebRTC], [blocking-webrtc-] [qa-])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Belt-and-suspenders safety
(Reporter)

Updated

5 years ago
Blocks: 800579

Updated

5 years ago
Priority: -- → P3
Whiteboard: [WebRTC], [blocking-webrtc-]
(Assignee)

Updated

5 years ago
Assignee: nobody → jib
(Assignee)

Comment 1

5 years ago
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.
Attachment #686386 - Flags: review?(rjesup)
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED
(Reporter)

Comment 2

5 years ago
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
Attachment #686386 - Flags: review?(rjesup) → review+
(Assignee)

Comment 3

5 years ago
I tried to make it symmetrical with the Audio code. Should I change the Audio code to the preferred syntax or leave both alone?
(Assignee)

Comment 4

5 years ago
Created attachment 686409 [details] [diff] [review]
Adds two MOZ_ASSERTs

Addressing comments from review, carrying forward r=jesup.
Attachment #686386 - Attachment is obsolete: true
Attachment #686409 - Flags: review+
(Assignee)

Updated

5 years ago
Attachment #686409 - Flags: checkin?(rjesup)
(Reporter)

Comment 5

5 years ago
 https://tbpl.mozilla.org/?tree=Try&rev=9ee1b88c7b7f
(Reporter)

Comment 6

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/3d26dff1f3e9
Target Milestone: --- → mozilla20
https://hg.mozilla.org/mozilla-central/rev/3d26dff1f3e9
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Whiteboard: [WebRTC], [blocking-webrtc-] → [WebRTC], [blocking-webrtc-] [qa-]
(Reporter)

Updated

4 years ago
Attachment #686409 - Flags: checkin?(rjesup) → checkin+
You need to log in before you can comment on or make changes to this bug.