Remove Decoder::GetDisplayName() in favor of GetDisplayType()

This replaces all the calls to GetDisplayName() that were mostly being
used in LOG/DLOG with an ostream operator that prints the name version
of GetDisplayType().

In a followup CL I'd like to delete FakeVideoDecoder in favor of
MockVideoDecoder, as well as merge the two implementations of MVD.

Bug: 1176374

Change-Id: Ie87c5d0108be5ff9aa050372088da2e285bb0dec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2617859
Commit-Queue: Ted Meyer <tmathmeyer@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#859250}

chrome/browser/media/history/media_history_browsertest.cc

@@ -775,7 +775,7 @@ IN_PROC_BROWSER_TEST_P(MediaHistoryBrowserTest, DISABLED_GetPlaybackSessions) {
 
 // TODO(crbug.com/1177109) Re-enable test
 IN_PROC_BROWSER_TEST_P(MediaHistoryBrowserTest,
-                       DISABLED_SaveImagesWithDifferentSessions) {
+                       SaveImagesWithDifferentSessions) {
   auto* browser = CreateBrowserFromParam();
   auto expected_metadata = GetExpectedMetadata();
   auto expected_artwork = GetExpectedArtwork();

media/base/async_destroy_video_decoder.h

@@ -40,11 +40,6 @@ class AsyncDestroyVideoDecoder final : public VideoDecoder {
     return wrapped_decoder_->GetDecoderType();
   }
 
-  std::string GetDisplayName() const override {
-    DCHECK(wrapped_decoder_);
-    return wrapped_decoder_->GetDisplayName();
-  }
-
   bool IsPlatformDecoder() const override {
     DCHECK(wrapped_decoder_);
     return wrapped_decoder_->IsPlatformDecoder();

media/base/decoder.cc

@@ -50,6 +50,11 @@ std::string GetDecoderName(VideoDecoderType type) {
       return "VideoDecoderPipeline (ChromeOs)";
     case VideoDecoderType::kVda:
       return "VideoDecodeAccelerator";
+    case VideoDecoderType::kTesting:
+      return "Testing or Mock Video decoder";
+    default:
+      NOTREACHED();
+      return "VideoDecoderType created through invalid static_cast";
   }
 }
 
@@ -67,7 +72,20 @@ std::string GetDecoderName(AudioDecoderType type) {
       return "MediaCodecAudioDecoder";
     case AudioDecoderType::kBroker:
       return "AudioDecoderBroker";
+    case AudioDecoderType::kTesting:
+      return "Testing or Mock Audio decoder";
+    default:
+      NOTREACHED();
+      return "VideoDecoderType created through invalid static_cast";
   }
 }
 
+std::ostream& operator<<(std::ostream& out, AudioDecoderType type) {
+  return out << GetDecoderName(type);
+}
+
+std::ostream& operator<<(std::ostream& out, VideoDecoderType type) {
+  return out << GetDecoderName(type);
+}
+
 }  // namespace media

media/base/decoder.h

@@ -23,8 +23,10 @@ enum class AudioDecoderType : int {
   kDecrypting = 3,  // DecryptingAudioDecoder
   kMediaCodec = 4,  // MediaCodecAudioDecoder (Android)
   kBroker = 5,      // AudioDecoderBroker
+  kTesting = 6,     // Never send this to UKM, for tests only.
 
-  kMaxValue = kBroker  // Keep this at the end and equal to the last entry.
+  // Keep this at the end and equal to the last entry.
+  kMaxValue = kTesting,
 };
 
 // List of known VideoDecoder implementations; recorded to UKM, always add new
@@ -49,11 +51,17 @@ enum class VideoDecoderType : int {
   // Chromeos uses VideoDecoderPipeline. This could potentially become more
   // granulated in the future.
   kChromeOs = 15,
-  kMaxValue = kChromeOs  // Keep this at the end and equal to the last entry.
+
+  kTesting = 16,  // Never send this to UKM, for tests only.
+
+  // Keep this at the end and equal to the last entry.
+  kMaxValue = kTesting
 };
 
 MEDIA_EXPORT std::string GetDecoderName(AudioDecoderType type);
 MEDIA_EXPORT std::string GetDecoderName(VideoDecoderType type);
