Rename misleading |is_ambiguous| parameter

media/base/mime_util_internal.h

Index: media/base/mime_util_internal.h
diff --git a/media/base/mime_util_internal.h b/media/base/mime_util_internal.h
index 67396cf654e11271680c048d17a76daeafc1c2ec..4096a0da2d098306b5b4f3fb9769ebe5be305fa8 100644
--- a/media/base/mime_util_internal.h
+++ b/media/base/mime_util_internal.h
@@ -98,29 +98,28 @@ class MEDIA_EXPORT MimeUtil {
                               const CodecSet& codecs_list,
                               bool is_proprietary_mime_type);
 
-  // Returns IsSupported if all codec IDs in |codecs| are unambiguous and are
-  // supported in |mime_type_lower_case|. MayBeSupported is returned if at least
-  // one codec ID in |codecs| is ambiguous but all the codecs are supported.
+  // Returns IsSupported if all codec IDs in |codecs| are unambiguous, known to

[ddorwin, 2016/04/18 23:43:37]

"known to be supported" here, which is not "confidently known to be supported"
below, is the most problematic naming issue.

[DaleCurtis, 2016/04/20 19:18:54]

Yeah, this is just redundant. It doesn't make sense to have both "known to be
supported" and "are supported". Replace one or the other with your preferred
wording.

[ddorwin, 2016/04/21 00:50:30]

It turns out this was a bit of a bad example. I've fixed the comments. We now
have:
1. "confidently known to be supported" - a codec string we fully recognize and
"know" we support
2. "supported in |mime_type_lower_case|" - a valid codec for the container.

We still have the larger question of how to describe #1.

+  // be supported, and are supported in |mime_type_lower_case|. MayBeSupported

[xhwang, 2016/04/19 23:59:42]

nit and not for this CL: Since "maybe" is one word, this should be
MaybeSupported?

[ddorwin, 2016/04/21 00:50:30]

I believe this is based on "the codec may be supported" just like "the codec is
[not] supported."

The single word would be something like "Maybe the codec is supported."

[xhwang, 2016/04/21 05:59:44]

Acknowledged.

+  // is returned if all the codecs are supported but at least one codec ID in
+  // |codecs| is ambiguous or not confidently known to be supported.

[DaleCurtis, 2016/04/20 19:18:54]

maybe something like "support can not be determined at this time" ?

[ddorwin, 2016/04/21 00:50:30]

Reworded. WDYT?

   // IsNotSupported is returned if |mime_type_lower_case| is not supported or at
-  // least one is not supported in |mime_type_lower_case|. |is_encrypted| means
-  // the codec will be used with encrypted blocks.
+  // least one of the |codecs| is not supported in |mime_type_lower_case|.
+  // |is_encrypted| means the codec will be used with encrypted blocks.
   SupportsType AreSupportedCodecs(const CodecSet& supported_codecs,

[xhwang, 2016/04/19 23:59:42]

What is this parameter for? It's not documented and is not obvious.

[ddorwin, 2016/04/21 00:50:30]

Done.

                                   const std::vector<std::string>& codecs,
                                   const std::string& mime_type_lower_case,
                                   bool is_encrypted) const;
 
   // Converts a codec ID into an Codec enum value and indicates

[xhwang, 2016/04/19 23:59:42]

nit: s/an/a

also needs reflow

[ddorwin, 2016/04/21 00:50:30]

Done.
Done.

-  // whether the conversion was ambiguous.
+  // whether the specified codec, including profile, etc., is is known supported

[xhwang, 2016/04/19 23:59:42]

s/is is/is/

[ddorwin, 2016/04/21 00:50:30]

Done.

+  // ("probably") or not ("maybe"), such as a potentially unsupported profile.
   // Returns true if this method was able to map |codec_id| to a specific
-  // Codec enum value. |codec| and |is_ambiguous| are only valid if true
+  // Codec enum value. |codec| and |is_known_supported| are only valid if true
   // is returned. Otherwise their value is undefined after the call.
-  // |is_ambiguous| is true if |codec_id| did not have enough information to
-  // unambiguously determine the proper Codec enum value. If |is_ambiguous|
-  // is true |codec| contains the best guess for the intended Codec enum value.
   // |is_encrypted| means the codec will be used with encrypted blocks.
   bool StringToCodec(const std::string& codec_id,
                      Codec* codec,
-                     bool* is_ambiguous,
+                     bool* is_known_supported,

[xhwang, 2016/04/19 23:59:42]

Thanks for the background context on naming. So is |is_known_supported| strictly
the opposite of |is_ambiguous|? If so, can this just be |is_supported|, which is
"unambiguously" supported? It may seems odd in MimeUtil::AreSupportedCodecs()
when we check Is*Supported twice. But I don't really feel |is_known_supported|
is clearer.

Or, can we have a comment here about what |is_known_supported| exactly means?

[ddorwin, 2016/04/21 00:50:30]

No, it is not strictly the opposite. Ambiguous (now) means the string does not
give us enough information to determine whether we support it. As I said above,
this is now just the three strings in kAmbiguousCodecStringMap.

Ambiguous is a subset of things that are not |is_known_supported|. There are
others, including values we don't support (i.e. AVC extended profile). Most
relevant to this discussion, there are things like Hi10p, which may be supported
but we don't actually know (except for FFmpeg in this specific case).

I agree that we should have a clearer name. Maybe |is_probably_supported|, which
is consistent with "probably". It's also similar to "likely", which was one of
my suggestions.

I've updated the comment. I'll hold off on renaming until we pick a name.

[xhwang, 2016/04/21 05:59:44]

|is_probably_supported| is probably better, but not known better :) Joke aside,
I do feel |is_known_supported| is a bit too strong for what you described.
|is_probably_supported| does sound better to me.

[ddorwin, 2016/04/21 17:30:48]

Done.

                      bool is_encrypted) const;
 
   // Returns true if |codec| is supported when contained in