Restructure media/learning experiment.

This CL renames core => common, updates build deps to be a less
tangled, renames a few classes, and updates imports.

Change-Id: I011f9ec6d704681b50da5be759341e7e3b48d076
Bug: 897463
Reviewed-on: https://chromium-review.googlesource.com/c/1289688
Commit-Queue: Frank Liberato <liberato@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#605443}

media/learning/BUILD.gn

@@ -2,40 +2,13 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-source_set("learning") {
-  # Do not expand the visibility here without double-checking with OWNERS, this
-  # is a roll-up target which is part of the //media component. Most other DEPs
-  # should be using //media and not directly DEP this roll-up target.
-  visibility = [ "//media" ]
-
-  sources = [
-    "instance.cc",
-    "instance.h",
-    "learner.h",
-    "learner_factory.h",
-    "learning_task.cc",
-    "learning_task.h",
-    "value.cc",
-    "value.h",
-  ]
-
-  public_deps = [
-    "//base",
-    "//media/base",
-  ]
-
-  configs += [ "//media:subcomponent_config" ]
-}
-
 source_set("unit_tests") {
   testonly = true
-  sources = [
-    "value_unittest.cc",
-  ]
 
   deps = [
     "//base/test:test_support",
     "//media:test_support",
+    "//media/learning/common:unit_tests",
     "//testing/gtest",
   ]
 }

media/learning/README.md

@@ -64,3 +64,8 @@ be found in the header for the class, an overview of the main ones is:
 ## Models
 
 All of our models are supervised.
+
+## Directory Structure
+
+ * `common/` - public interfaces
+ * `impl/` - learning algorithms, other implementation details

media/learning/common/BUILD.gn

+# Copyright 2018 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.
+
+component("common") {
+  output_name = "learning_common"
+  visibility = [
+    "//media/learning",
+    "//media/learning/impl:*",
+    "//media/learning/impl:test_support",
+    "//media/learning/common:unit_tests",
+  ]
+
+  defines = [ "IS_LEARNING_COMMON_IMPL" ]
+
+  sources = [
+    "instance.cc",
+    "instance.h",
+    "learning_session.cc",
+    "learning_session.h",
+    "learning_task.cc",
+    "learning_task.h",
+    "value.cc",
+    "value.h",
+  ]
+
+  deps = [
+    "//base",
+  ]
+}
+
+source_set("unit_tests") {
+  testonly = true
+  sources = [
+    "value_unittest.cc",
+  ]
+
+  deps = [
+    ":common",
+    "//base/test:test_support",
+    "//media:test_support",
+    "//testing/gtest",
+  ]
+}

media/learning/instance.cc → media/learning/common/instance.cc

