Make explicit the PipelineStatus enum values and warn that they are logged to UMA.

media/base/pipeline_status.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef MEDIA_BASE_PIPELINE_STATUS_H_
 #define MEDIA_BASE_PIPELINE_STATUS_H_
 
 #include "base/callback.h"
 
 #include <string>
 
 namespace media {
 
 // Status states for pipeline.  All codes except PIPELINE_OK indicate errors.
+// Logged to UMA, so never reuse a value, always add new/greater ones!

[scherkus (not reviewing), 2012/08/13 21:08:59]

is this only logged for GpuVideoDecoder::Initialize()?

how many values are actually used?

[Ami GONE FROM CHROMIUM, 2012/08/13 21:23:54]

Today, yes.
Today GVD can return OK (yay!), DECODER_ERROR_NOT_SUPPORTED (HW doesn't support
specific profile/codec), and PIPELINE_ERROR_DECODE (oh crap, something went
wrong; invalid demuxer config, missing stream, etc.).

[scherkus (not reviewing), 2012/08/14 21:40:30]

Should VideoDecoders have their own status codes / should they be expanded to
include the problems you listed here?

[Ami GONE FROM CHROMIUM, 2012/08/14 21:43:28]

My rule of thumb is that if you're not going to handle a status code
specifically, it shouldn't exist.
So, no, I don't think we should make these codes more granular than they already
are.

[scherkus (not reviewing), 2012/08/15 01:03:44]

Think about it from WMPI's perspective and http://crbug.com/126070

Today Pipeline converts all VideoRenderer initialization error codes into
PIPELINE_ERROR_COULD_NOT_RENDER, so DECODER_ERROR_NOT_SUPPORTED seems ripe for
removal / moved into a VideoDecoder-specific set of enums that VideoRendererBase
knows to handle, reducing the # of errors that Pipeline/WMPI need to concern
themselves with

My general comment is that we seem to use PipelineStatus[CB] more out of
convenience than anything else -- should we change that?

 // TODO(vrk/scherkus): Trim the unused status codes. (crbug.com/126070)
 enum PipelineStatus {
-  PIPELINE_OK,
-  PIPELINE_ERROR_URL_NOT_FOUND,
-  PIPELINE_ERROR_NETWORK,
-  PIPELINE_ERROR_DECODE,
-  PIPELINE_ERROR_DECRYPT,
-  PIPELINE_ERROR_ABORT,
-  PIPELINE_ERROR_INITIALIZATION_FAILED,
-  PIPELINE_ERROR_REQUIRED_FILTER_MISSING,
-  PIPELINE_ERROR_COULD_NOT_RENDER,
-  PIPELINE_ERROR_READ,
-  PIPELINE_ERROR_OPERATION_PENDING,
-  PIPELINE_ERROR_INVALID_STATE,
+  PIPELINE_OK = 0,
+  PIPELINE_ERROR_URL_NOT_FOUND = 1,
+  PIPELINE_ERROR_NETWORK = 2,
+  PIPELINE_ERROR_DECODE = 3,
+  PIPELINE_ERROR_DECRYPT = 4,
+  PIPELINE_ERROR_ABORT = 5,
+  PIPELINE_ERROR_INITIALIZATION_FAILED = 6,
+  PIPELINE_ERROR_REQUIRED_FILTER_MISSING = 7,
+  PIPELINE_ERROR_COULD_NOT_RENDER = 8,
+  PIPELINE_ERROR_READ = 9,
+  PIPELINE_ERROR_OPERATION_PENDING = 10,
+  PIPELINE_ERROR_INVALID_STATE = 11,
   // Demuxer related errors.
-  DEMUXER_ERROR_COULD_NOT_OPEN,
-  DEMUXER_ERROR_COULD_NOT_PARSE,
-  DEMUXER_ERROR_NO_SUPPORTED_STREAMS,
+  DEMUXER_ERROR_COULD_NOT_OPEN = 12,
+  DEMUXER_ERROR_COULD_NOT_PARSE = 13,
+  DEMUXER_ERROR_NO_SUPPORTED_STREAMS = 14,
   // Decoder related errors.
-  DECODER_ERROR_NOT_SUPPORTED,
+  DECODER_ERROR_NOT_SUPPORTED = 15,
   PIPELINE_STATUS_MAX,  // Must be greater than all other values logged.
 };
 
 typedef base::Callback<void(PipelineStatus)> PipelineStatusCB;
 
 // Wrap & return a callback around |cb| which reports its argument to UMA under
 // the requested |name|.
 PipelineStatusCB CreateUMAReportingPipelineCB(const std::string& name,
                                               const PipelineStatusCB& cb);
 
@@ skipping 11 matching lines @@
   uint32 video_frames_decoded;
   uint32 video_frames_dropped;
 };
 
 // Used for updating pipeline statistics.
 typedef base::Callback<void(const PipelineStatistics&)> StatisticsCB;
 
 }  // namespace media
 
 #endif  // MEDIA_BASE_PIPELINE_STATUS_H_