FFmpegAudioDecoder.h:38:42: error: private field 'mConfig' is not used [-Werror,-Wunused-private-field]

RESOLVED FIXED in mozilla36

Status

()

Core
Audio/Video
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

(Blocks: 1 bug)

Trunk
mozilla36
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
New build warning (treated as an error in --enable-warnings-as-errors builds), with clang 3.5:
{
dom/media/fmp4/ffmpeg/libav54/../FFmpegAudioDecoder.h:38:42: error: private field 'mConfig' is not used [-Werror,-Wunused-private-field]
 1:01.93   const mp4_demuxer::AudioDecoderConfig& mConfig;
 1:01.93                                          ^
 1:01.93 1 error generated.
}


Looks like the last usage of this "mConfig" variable was just removed yesterday, in bug 1096764:
 https://hg.mozilla.org/mozilla-central/rev/1ab82eabedf0#l1.33

I guess we should just get rid of this member-var, since that change left it unused (?)
(Assignee)

Comment 1

3 years ago
In particular, it looks like that changeset made it so we'll now get everything we need out of the "config" object during the constructor.

So it makes sense that we don't need to hold onto it in a member-var.
(Assignee)

Comment 2

3 years ago
Created attachment 8522292 [details] [diff] [review]
fix: drop the variable
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #8522292 - Flags: review?(edwin)
Attachment #8522292 - Flags: review?(edwin) → review+
oh.. I've done it in bug 1098637 while at it ...
(Assignee)

Comment 4

3 years ago
Ah, gotcha.  We could land them separately or just land your existing patch over there, then, I guess.

I lean slightly towards landing this separately[1], because if we end up having to back out bug 1098637 (e.g. because it breaks a test or causes something unexpected), it'd be nice to still have this build error stay fixed.

Would that be OK?  (It means your patch will bitrot slightly, but it looks like the bitrot should be easy because the chunks that won't apply are literally just the chunks that are the same as this patch.)
(Assignee)

Updated

3 years ago
Flags: needinfo?(jyavenard)
(Assignee)

Comment 5

3 years ago
Actually, I just went ahead and pushed this change, since it's better to have it fixed sooner (to fix breakage in clang3.5 warnings-as-errors builds) instead of waiting on a back-and-forth here, and per comment 4, I think it's strictly better to have this de-coupled from an actual functional change.

Push: https://hg.mozilla.org/integration/mozilla-inbound/rev/76fc2a130baa

Sorry for causing (minor) bitrot in bug 1098637. :)
Blocks: 187528
Flags: needinfo?(jyavenard) → in-testsuite-
Duplicate of this bug: 1099225
https://hg.mozilla.org/mozilla-central/rev/76fc2a130baa
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.