@@ -2,16 +2,17 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "media/learning/instance.h"
+#include "media/learning/common/instance.h"
 
 namespace media {
 namespace learning {
 
 Instance::Instance() = default;
+
+Instance::Instance(std::initializer_list<FeatureValue> init_list)
+    : features(init_list) {}
+
 Instance::~Instance() = default;
-Instance::Instance(Instance&& rhs) noexcept {
-  features = std::move(rhs.features);
-}
 
 std::ostream& operator<<(std::ostream& out, const Instance& instance) {
   for (const auto& feature : instance.features)
@@ -20,5 +21,9 @@ std::ostream& operator<<(std::ostream& out, const Instance& instance) {
   return out;
 }
 
+bool Instance::operator==(const Instance& rhs) const {
+  return features == rhs.features;
+}
+
 }  // namespace learning
 }  // namespace media

media/learning/instance.h → media/learning/common/instance.h

@@ -2,38 +2,40 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef MEDIA_LEARNING_INSTANCE_H_
-#define MEDIA_LEARNING_INSTANCE_H_
+#ifndef MEDIA_LEARNING_COMMON_INSTANCE_H_
+#define MEDIA_LEARNING_COMMON_INSTANCE_H_
 
+#include <initializer_list>
 #include <ostream>
 #include <vector>
 
+#include "base/component_export.h"
 #include "base/macros.h"
-#include "media/base/media_export.h"
-#include "media/learning/value.h"
+#include "media/learning/common/value.h"
 
 namespace media {
 namespace learning {
 
 // One instance == group of feature values.
-struct MEDIA_EXPORT Instance {
+struct COMPONENT_EXPORT(LEARNING_COMMON) Instance {
   Instance();
+  Instance(std::initializer_list<FeatureValue> init_list);
   ~Instance();
-  // Declare a no-exception move constructor so that std::vector will use it.
-  Instance(Instance&& rhs) noexcept;
+
+  bool operator==(const Instance& rhs) const;
 
   // It's up to you to add the right number of features to match the learner
   // description.  Otherwise, the learner will ignore (training) or lie to you
   // (inference), silently.
   std::vector<FeatureValue> features;
 
-  DISALLOW_COPY_AND_ASSIGN(Instance);
+  // Copy / assignment is allowed.
 };
 
-MEDIA_EXPORT std::ostream& operator<<(std::ostream& out,
-                                      const Instance& instance);
+COMPONENT_EXPORT(LEARNING_COMMON)
+std::ostream& operator<<(std::ostream& out, const Instance& instance);
 
 }  // namespace learning
 }  // namespace media
 
-#endif  // MEDIA_LEARNING_INSTANCE_H_
+#endif  // MEDIA_LEARNING_COMMON_INSTANCE_H_

media/learning/learner_factory.h → media/learning/common/learning_session.cc

@@ -2,28 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef MEDIA_LEARNING_LEARNER_FACTORY_H_
-#define MEDIA_LEARNING_LEARNER_FACTORY_H_
-
-#include <string>
-
-#include "media/base/media_export.h"
-#include "media/learning/learing_task.h"
-#include "media/learning/learner.h"
+#include "media/learning/common/learning_session.h"
 
 namespace media {
 namespace learning {
 
-// Factory class for learner instances.
-class MEDIA_EXPORT LearnerFactory {
- public:
-  virtual ~LearnerFactory() = default;
+LearningSession::LearningSession() = default;
 
-  // Provide a learner that matches |task|.
-  virtual std::unique_ptr<Learner> CreateLearner(const LearningTask& task) = 0;
-};
+LearningSession::~LearningSession() = default;
 
 }  // namespace learning
 }  // namespace media
-
-#endif  // MEDIA_LEARNING_LEARNER_FACTORY_H_

media/learning/common/learning_session.h

+// Copyright 2018 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_LEARNING_COMMON_LEARNING_SESSION_H_
+#define MEDIA_LEARNING_COMMON_LEARNING_SESSION_H_
+
+#include <string>
+
+#include "base/component_export.h"
+#include "base/macros.h"
+#include "media/learning/common/learning_task.h"
+
+namespace media {
+namespace learning {
+
+// Interface to provide a Learner given the task name.
+class COMPONENT_EXPORT(LEARNING_COMMON) LearningSession {
+ public:
+  LearningSession();
+  virtual ~LearningSession();
+
+  // Add an observed example of |instance| with target value |target| to the
+  // learning task |task_name|.
+  // TODO(liberato): Consider making this an enum to match mojo.
+  virtual void AddExample(const std::string& task_name,
+                          const Instance& instance,
+                          const TargetValue& target) = 0;
+
+  // TODO(liberato): Add prediction API.
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(LearningSession);
+};
+
+}  // namespace learning
+}  // namespace media
+
+#endif  // MEDIA_LEARNING_COMMON_LEARNING_SESSION_H_

media/learning/learning_task.cc → media/learning/common/learning_task.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "media/learning/learning_task.h"
+#include "media/learning/common/learning_task.h"
 
 namespace media {
 namespace learning {

media/learning/learning_task.h → media/learning/common/learning_task.h

@@ -2,14 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef MEDIA_LEARNING_LEARNING_TASK_H_
-#define MEDIA_LEARNING_LEARNING_TASK_H_
+#ifndef MEDIA_LEARNING_COMMON_LEARNING_TASK_H_
+#define MEDIA_LEARNING_COMMON_LEARNING_TASK_H_
 
 #include <initializer_list>
 #include <string>
 
-#include "media/base/media_export.h"
-#include "media/learning/instance.h"
+#include "base/component_export.h"
+#include "media/learning/common/instance.h"
 
 namespace media {
 namespace learning {
@@ -18,7 +18,9 @@ namespace learning {
 // inputs (features) and output (target value), plus a choice of the model and
 // parameters for learning.
 // TODO(liberato): Consider separating the task from the choice of model.
-struct MEDIA_EXPORT LearningTask {
+// TODO(liberato): should this be in impl?  Probably not if we want to allow
+// registering tasks.
+struct COMPONENT_EXPORT(LEARNING_COMMON) LearningTask {
   // Not all models support all feature / target descriptions.  For example,
   // NaiveBayes requires kUnordered features.  Similarly, kLogLinear doesn't
   // support kUnordered features or targets.  kRandomForest might support more
@@ -93,4 +95,4 @@ struct MEDIA_EXPORT LearningTask {
 }  // namespace learning
 }  // namespace media
 
-#endif  // MEDIA_LEARNING_LEARNING_TASK_H_
+#endif  // MEDIA_LEARNING_COMMON_LEARNING_TASK_H_

media/learning/value.cc → media/learning/common/value.cc

@@ -2,14 +2,17 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "media/learning/value.h"
+#include "media/learning/common/value.h"
 
 #include "base/hash.h"
 
 namespace media {
 namespace learning {
 
+Value::Value() = default;
+
 Value::Value(int x) : value_(x) {}
+
 Value::Value(const char* x) {
   // std::hash would be nice, but it can (and does) change values between
   // different instances of the class.  In other words, Value("foo") !=

media/learning/value.h → media/learning/common/value.h

@@ -2,14 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef MEDIA_LEARNING_VALUE_H_
-#define MEDIA_LEARNING_VALUE_H_
+#ifndef MEDIA_LEARNING_COMMON_VALUE_H_
+#define MEDIA_LEARNING_COMMON_VALUE_H_
 
 #include <cstdint>
 #include <ostream>
 #include <string>
 
-#include "media/base/media_export.h"
+#include "base/component_export.h"
 
 namespace media {
 namespace learning {
@@ -22,8 +22,9 @@ namespace learning {
 // For numeric values, ==, !=, > operators behave as one would expect.
 //
 // For strings, only == and != are guaranteed to be meaningful.
-class MEDIA_EXPORT Value {
+class COMPONENT_EXPORT(LEARNING_COMMON) Value {
  public:
+  Value();
   explicit Value(int x);
   explicit Value(const char* x);
   explicit Value(const std::string& x);
@@ -34,10 +35,13 @@ class MEDIA_EXPORT Value {
   bool operator!=(const Value& rhs) const;
   bool operator<(const Value& rhs) const;
 
+  int64_t value() const { return value_; }
+
  private:
   int64_t value_ = 0;
 
-  friend MEDIA_EXPORT std::ostream& operator<<(std::ostream& out,
+  friend COMPONENT_EXPORT(LEARNING_COMMON) std::ostream& operator<<(
+      std::ostream& out,
       const Value& value);
 
   // Copy and assign are fine.
@@ -47,9 +51,10 @@ class MEDIA_EXPORT Value {
 using FeatureValue = Value;
 using TargetValue = Value;
 
-MEDIA_EXPORT std::ostream& operator<<(std::ostream& out, const Value& value);
+COMPONENT_EXPORT(LEARNING_COMMON)
+std::ostream& operator<<(std::ostream& out, const Value& value);
 
 }  // namespace learning
 }  // namespace media
 
-#endif  // MEDIA_LEARNING_VALUE_H_
+#endif  // MEDIA_LEARNING_COMMON_VALUE_H_

media/learning/value_unittest.cc → media/learning/common/value_unittest.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "media/learning/value.h"
+#include "media/learning/common/value.h"
 
 #include "testing/gtest/include/gtest/gtest.h"
 

media/learning/impl/BUILD.gn

+# Copyright 2018 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.
+
+component("impl") {
+  output_name = "learning_impl"
+  sources = [
+    "learner.h",
+    "learning_session_impl.cc",
+    "learning_session_impl.h",
+  ]
+
+  defines = [ "IS_LEARNING_IMPL_IMPL" ]
+
+  deps = [
+    "//base",
+  ]
+
+  public_deps = [
+    "//media/learning/common",
+  ]
+}

media/learning/learner.h → media/learning/impl/learner.h

@@ -2,12 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef MEDIA_LEARNING_LEARNER_H_
-#define MEDIA_LEARNING_LEARNER_H_
+#ifndef MEDIA_LEARNING_IMPL_LEARNER_H_
+#define MEDIA_LEARNING_IMPL_LEARNER_H_
 
+#include "base/component_export.h"
 #include "base/values.h"
-#include "media/base/media_export.h"
-#include "media/learning/instance.h"
+#include "media/learning/common/instance.h"
 
 namespace media {
 namespace learning {
@@ -16,7 +16,7 @@ namespace learning {
 // and trains a model to predict the target given the features.  The target may
 // be either nominal (classification) or numeric (regression), though this must
 // be chosen in advance when creating the learner via LearnerFactory.
-class MEDIA_EXPORT Learner {
+class COMPONENT_EXPORT(LEARNING_IMPL) Learner {
  public:
   virtual ~Learner() = default;
 
@@ -29,4 +29,4 @@ class MEDIA_EXPORT Learner {
 }  // namespace learning
 }  // namespace media
 
-#endif  // MEDIA_LEARNING_LEARNER_H_
+#endif  // MEDIA_LEARNING_IMPL_LEARNER_H_

media/learning/impl/learning_session_impl.cc

+// Copyright 2018 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.
+
+#include "media/learning/impl/learning_session_impl.h"
+
+#include "base/logging.h"
+
+namespace media {
+namespace learning {
+
+LearningSessionImpl::LearningSessionImpl() = default;
+LearningSessionImpl::~LearningSessionImpl() = default;
+
+void LearningSessionImpl::AddExample(const std::string& task_name,
+                                     const Instance& instance,
+                                     const TargetValue& target) {
+  // TODO: match |task_name| against a list of learning tasks, and find the
+  // learner(s) for it.  Then add |instance|, |target| to it.
+  NOTIMPLEMENTED();
+}
+
+}  // namespace learning
+}  // namespace media

media/learning/impl/learning_session_impl.h

+// Copyright 2018 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_LEARNING_IMPL_LEARNING_SESSION_IMPL_H_
+#define MEDIA_LEARNING_IMPL_LEARNING_SESSION_IMPL_H_
+
+#include "base/component_export.h"
+#include "media/learning/common/learning_session.h"
+
+namespace media {
+namespace learning {
+
+// Concrete implementation of a LearningSession.  This would have a list of
+// learning tasks, and could provide local learners for each task.
+class COMPONENT_EXPORT(LEARNING_IMPL) LearningSessionImpl
+    : public LearningSession {
+ public:
+  LearningSessionImpl();
+  ~LearningSessionImpl() override;
+
+  // LearningSession
+  void AddExample(const std::string& task_name,
+                  const Instance& instance,
+                  const TargetValue& target) override;
+};
+
+}  // namespace learning
+}  // namespace media
+
+#endif  // MEDIA_LEARNING_IMPL_LEARNING_SESSION_IMPL_H_

media/media_options.gni

@@ -21,7 +21,6 @@ media_subcomponent_deps = [
   "//media/device_monitors",
   "//media/filters",
   "//media/formats",
-  "//media/learning",
   "//media/muxers",
   "//media/renderers",
   "//media/video",

media/mojo/DEPS

 include_rules = [
   "+components/ukm/test_ukm_recorder.h",
-  "+mojo/converters",
-  "+mojo/logging",
   "+mojo/public",
 
   # For changing the bad message handler for tests.