+MEDIA_EXPORT std::ostream& operator<<(std::ostream& out, AudioDecoderType type);
+MEDIA_EXPORT std::ostream& operator<<(std::ostream& out, VideoDecoderType type);
 
 class MEDIA_EXPORT Decoder {
  public:
@@ -72,12 +80,6 @@ class MEDIA_EXPORT Decoder {
   // |DecoderSelector| potentially slowing down the selection process.
   virtual bool SupportsDecryption() const;
 
-  // Returns the name of the decoder for logging and decoder selection purposes.
-  // This name should be available immediately after construction, and should
-  // also be stable in the sense that the name does not change across multiple
-  // constructions.
-  virtual std::string GetDisplayName() const = 0;
-
  protected:
   Decoder();
 };

media/base/mock_filters.cc

@@ -72,17 +72,17 @@ void MockDemuxerStream::set_liveness(DemuxerStream::Liveness liveness) {
   liveness_ = liveness;
 }
 
-MockVideoDecoder::MockVideoDecoder() : MockVideoDecoder("MockVideoDecoder") {}
+MockVideoDecoder::MockVideoDecoder() : MockVideoDecoder(0) {}
 
-MockVideoDecoder::MockVideoDecoder(std::string decoder_name)
-    : MockVideoDecoder(false, false, std::move(decoder_name)) {}
+MockVideoDecoder::MockVideoDecoder(int decoder_id)
+    : MockVideoDecoder(false, false, decoder_id) {}
 
 MockVideoDecoder::MockVideoDecoder(bool is_platform_decoder,
                                    bool supports_decryption,
-                                   std::string decoder_name)
+                                   int decoder_id)
     : is_platform_decoder_(is_platform_decoder),
       supports_decryption_(supports_decryption),
