Remove TODOs

Some TODOs remain but are replaced with word NOTE instead. Some TODOs
are turned into bugs.

The patch are prepared using script:
find . -regex ".*/\(app\|media\|mpd\)/.*\.\(cc\|h\|gyp\)" \
  -exec sed -i "/TODO/d" {} \;
(remove the line containing TODO) with some post editing.

Change-Id: I6dd3539cce2bbeefee32d6307f78c13aacb94d1b

media/base/aes_encryptor.cc

@@ -50,7 +50,6 @@ AesCtrEncryptor::~AesCtrEncryptor() {}
 
 bool AesCtrEncryptor::InitializeWithRandomIv(const std::vector<uint8>& key,
                                              uint8 iv_size) {
-  // TODO: Should we use RAND_bytes provided by openssl instead?
   std::vector<uint8> iv(iv_size, 0);
   base::RandBytes(&iv[0], iv_size);
   return InitializeWithIv(key, iv);

media/base/buffers.h

@@ -1,43 +0,0 @@
-// 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.
-
-// Timestamps are derived directly from the encoded media file and are commonly
-// known as the presentation timestamp (PTS).  Durations are a best-guess and
-// are usually derived from the sample/frame rate of the media file.
-//
-// Due to encoding and transmission errors, it is not guaranteed that timestamps
-// arrive in a monotonically increasing order nor that the next timestamp will
-// be equal to the previous timestamp plus the duration.
-//
-// In the ideal scenario for a 25fps movie, buffers are timestamped as followed:
-//
-//               Buffer0      Buffer1      Buffer2      ...      BufferN
-// Timestamp:        0us      40000us      80000us      ...   (N*40000)us
-// Duration*:    40000us      40000us      40000us      ...       40000us
-//
-//  *25fps = 0.04s per frame = 40000us per frame
-
-#ifndef MEDIA_BASE_BUFFERS_H_
-#define MEDIA_BASE_BUFFERS_H_
-
-#include "base/basictypes.h"
-#include "base/time/time.h"
-
-namespace media {
-
-// TODO: Move the contents of this file elsewhere.
-
-// Indicates an invalid or missing timestamp.
-extern inline base::TimeDelta kNoTimestamp() {
-  return base::TimeDelta::FromMicroseconds(kint64min);
-}
-
-// Represents an infinite stream duration.
-extern inline base::TimeDelta kInfiniteDuration() {
-  return base::TimeDelta::FromMicroseconds(kint64max);
-}
-
-}  // namespace media
-
-#endif  // MEDIA_BASE_BUFFERS_H_

media/base/decrypt_config.h

@@ -60,7 +60,6 @@ class DecryptConfig {
   // Initialization vector.
   const std::string iv_;
 
-  // TODO: Remove |data_offset_| if there is no plan to use it in the future.
   // Amount of data to be discarded before applying subsample information.
   const int data_offset_;
 

media/base/demuxer.cc

@@ -70,7 +70,6 @@ Status Demuxer::Initialize() {
     return Status(error::PARSER_FAILURE,
                   "Cannot parse media file " + file_name_);
 
-  // TODO: Does not look clean. Consider refactoring later.
   Status status;
   while (!init_event_received_) {
     if (!(status = Parse()).ok())

media/base/encryptor_source.h

@@ -30,7 +30,6 @@ class EncryptorSource {
   virtual Status Initialize() = 0;
 
   // Refresh the encryptor. NOP except for key rotation encryptor source.
-  // TODO: Do we need to pass in duration or fragment number?
   virtual void RefreshEncryptor() {}
 
   // Create an encryptor from this encryptor source. The encryptor will be

media/base/http_fetcher.h

@@ -38,7 +38,6 @@ class HttpFetcher {
 // A simple HttpFetcher implementation using happyhttp.
 class SimpleHttpFetcher : public HttpFetcher {
  public:
-  // TODO: Add timeout support.
   SimpleHttpFetcher();
   virtual ~SimpleHttpFetcher();
 

media/base/media_base.gyp

@@ -67,7 +67,6 @@
         'audio_stream_info.h',
         'bit_reader.cc',
         'bit_reader.h',
-        'buffers.h',
         'buffer_reader.cc',
         'buffer_reader.h',
         'buffer_writer.cc',

media/base/media_parser.h

@@ -57,7 +57,7 @@ class MediaParser {
   // Called when there is new data to parse.
   //
   // Returns true if the parse succeeds.
-  // TODO: Change to return Status.
+  // NOTE: Change to return Status.
   virtual bool Parse(const uint8* buf, int size) = 0;
 
  private:

media/base/media_sample.h

@@ -43,7 +43,6 @@ class MediaSample : public base::RefCountedThreadSafe<MediaSample> {
   //
   // Calling any method other than end_of_stream() on the resulting buffer
   // is disallowed.
-  // TODO: Do we need it?
   static scoped_refptr<MediaSample> CreateEOSBuffer();
 
   int64 dts() const {

media/base/widevine_encryptor_source.cc

@@ -15,8 +15,6 @@
 #include "media/base/http_fetcher.h"
 #include "media/base/request_signer.h"
 
-// TODO: Move media/mp4/rcheck.h to media/base/. Remove this definition and use
-// RCHECK in rcheck.h instead.
 #define RCHECK(x)                                       \
   do {                                                  \
     if (!(x)) {                                         \
@@ -70,7 +68,6 @@ bool GetPssh(const base::DictionaryValue& track_dict,
              std::vector<uint8>* pssh) {
   DCHECK(pssh);
 
-  // TODO: Add support for multiple pssh.
   const base::ListValue* pssh_list;
   RCHECK(track_dict.GetList("pssh", &pssh_list));
   // Invariant check. We don't want to crash in release mode if possible.
@@ -186,7 +183,6 @@ void WidevineEncryptorSource::FillRequest(const std::string& content_id,
 
   base::DictionaryValue request_dict;
   request_dict.SetString("content_id", content_id_base64_string);
-  // TODO: Determine the need for policy.
   request_dict.SetString("policy", "");
 
   // Build tracks.

media/event/muxer_listener.h

@@ -20,8 +20,6 @@ class StreamInfo;
 
 namespace event {
 
-// TODO: Need a solution to report a problem to the user. One idea is to add
-// GetStatus() method somewhere (maybe in MuxerListener, maybe not).
 class MuxerListener {
  public:
   enum ContainerType {

media/event/vod_media_info_dump_muxer_listener.cc

@@ -125,8 +125,11 @@ void VodMediaInfoDumpMuxerListener::OnMediaEnd(
   }
 
   if (is_encrypted) {
-    // TODO: Use the return value to set error status.
-    AddContentProtectionElements(container_type_, scheme_id_uri_, &media_info);
+    if (!AddContentProtectionElements(
+            container_type_, scheme_id_uri_, &media_info)) {
+      LOG(ERROR) << "Failed to add content protection elements.";
+      return;
+    }
   }
 
   SerializeMediaInfoToFile(media_info);

media/event/vod_media_info_dump_muxer_listener.h

@@ -46,7 +46,6 @@ class VodMediaInfoDumpMuxerListener : public MuxerListener {
                             uint32 time_scale,
                             ContainerType container_type) OVERRIDE;
 
-  // TODO: Make an Event structure for passing parameters.
   virtual void OnMediaEnd(const std::vector<StreamInfo*>& stream_infos,
                           bool has_init_range,
                           uint64 init_range_start,

media/event/vod_muxer_listener_internal.cc

@@ -113,7 +113,6 @@ void AddVideoInfo(const VideoStreamInfo* video_stream_info,
   if (!extra_data.empty()) {
     video_info->set_decoder_config(&extra_data[0], extra_data.size());
   }
-  // TODO: Get frame duration.
 }
 
 void AddAudioInfo(const AudioStreamInfo* audio_stream_info,
@@ -182,8 +181,6 @@ bool GenerateMediaInfo(const MuxerOptions& muxer_options,
                        MediaInfo* media_info) {
   DCHECK(media_info);
   if (file_size == 0) {
-    // TODO: |bandwidth| is a required field for MPD. But without the file size,
-    // AFAIK there's not much I can do. Fail silently?
     LOG(ERROR) << "File size not specified.";
     return false;
   }

media/mp4/box_definitions.cc

@@ -1449,7 +1449,6 @@ bool TrackFragmentHeader::ReadWrite(BoxBuffer* buffer) {
   RCHECK(FullBox::ReadWrite(buffer) &&
          buffer->ReadWriteUInt32(&track_id));
 
-  // TODO: We should support base-data-offset-present.
   // Media Source specific: reject tracks that set 'base-data-offset-present'.
   // Although the Media Source requires that 'default-base-is-moof' (14496-12
   // Amendment 2) be set, we omit this check as many otherwise-valid files in
@@ -1457,7 +1456,10 @@ bool TrackFragmentHeader::ReadWrite(BoxBuffer* buffer) {
   //
   //  RCHECK((flags & kDefaultBaseIsMoofMask) &&
   //         !(flags & kBaseDataOffsetPresentMask));
-  RCHECK(!(flags & kDataOffsetPresentMask));
+  if (flags & kDataOffsetPresentMask) {
+    NOTIMPLEMENTED() << " base-data-offset-present is not supported.";
+    return false;
+  }
 
   if (flags & kSampleDescriptionIndexPresentMask) {
     RCHECK(buffer->ReadWriteUInt32(&sample_description_index));
@@ -1514,7 +1516,7 @@ bool TrackFragmentRun::ReadWrite(BoxBuffer* buffer) {
   if (data_offset_present) {
     RCHECK(buffer->ReadWriteUInt32(&data_offset));
   } else {
-    // TODO: If the data-offset is not present, then the data for this run
+    // NOTE: If the data-offset is not present, then the data for this run
     // starts immediately after the data of the previous run, or at the
     // base-data-offset defined by the track fragment header if this is the
     // first run in a track fragment. If the data-offset is present, it is

media/mp4/es_descriptor.cc

@@ -131,9 +131,6 @@ bool ESDescriptor::ParseDecoderSpecificInfo(BitReader* reader) {
 }
 
 void ESDescriptor::Write(BufferWriter* writer) const {
-  // TODO: Consider writing Descriptor classes.
-  // ElementaryStreamDescriptor, DecoderConfigDescriptor, SLConfigDescriptor,
-  // DecoderSpecificInfoDescriptor.
   DCHECK(writer);
   CHECK_LT(decoder_specific_info_.size(), kMaxDecoderSpecificInfoSize);
 

media/mp4/mp4_fragmenter.cc

@@ -87,7 +87,8 @@ Status MP4Fragmenter::AddSample(scoped_refptr<MediaSample> sample) {
   if (normalize_presentation_timestamp_) {
     // Normalize PTS to start from 0. Some players do not like non-zero
     // presentation starting time.
-    // TODO: Do we need to add an EditList?
+    // NOTE: The timeline of the remuxed video may not be exactly the same as
+    // the original video. An EditList box may be useful to solve this.
     if (presentation_start_time_ == kInvalidTime) {
       presentation_start_time_ = pts;
       pts = 0;
@@ -188,7 +189,7 @@ void MP4Fragmenter::FinalizeFragment() {
 }
 
 void MP4Fragmenter::GenerateSegmentReference(SegmentReference* reference) {
-  // TODO: Support daisy chain?
+  // NOTE: Daisy chain is not supported currently.
   reference->reference_type = false;
   reference->subsegment_duration = fragment_duration_;
   reference->starts_with_sap = StartsWithSAP();

media/mp4/mp4_general_segmenter.cc

@@ -58,8 +58,6 @@ Status MP4GeneralSegmenter::Initialize(
   return status;
 }
 
-// TODO: Maybe GetInitRange() should return true. Init segment does exist and we
-// know the size and offset.
 bool MP4GeneralSegmenter::GetInitRange(size_t* offset, size_t* size) {
   DLOG(INFO) << "MP4GeneralSegmenter outputs init segment: "
              << options().output_file_name;
@@ -154,7 +152,6 @@ Status MP4GeneralSegmenter::WriteSegment() {
           "Cannot open file for append " + options().output_file_name);
     }
   } else {
-    // TODO: Generate the segment template name.
     file_name = options().segment_template;
     ReplaceSubstringsAfterOffset(
         &file_name, 0, "$Number$", base::UintToString(++num_segments_));

media/mp4/mp4_media_parser.cc

@@ -106,8 +106,6 @@ bool MP4MediaParser::ParseBox(bool* err) {
 
     NOTIMPLEMENTED() << " Files with MDAT before MOOV is not supported yet.";
     *err = true;
-    // TODO: Change to return a status so upper level can take appropriate
-    // actions based on the returned status.
     return false;
   }
 
@@ -164,9 +162,6 @@ bool MP4MediaParser::ParseMoov(BoxReader* reader) {
     const SampleDescription& samp_descr =
         track->media.information.sample_table.description;
 
-    // TODO: When codec reconfigurations are supported, detect and send a codec
-    // reconfiguration for fragments using a sample description index different
-    // from the previous one.
     size_t desc_idx = 0;
 
     // Read sample description index from mvex if it exists otherwise read

media/mp4/mp4_vod_segmenter.cc

@@ -124,8 +124,9 @@ Status MP4VODSegmenter::FinalizeSegment() {
       refs[0].sap_delta_time + refs[0].earliest_presentation_time;
   for (uint32 i = 1; i < sidx()->references.size(); ++i) {
     vod_ref.referenced_size += refs[i].referenced_size;
-    // TODO: Should we calculate subsegment duration by subtracting
-    // earliest_presentation time instead?
+    // NOTE: We calculate subsegment duration based on the total duration of
+    // this subsegment instead of subtracting earliest_presentation_time as
+    // indicated in the spec.
     vod_ref.subsegment_duration += refs[i].subsegment_duration;
     vod_ref.earliest_presentation_time = std::min(
         vod_ref.earliest_presentation_time, refs[i].earliest_presentation_time);

mpd/base/mpd_builder.cc

@@ -17,7 +17,6 @@
 #include "third_party/libxml/src/include/libxml/tree.h"
 #include "third_party/libxml/src/include/libxml/xmlstring.h"
 
-// TODO: If performance is a problem, work on fine grained locking.
 namespace dash_packager {
 
 using xml::XmlNode;
@@ -33,8 +32,7 @@ std::string GetMimeType(
     case MediaInfo::CONTAINER_MP4:
       return prefix + "/mp4";
     case MediaInfo::CONTAINER_MPEG2_TS:
-      // TODO: Find out whether uppercase or lowercase should be used for mp2t.
-      // DASH MPD spec uses lowercase but RFC3555 says uppercase.
+      // NOTE: DASH MPD spec uses lowercase but RFC3555 says uppercase.
       return prefix + "/MP2T";
     case MediaInfo::CONTAINER_WEBM:
       return prefix + "/webm";
@@ -112,7 +110,7 @@ bool MpdBuilder::WriteMpd() {
   std::string mpd;
   bool result = ToStringImpl(&mpd);
 
-  // TODO: Write to file, after interface change.
+  // NOTE: Write to file, after interface change.
   return result;
 }
 
@@ -144,7 +142,6 @@ bool MpdBuilder::ToStringImpl(std::string* output) {
   return true;
 }
 
-// TODO: This function is too big.
 xmlDocPtr MpdBuilder::GenerateMpd() {
   // Setup nodes.
   static const char kXmlVersion[] = "1.0";
@@ -152,7 +149,6 @@ xmlDocPtr MpdBuilder::GenerateMpd() {
   XmlNode mpd("MPD");
   AddMpdNameSpaceInfo(&mpd);
 
-  // TODO: Currently set to 2. Does this need calculation?
   const float kMinBufferTime = 2.0f;
   mpd.SetStringAttribute("minBufferTime", SecondsToXmlDuration(kMinBufferTime));
 
@@ -314,7 +310,6 @@ bool Representation::Init() {
   const bool has_audio_info = media_info_.audio_info_size() > 0;
 
   if (!has_video_info && !has_audio_info) {
-    // TODO: Allow text input.
     // This is an error. Segment information can be in AdaptationSet, Period, or
     // MPD but the interface does not provide a way to set them.
     // See 5.3.9.1 ISO 23009-1:2012 for segment info.
@@ -323,8 +318,6 @@ bool Representation::Init() {
   }
 
   if (media_info_.container_type() == MediaInfo::CONTAINER_UNKNOWN) {
-    // TODO: This might not be the right behavior. Maybe somehow infer from
-    // something else like media file name?
     LOG(ERROR) << "'container_type' in MediaInfo cannot be CONTAINER_UNKNOWN.";
     return false;
   }
@@ -354,10 +347,6 @@ bool Representation::AddNewSegment(uint64 start_time, uint64 duration) {
   return true;
 }
 
-// TODO: We don't need to create a node every single time. Make an internal copy
-// of this element. Then move most of the logic to RepresentationXmlNode so that
-// all the work is done in it so that this class just becomes a thin layer.
-//
 // Uses info in |media_info_| and |content_protection_elements_| to create a
 // "Representation" node.
 // MPD schema has strict ordering. The following must be done in order.
@@ -398,14 +387,12 @@ xml::ScopedXmlPtr<xmlNode>::type Representation::GetXml() {
   if (!representation.AddContentProtectionElementsFromMediaInfo(media_info_))
     return xml::ScopedXmlPtr<xmlNode>::type();
 
-  // TODO: Add TextInfo.
   if (HasVODOnlyFields(media_info_) &&
       !representation.AddVODOnlyInfo(media_info_)) {
     LOG(ERROR) << "Failed to add VOD segment info.";
     return xml::ScopedXmlPtr<xmlNode>::type();
   }
 
-  // TODO: Handle Live case. Handle data in segment_starttime_duration_pairs_.
   return representation.PassScopedPtr();
 }
 

mpd/base/mpd_builder.h

@@ -23,8 +23,6 @@
 #include "mpd/base/mpd_utils.h"
 #include "mpd/base/xml/scoped_xml_ptr.h"
 
-// TODO: For classes with |id_|, consider removing the field and let the MPD
-// (XML) generation functions take care of assigning an ID to each element.
 namespace dash_packager {
 
 class AdaptationSet;
@@ -51,8 +49,6 @@ class MpdBuilder {
   // The returned pointer is owned by this object.
   AdaptationSet* AddAdaptationSet();
 
-  // TODO: Once File interface is defined, make this method take a pointer to a
-  // File.
   // This will write to stdout until File interface is defined.
   bool WriteMpd();
   bool ToString(std::string* output);
@@ -76,7 +72,6 @@ class MpdBuilder {
 
   std::list<std::string> base_urls_;
 
-  // TODO: Investigate alternatives to locks.
   base::Lock lock_;
   base::AtomicSequenceNumber adaptation_set_counter_;
   base::AtomicSequenceNumber representation_counter_;

mpd/base/mpd_builder_unittest.cc

@@ -51,8 +51,6 @@ TEST_F(StaticMpdBuilderTest, VideoAndAudio) {
   MediaInfo audio_media_info = GetTestMediaInfo(kFileNameAudioMediaInfo1);
 
   // The order matters here to check against expected output.
-  // TODO: Investigate if I can deal with IDs and order elements
-  // deterministically.
   AdaptationSet* video_adaptation_set = mpd_.AddAdaptationSet();
   ASSERT_TRUE(video_adaptation_set);
 

mpd/base/simple_vod_mpd_notifier.h

@@ -20,7 +20,6 @@ namespace dash_packager {
 // container is for an AdaptationSet.
 class SimpleVodMpdNotifier : public MpdNotifier {
  public:
-  // TODO: Take File pointer for MPD output.
   // MpdBuilder must be initialized before passing a pointer to this object.
   // The ownership of |mpd_builder| does not transfer to this object and it must
   // be non-NULL.

mpd/base/xml/xml_node.cc

@@ -340,7 +340,6 @@ bool RepresentationXmlNode::AddAudioInfo(
 
   AddAudioSamplingRateInfo(repeated_audio_info);
 
-  // TODO: Find out where language goes.
   return true;
 }
 

mpd/base/xml/xml_node.h

@@ -55,9 +55,6 @@ class XmlNode {
   // any methods of this object, except the destructor, is undefined.
   xmlNodePtr Release();
 
-  // TODO: This isn't elegant. The only place this is used is when getting the
-  // duration of the MPD. Maybe make MpdXmlNode that does stuff internally, for
-  // example get 'duration' from all Representation nodes?
   xmlNodePtr GetRawPtr();
 
  private:
@@ -100,7 +97,6 @@ class AdaptationSetXmlNode : public RepresentationBaseXmlNode {
 };
 
 // RepresentationType in MPD.
-// TODO: Maybe provide methods to add mimetype, codecs, and bandwidth?
 class RepresentationXmlNode : public RepresentationBaseXmlNode {
  public:
   typedef ::google::protobuf::RepeatedPtrField<MediaInfo_VideoInfo>
@@ -119,7 +115,6 @@ class RepresentationXmlNode : public RepresentationBaseXmlNode {
 
   // Check MediaInfo protobuf definition for which fields are specific to VOD.
   bool AddVODOnlyInfo(const MediaInfo& media_info);
-  // TODO: Add Live info.
 
  private:
   // Add AudioChannelConfiguration elements. This will add multiple

mpd/base/xml/xml_node_unittest.cc

@@ -15,10 +15,6 @@ namespace xml {
 
 namespace {
 
-// TODO: Add XmlStringCompare() that does not care about the prettiness of the
-// string representation of the XML. We currently use CollapseWhitespaceASCII()
-// with carefully handcrafted expectations so that we can compare the result.
-
 // Template so that it works for ContentProtectionXml and
 // ContentProtectionXml::Element.
 template <typename XmlElement>

mpd/util/mpd_writer.cc

@@ -156,8 +156,7 @@ void MpdWriter::AddBaseUrl(const std::string& base_url) {
   base_urls_.push_back(base_url);
 }
 
-// TODO: The only use case we have for this is static profile, i.e.
-// VOD. But we might want to support dynamic profile for live.
+// NOTE: The only use case we have for this is static profile, i.e. VOD.
 bool MpdWriter::WriteMpdToString(std::string* output) {
   CHECK(output);
 
@@ -180,8 +179,6 @@ bool MpdWriter::WriteMpdToString(std::string* output) {
 bool MpdWriter::WriteMpdToFile(const char* file_name) {
   CHECK(file_name);
 
-  // TODO: MpdBuilder doesn't take File pointer yet. Once it does,
-  // skip intermediate ToString().
   std::string mpd;
   if (!WriteMpdToString(&mpd)) {
     LOG(ERROR) << "Failed to write MPD to string.";
@@ -194,8 +191,6 @@ bool MpdWriter::WriteMpdToFile(const char* file_name) {
     return false;
   }
 
-  // TODO: If File::Write() changes to best effort write-all then remove
-  // this loop.
   const char* mpd_char_ptr = mpd.data();
   size_t mpd_bytes_left = mpd.size();
   while (mpd_bytes_left > 0) {