899240 - neteq_defines.h:350:5 [-Wcomment] "/*" within comment

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Daniel Holbert [:dholbert]]

Build warning:
{
media/webrtc/trunk/webrtc/modules/audio_coding/neteq/neteq_defines.h:350:5 [-Wcomment] "/*" within comment
}

The code in question is here:
> 348     /*#define NETEQ_OPUS_CODEC/
> 349 
> 350     /* hack in 48 kHz support */
> 351     #define NETEQ_48KHZ_WIDEBAND
http://mxr.mozilla.org/mozilla-central/source/media/webrtc/trunk/webrtc/modules/audio_coding/neteq/neteq_defines.h?rev=edd376cd77bf&mark=348-350#348

It looks pretty clearly that the trailing "/" is supposed to be a "*/" on line 348 -- but since it's just a "/", everything until the next */ ends up being commented out.

Luckily, this just affects a blank line and a comment (no code), for now. Still, worth fixing.

This line was added here:
http://hg.mozilla.org/mozilla-central/diff/edd376cd77bf/media/webrtc/trunk/webrtc/modules/audio_coding/neteq/neteq_defines.h#l1.53

Randell, can we fix this locally, or do we need it to be fixed upstream?

Comment 1

[Daniel Holbert [:dholbert]]

I'm guessing this was actually a merge error, since upstream WebRTC 3.30 (what we were importing in the changeset that broke this) doesn't have this line at all, in the "* Super wideband 32kHz codecs */" section:
http://code.google.com/p/webrtc/source/browse/branches/3.30/webrtc/modules/audio_coding/neteq/neteq_defines.h#338

So a local fix is probably appropriate.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: fix

Comment 3

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #1)
> I'm guessing this was actually a merge error, since upstream WebRTC 3.30
> (what we were importing in the changeset that broke this) doesn't have this
> line at all, in the "* Super wideband 32kHz codecs */" section:
> http://code.google.com/p/webrtc/source/browse/branches/3.30/webrtc/modules/
> audio_coding/neteq/neteq_defines.h#338

FWIW, this chunk of neteq_defines.h is identical (& lacks the typo) in the upstream webrtc "trunk" and "stable" branches, too:

http://code.google.com/p/webrtc/source/browse/trunk/webrtc/modules/audio_coding/neteq/neteq_defines.h#338
http://code.google.com/p/webrtc/source/browse/stable/webrtc/modules/audio_coding/neteq/neteq_defines.h#338
(the whole file might be identical, actually)

Comment 4

[Daniel Holbert [:dholbert]]

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

Comment 5

[Ryan VanderMeulen [:RyanVM]]

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