Basic media tracks implementation for media stream parsers

media/base/media_tracks.h

Index: media/base/media_tracks.h
diff --git a/media/base/media_tracks.h b/media/base/media_tracks.h
new file mode 100644
index 0000000000000000000000000000000000000000..e1c9a320e655b5221d39710760f9afb867cb12ec
--- /dev/null
+++ b/media/base/media_tracks.h
@@ -0,0 +1,60 @@
+// Copyright (c) 2016 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_MEDIA_TRACKS_H_
+#define MEDIA_BASE_MEDIA_TRACKS_H_
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "base/callback.h"

[wolenetz, 2016/02/26 00:06:21]

nit: is this include necessary in this file?

[servolk, 2016/02/26 01:56:32]

Done.

+#include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
+#include "media/base/media_export.h"
+#include "media/base/media_track.h"
+
+namespace media {
+
+class AudioDecoderConfig;
+class VideoDecoderConfig;
+
+class MEDIA_EXPORT MediaTracks {
+ public:
+  MediaTracks();
+  virtual ~MediaTracks();

[wolenetz, 2016/02/26 00:06:21]

ditto: is virtual needed here?

[servolk, 2016/02/26 01:56:31]

Done.

+
+  void AddAudioTrack(const AudioDecoderConfig& config,

[wolenetz, 2016/02/26 00:06:21]

What is the behavior if any of these params are invalid (ditto for
AddVideoTrack())?
Similarly, where do we centralize the logic for "max 1 audio" "max 1 video"
track in Chromium as it is limited to right now? Should an attempt to add a
second video / audio track here be disallowed here and/or some other place too?

[servolk, 2016/02/26 01:56:32]

Well this CL was meant to be a quick prototype to see if it's even feasible to
achieve my goals this way, so I did a fairly minimal implementation and didn't
bother with proper error handling and all the corner cases. We can harden it
later.

[wolenetz, 2016/02/26 19:29:27]

Please add a comment that |id| collision within each track type (audio/video)
must be protected against by caller. Otherwise, we risk misunderstanding this
API later and assuming too much of it :)

[servolk, 2016/02/26 22:02:41]

Done + actually added CHECKs in the method impl to ensure we don't get
duplicated ids from callers.

+                     const std::string& id,
+                     const std::string& kind,
+                     const std::string& label,
+                     const std::string& language);
+  void AddVideoTrack(const VideoDecoderConfig& config,
+                     const std::string& id,
+                     const std::string& kind,
+                     const std::string& label,
+                     const std::string& language);
+
+  typedef std::vector<scoped_ptr<MediaTrack>> MediaTracksCollection;

[wolenetz, 2016/02/26 00:06:21]

nit: typedef at top of class (per
https://google.github.io/styleguide/cppguide.html#Declaration_Order)

[servolk, 2016/02/26 01:56:32]

Done.

+  const MediaTracksCollection& tracks() const { return tracks_; }
+
+  const AudioDecoderConfig& getAudioConfig(const std::string& id) const;

[wolenetz, 2016/02/26 00:06:21]

What is the behavior if there is no audio/video config for the supplied id?

[servolk, 2016/02/26 01:56:32]

it'll return an invalid a/v config. 

+  const VideoDecoderConfig& getVideoConfig(const std::string& id) const;
+
+  // TODO(servolk): These are temporary helpers useful until all code paths are

[wolenetz, 2016/02/26 00:06:21]

nit: what about text tracks?

[servolk, 2016/02/26 01:56:32]

Currently the text tracks seem to be already supported to some extent, although
in a very different way (at least from stream parser level perspective), I
wanted to avoid breaking them while prototyping this, so I didn't touch text
track code for now. Once we have a/v tracks working I can look into unifying
handling of a/v and t tracks.

+  // converted to properly handle multiple media tracks.
+  const AudioDecoderConfig& getFirstAudioConfig() const;

[wolenetz, 2016/02/26 00:06:21]

What is the behavior if there is no first audio/video config?

[servolk, 2016/02/26 01:56:32]

Current behavior is to return invalid A/V configs. These methods will be removed
soon, there's no major obstacles, I added them just to simplify converting old
code that typically used a pair of A/V configs directly, so that I could unblock
myself on making further progress on actual task at hand (media tracks).

+  const VideoDecoderConfig& getFirstVideoConfig() const;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MediaTracks);

[wolenetz, 2016/02/26 00:06:21]

nit: should be last thing in the class
(https://google.github.io/styleguide/cppguide.html#Declaration_Order)

[servolk, 2016/02/26 01:56:32]

Done.

+
+  MediaTracksCollection tracks_;
+  std::map<std::string, AudioDecoderConfig> audio_configs_;
+  std::map<std::string, VideoDecoderConfig> video_configs_;
+};
+
+}  // namespace media
+
+#endif  // MEDIA_BASE_MEDIA_TRACKS_H_