-      decoder_name_(std::move(decoder_name)) {
+      decoder_id_(decoder_id) {
   ON_CALL(*this, CanReadWithoutStalling()).WillByDefault(Return(true));
   ON_CALL(*this, IsOptimizedForRTC()).WillByDefault(Return(false));
 }
@@ -97,14 +97,6 @@ bool MockVideoDecoder::SupportsDecryption() const {
   return supports_decryption_;
 }
 
-std::string MockVideoDecoder::GetDisplayName() const {
-  return decoder_name_;
-}
-
-VideoDecoderType MockVideoDecoder::GetDecoderType() const {
-  return VideoDecoderType::kUnknown;
-}
-
 MockAudioEncoder::MockAudioEncoder() = default;
 MockAudioEncoder::~MockAudioEncoder() {
   OnDestruct();
@@ -115,17 +107,17 @@ MockVideoEncoder::~MockVideoEncoder() {
   Dtor();
 }
 
-MockAudioDecoder::MockAudioDecoder() : MockAudioDecoder("MockAudioDecoder") {}
+MockAudioDecoder::MockAudioDecoder() : MockAudioDecoder(0) {}
 
-MockAudioDecoder::MockAudioDecoder(std::string decoder_name)
-    : MockAudioDecoder(false, false, std::move(decoder_name)) {}
+MockAudioDecoder::MockAudioDecoder(int decoder_id)
+    : MockAudioDecoder(false, false, decoder_id) {}
 
 MockAudioDecoder::MockAudioDecoder(bool is_platform_decoder,
                                    bool supports_decryption,
-                                   std::string decoder_name)
+                                   int decoder_id)
     : is_platform_decoder_(is_platform_decoder),
       supports_decryption_(supports_decryption),
-      decoder_name_(decoder_name) {}
+      decoder_id_(decoder_id) {}
 
 MockAudioDecoder::~MockAudioDecoder() = default;
 
@@ -137,14 +129,6 @@ bool MockAudioDecoder::SupportsDecryption() const {
   return supports_decryption_;
 }
 
-std::string MockAudioDecoder::GetDisplayName() const {
-  return decoder_name_;
-}
-
-AudioDecoderType MockAudioDecoder::GetDecoderType() const {
-  return AudioDecoderType::kUnknown;
-}
-
 MockRendererClient::MockRendererClient() = default;
 
 MockRendererClient::~MockRendererClient() = default;

media/base/mock_filters.h

@@ -220,17 +220,25 @@ class MockDemuxerStream : public DemuxerStream {
 class MockVideoDecoder : public VideoDecoder {
  public:
   MockVideoDecoder();
-  explicit MockVideoDecoder(std::string decoder_name);
+  // Give a decoder a specific ID, like 42, so that different decoders in unit
+  // tests can be differentiated from one another. All of these decoders have
+  // a decoder type of kTesting, so that can't be used to differentiate them.
+  explicit MockVideoDecoder(int decoder_id);
   MockVideoDecoder(bool is_platform_decoder,
                    bool supports_decryption,
-                   std::string decoder_name);
+                   int decoder_id);
   ~MockVideoDecoder() override;
 
   // Decoder implementation
   bool IsPlatformDecoder() const override;
   bool SupportsDecryption() const override;
-  std::string GetDisplayName() const override;
-  VideoDecoderType GetDecoderType() const override;
+  VideoDecoderType GetDecoderType() const override {
+    return VideoDecoderType::kTesting;
+  }
+
+  // Allows getting the unique ID from a mock decoder so that they can be
+  // identified during tests without having to add unique VideoDecoderTypes.
+  int GetDecoderId() const { return decoder_id_; }
 
   // VideoDecoder implementation.
   void Initialize(const VideoDecoderConfig& config,
@@ -262,7 +270,7 @@ class MockVideoDecoder : public VideoDecoder {
  private:
   const bool is_platform_decoder_;
   const bool supports_decryption_;
-  const std::string decoder_name_;
+  const int decoder_id_ = 0;
   DISALLOW_COPY_AND_ASSIGN(MockVideoDecoder);
 };
 
@@ -335,17 +343,22 @@ class MockVideoEncoder : public VideoEncoder {
 class MockAudioDecoder : public AudioDecoder {
  public:
   MockAudioDecoder();
-  explicit MockAudioDecoder(std::string decoder_name);
+  explicit MockAudioDecoder(int decoder_id);
   explicit MockAudioDecoder(bool is_platform_decoder,
                             bool supports_decryption,
-                            std::string decoder_name);
+                            int decoder_id);
   ~MockAudioDecoder() override;
 
   // Decoder implementation
   bool IsPlatformDecoder() const override;
   bool SupportsDecryption() const override;
-  std::string GetDisplayName() const override;
-  AudioDecoderType GetDecoderType() const override;
+  AudioDecoderType GetDecoderType() const override {
+    return AudioDecoderType::kTesting;
+  }
+
+  // Allows getting the unique ID from a mock decoder so that they can be
+  // identified during tests without having to add unique VideoDecoderTypes.
+  int GetDecoderId() const { return decoder_id_; }
 
   // AudioDecoder implementation.
   void Initialize(const AudioDecoderConfig& config,
@@ -368,7 +381,7 @@ class MockAudioDecoder : public AudioDecoder {
  private:
   const bool is_platform_decoder_;
   const bool supports_decryption_;
-  const std::string decoder_name_;
+  const int decoder_id_ = 0;
   DISALLOW_COPY_AND_ASSIGN(MockAudioDecoder);
 };
 

media/filters/android/media_codec_audio_decoder.cc

@@ -48,10 +48,6 @@ MediaCodecAudioDecoder::~MediaCodecAudioDecoder() {
   ClearInputQueue(DecodeStatus::ABORTED);
 }
 
-std::string MediaCodecAudioDecoder::GetDisplayName() const {
-  return "MediaCodecAudioDecoder";
-}
-
 AudioDecoderType MediaCodecAudioDecoder::GetDecoderType() const {
   return AudioDecoderType::kMediaCodec;
 }

media/filters/android/media_codec_audio_decoder.h

@@ -85,7 +85,6 @@ class MEDIA_EXPORT MediaCodecAudioDecoder : public AudioDecoder,
   ~MediaCodecAudioDecoder() override;
 
   // AudioDecoder implementation.
-  std::string GetDisplayName() const override;
   AudioDecoderType GetDecoderType() const override;
   void Initialize(const AudioDecoderConfig& config,
                   CdmContext* cdm_context,

media/filters/dav1d_video_decoder.cc

@@ -151,10 +151,6 @@ Dav1dVideoDecoder::~Dav1dVideoDecoder() {
   CloseDecoder();
 }
 
-std::string Dav1dVideoDecoder::GetDisplayName() const {
-  return "Dav1dVideoDecoder";
-}
-
 VideoDecoderType Dav1dVideoDecoder::GetDecoderType() const {
   return VideoDecoderType::kDav1d;
 }

media/filters/dav1d_video_decoder.h

@@ -33,7 +33,6 @@ class MEDIA_EXPORT Dav1dVideoDecoder : public OffloadableVideoDecoder {
 
   // VideoDecoder implementation.
   VideoDecoderType GetDecoderType() const override;
-  std::string GetDisplayName() const override;
   void Initialize(const VideoDecoderConfig& config,
                   bool low_delay,
                   CdmContext* cdm_context,

media/filters/decoder_selector.cc

@@ -286,9 +286,9 @@ void DecoderSelector<StreamType>::InitializeDecoder() {
   decoders_.erase(decoders_.begin());
   is_platform_decoder_ = decoder_->IsPlatformDecoder();
   TRACE_EVENT_ASYNC_STEP_INTO0("media", kSelectDecoderTrace, this,
-                               decoder_->GetDisplayName());
+                               GetDecoderName(decoder_->GetDecoderType()));
 
-  DVLOG(2) << __func__ << ": initializing " << decoder_->GetDisplayName();
+  DVLOG(2) << __func__ << ": initializing " << decoder_->GetDecoderType();
   const bool is_live = stream_->liveness() == DemuxerStream::LIVENESS_LIVE;
   traits_->InitializeDecoder(
       decoder_.get(), config_, is_live, cdm_context_,
@@ -299,7 +299,7 @@ void DecoderSelector<StreamType>::InitializeDecoder() {
 
 template <DemuxerStream::Type StreamType>
 void DecoderSelector<StreamType>::OnDecoderInitializeDone(Status status) {
-  DVLOG(2) << __func__ << ": " << decoder_->GetDisplayName()
+  DVLOG(2) << __func__ << ": " << decoder_->GetDecoderType()
            << " success=" << std::hex << status.code();
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
@@ -308,7 +308,7 @@ void DecoderSelector<StreamType>::OnDecoderInitializeDone(Status status) {
     // together and then send them as an informational notice instead of
     // using NotifyError.
     MEDIA_LOG(INFO, media_log_)
-        << "Failed to initialize " << decoder_->GetDisplayName();
+        << "Failed to initialize " << decoder_->GetDecoderType();
 
     // Try the next decoder on the list.
     decoder_.reset();
@@ -381,7 +381,9 @@ void DecoderSelector<StreamType>::RunSelectDecoderCB() {
       "media", kSelectDecoderTrace, this, "type",
       DemuxerStream::GetTypeName(StreamType), "decoder",
       base::StringPrintf(
-          "%s (%s)", decoder_ ? decoder_->GetDisplayName().c_str() : "null",
+          "%s (%s)",
+          decoder_ ? GetDecoderName(decoder_->GetDecoderType()).c_str()
+                   : "null",
           decrypting_demuxer_stream_ ? "encrypted" : "unencrypted"));
 
   task_runner_->PostTask(