Closed Bug 843929 Opened 7 years ago Closed 7 years ago

MediaEngineWebRTC.h:147:11: warning: private field 'mTrackID' is not used [-Wunused-private-field] (and more)

Categories

(Core :: WebRTC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: dholbert, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

(Whiteboard: [qa-])

Attachments

(3 files, 1 obsolete file)

Noticed some WebRTC unused-private-variable warnings, during a clang build today:
{
28:53.70 In file included from/mozilla-central/content/media/webrtc/MediaEngineWebRTCVideo.cpp:5:
28:53.70 Warning: -Wunused-private-field in/mozilla-central/content/media/webrtc/MediaEngineWebRTC.h: private field 'mTrackID' is not used
28:53.70/mozilla-central/content/media/webrtc/MediaEngineWebRTC.h:147:11: warning: private field 'mTrackID' is not used [-Wunused-private-field]
28:53.70   TrackID mTrackID;
28:53.70           ^


28:53.70 Warning: -Wunused-private-field in/mozilla-central/content/media/webrtc/MediaEngineWebRTC.h: private field 'mLastEndTime' is not used
28:53.70/mozilla-central/content/media/webrtc/MediaEngineWebRTC.h:148:14: warning: private field 'mLastEndTime' is not used [-Wunused-private-field]
28:53.70   TrackTicks mLastEndTime;
28:53.70              ^


28:53.70 Warning: -Wunused-private-field in/mozilla-central/content/media/webrtc/MediaEngineWebRTC.h: private field 'mFps' is not used
28:53.70/mozilla-central/content/media/webrtc/MediaEngineWebRTC.h:157:7: warning: private field 'mFps' is not used [-Wunused-private-field]
28:53.70   int mFps; // Track rate (30 fps by default)
28:53.70       ^


28:56.05 In file included from/mozilla-central/content/media/webrtc/MediaEngineWebRTCAudio.cpp:5:
28:56.05 Warning: -Wunused-private-field in/mozilla-central/content/media/webrtc/MediaEngineWebRTC.h: private field 'mEchoCancel' is not used
28:56.05/mozilla-central/content/media/webrtc/MediaEngineWebRTC.h:254:20: warning: private field 'mEchoCancel' is not used [-Wunused-private-field]
28:56.05   webrtc::EcModes  mEchoCancel;
28:56.05                    ^
}
Looks like mTrackID is intentionally unused (it's not supposed to be used) -- its last usages were removed (replaced w/ a function-param) e.g. here:
  https://hg.mozilla.org/mozilla-central/rev/8c3c368478a0#l5.121

Not sure yet if the other ones can be removed, too, or if they need to be there & it's a bug that we're not using them...
More hg-archeology turns up the following:
* The last usages of MediaEngineWebRTCVideoSource::mLastEndTime were removed (and replaced with a function-parameter "aLastEndTime") here:
  https://hg.mozilla.org/mozilla-central/rev/8c3c368478a0#l6.11
  (that's the same cset as for mTrackID in previous comment, btw)

* The last usage of MediaEngineWebRTCVideoSource::mFPS was removed (and replaced with USECS_PER_S...?)  here:
  https://hg.mozilla.org/mozilla-central/rev/b5964726ad30#l6.108

* The variable MediaEngineWebRTCAudioSource::mEchoCancel was unused when it was added, though it's got one usage in an "#if 0" block (which still exists in current m-c, but is of course invisible to compilers):
 https://hg.mozilla.org/mozilla-central/rev/83305a2fa224#l4.47

==========

CONCLUSIONS:
 * it looks like mTrackID and mLastEndTime are both definitely-safe-to-remove, since their usages were all replaced with function variables as part of 8c3c368478a0 (Bug 811757)

 * I suspect mFPS is probably also safe to remove, though it's a bit odd that we replaced its usage with USECS_PER_S (implying that we're shooting for 1 million FPS?).

 * And for mEchoCancel, it looks like we're *intending* to use it once we can enable that "#if 0" chunk. In the meantime, it probably needs 'mozilla::unused <<' action, to silence the compiler warning about it being unused.
Depends on: 801843, 811757, 818670
Attachment #716961 - Flags: review?(rjesup)
This adds mEchoCancel to a line of existing (void) casts right after the #if 0 block that _actually_ uses it.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #716963 - Flags: review?(rjesup)
Attachment #716961 - Flags: review?(rjesup) → review+
Comment on attachment 716962 [details] [diff] [review]
suppress warnings about unused paramers in MediaEngine

Not actually needed for func params
Attachment #716962 - Attachment is obsolete: true
Attachment #716963 - Flags: review?(rjesup) → review+
jesup says that mFps will potentially be used in the future, so it makes sense to cast it as (void) right now with a comment to remove that once it's actually used.
Attachment #716968 - Flags: review?(rjesup)
Attachment #716968 - Flags: review?(rjesup) → review+
Keywords: checkin-needed
Whiteboard: [checkin needed for all 3 patches]
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.