942399 - Fix -Wunused-private-field warnings in media/webrtc/signaling

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: fix-webrtc-warnings.patch

1. Fix the following clang warnings:

media/webrtc/signaling/src/media-conduit/AudioConduit.h:232:12 [-Wunused-private-field] private field 'mLastTimestamp' is not used

media/webrtc/signaling/src/mediapipeline/MediaPipeline.h:399:13 [-Wunused-private-field] private field 'last_img_' is not used

2. Also change WebrtcAudioConduit::mLastTimestamp's type from uint32_t to unsigned int so some redundant static_casts can be removed.

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8337184 [details] [diff] [review]
fix-webrtc-warnings.patch

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

Well, the timestamp really *is* a uint32_t (comes from RTP headers); the API that exposes it as a unsigned int is slightly wrong (but it's an import, so may not get changed).  And internally the value returned comes from playout_timestamp_rtp_, which is a uint32_t.  So I'd prefer to keep that type and the cast, to avoid people thinking rollover is impossible (it can and does happen, since the initial value is a random 32-bit value)

I hate the MOZILLA_INTERNAL_API ifdef infection getting any worse (and the warnings are only for the c++ unit tests because of the stupid linkage problems there) - but I understand it's a pain to have the warnings.  Sigh.

Comment 2

[Chris Peterson [:cpeterson]]

Attachment: fix-webrtc-warnings-v2.patch

Randell: if I change the "unused" member variables from private to protected (or public) visibility, then clang no longer warns that they are unused. I can then omit the ugly #ifdef MOZILLA_INTERNAL_API. Is this preferable?

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8337537 [details] [diff] [review]
fix-webrtc-warnings-v2.patch

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

No, let's not hide the problem - it would just bite us later.  Thanks.

Comment 4

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e8acaec23a55

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e8acaec23a55