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

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Daniel Holbert [:dholbert]]

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 (?)

Comment 1

[Daniel Holbert [:dholbert]]

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.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: fix: drop the variable

Comment 3

[Jean-Yves Avenard [:jya]]

oh.. I've done it in bug 1098637 while at it ...

Comment 4

[Daniel Holbert [:dholbert]]

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.)

Comment 5

[Daniel Holbert [:dholbert]]

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. :)

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/76fc2a130baa