Updated AAC:Parse() to emit better error logs.

media/formats/mp4/aac.cc

 // Copyright 2014 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/formats/mp4/aac.h"
 
 #include <algorithm>
 
 #include "base/logging.h"
 #include "media/base/bit_reader.h"
@@ skipping 20 matching lines @@
     return false;
 
   BitReader reader(&data[0], data.size());
   uint8 extension_type = 0;
   bool ps_present = false;
   uint8 extension_frequency_index = 0xff;
 
   frequency_ = 0;
   extension_frequency_ = 0;
 
   // The following code is written according to ISO 14496 Part 3 Table 1.13 -

[wolenetz, 2015/09/15 23:53:17]

nit: s/14496 Part 3 Table 1.13/14496-3-2009 Table 1.15/, though profile_ doesn't
seem to allow for audioObjectTypeExt in the stream.

Given this and similar I saw on simple digging, I think we should keep this CL
simpler (refer to 14496-3-2005, not ...2009 table references) and add a TODO
(and file related crbug) to consider any necessary updates to comments/logic to
conform to the ...2009 spec.

So, please leave this comment as-is, but adjust the new MEDIA_LOG table
references to reference explicitly the 14496-3-2005 version and table numbers.
And add the TODO referencing a newly filed crbug please :)

[msu.koo, 2015/09/16 08:16:23]

Thank you for your comments. I tried to post a new issue for this, but maybe I
don't have enough permissions to raise all-platform type issue.
Can you raise it and let me know the url?

[wolenetz, 2015/09/16 19:39:19]

It looks to me like you have successfully filed https://crbug.com/532281 (or do
I misunderstand?)

[msu.koo, 2015/09/17 02:49:26]

Oh, I forgot to update this comment :) I filed that bug as linux issue, and you
already changed to all-platform. 
By the way, can you let me know how can I get the permission to edit issue?

[wolenetz, 2015/09/24 19:05:39]

The process for obtaining such edit issue permissions is documented at
https://www.chromium.org/getting-involved/get-bug-editing-privileges

For short-term, you can always request another contributor with those
permissions to make the necessary changes. For long-term, that documented
process is the way to go :)

   // Syntax of AudioSpecificConfig.
 
   // Read base configuration
   RCHECK(reader.ReadBits(5, &profile_));
   RCHECK(reader.ReadBits(4, &frequency_index_));
   if (frequency_index_ == 0xf)
     RCHECK(reader.ReadBits(24, &frequency_));
   RCHECK(reader.ReadBits(4, &channel_config_));
 
   // Read extension configuration.
   if (profile_ == 5 || profile_ == 29) {
     ps_present = (profile_ == 29);
     extension_type = 5;
     RCHECK(reader.ReadBits(4, &extension_frequency_index));
     if (extension_frequency_index == 0xf)
       RCHECK(reader.ReadBits(24, &extension_frequency_));
     RCHECK(reader.ReadBits(5, &profile_));
   }
 
-  MEDIA_LOG(INFO, media_log) << "Audio codec: mp4a.40." << std::hex
-                             << static_cast<int>(profile_);
-
   RCHECK(SkipDecoderGASpecificConfig(&reader));
   RCHECK(SkipErrorSpecificConfig());
 
   // Read extension configuration again
   // Note: The check for 16 available bits comes from the AAC spec.
   if (extension_type != 5 && reader.bits_available() >= 16) {
     uint16 sync_extension_type;
     uint8 sbr_present_flag;
     uint8 ps_present_flag;
 
@@ skipping 15 matching lines @@
               RCHECK(reader.ReadBits(1, &ps_present_flag));
               ps_present = ps_present_flag != 0;
             }
           }
         }
       }
     }
   }
 
   if (frequency_ == 0) {
-    RCHECK(frequency_index_ < kADTSFrequencyTableSize);
+    if (frequency_index_ >= kADTSFrequencyTableSize) {
+      MEDIA_LOG(ERROR, media_log)
+          << "Sampling Frequency Index(0x"
+          << std::hex << static_cast<int>(frequency_index_)
+          << ") is not supported. Please see ISO 14496 Part 3 Table 1.18 "
+          << "for supported Sampling Frequencies.";
+      return false;
+    }
     frequency_ = kADTSFrequencyTable[frequency_index_];
   }
 
   if (extension_frequency_ == 0 && extension_frequency_index != 0xff) {
-    RCHECK(extension_frequency_index < kADTSFrequencyTableSize);
+    if (extension_frequency_index >= kADTSFrequencyTableSize) {
+      MEDIA_LOG(ERROR, media_log)
+          << "Extension Sampling Frequency Index(0x"
+          << std::hex << static_cast<int>(extension_frequency_index)
+          << ") is not supported. Please see ISO 14496 Part 3 Table 1.18 "
+          << "for supported Sampling Frequencies.";
+      return false;
+    }
     extension_frequency_ = kADTSFrequencyTable[extension_frequency_index];
   }
 
   // When Parametric Stereo is on, mono will be played as stereo.
   if (ps_present && channel_config_ == 1) {
     channel_layout_ = CHANNEL_LAYOUT_STEREO;
   } else {
-    RCHECK(channel_config_ < kADTSChannelLayoutTableSize);
+    if (channel_config_ >= kADTSChannelLayoutTableSize) {
+      MEDIA_LOG(ERROR, media_log)
+          << "Channel Configuration("
+          << static_cast<int>(channel_config_)
+          << ") is not supported. Please see ISO 14496 Part 3 Table 1.19 "
+          << "for supported Channel Configurations.";
+      return false;
+    }
     channel_layout_ = kADTSChannelLayoutTable[channel_config_];
   }
 
-  return frequency_ != 0 && channel_layout_ != CHANNEL_LAYOUT_NONE &&
-         profile_ >= 1 && profile_ <= 4;
+  RCHECK_MEDIA_LOGGED(channel_layout_ != CHANNEL_LAYOUT_NONE, media_log,

[wolenetz, 2015/09/15 23:53:17]

nit: it looks like this log is also possible to test.

[msu.koo, 2015/09/16 08:16:23]

This code block is dead. "|channel_config_| == 0" is filtered out during
"RCHECK(SkipDecoderGASpecificConfig(&reader));"(line 61), which tested at line
263.

[wolenetz, 2015/09/16 19:39:19]

Acknowledged. Since the helper method might change its implementation, let's
protect our borders in debug builds at least and add a DCHECK(channel_layout_ !=
CHANNEL_LAYOUT_NONE) here.

[msu.koo, 2015/09/17 02:49:26]

Done.

+                      "Undefined Audio Channel layout with value "
+                      "CHANNEL_LAYOUT_NONE.");
+  if (profile_ < 1 || profile_ > 4) {
+    MEDIA_LOG(ERROR, media_log)
+        << "Audio codec(mp4a.40." << static_cast<int>(profile_)
+        << ") is not supported. Please see ISO 14496 Part 3 Table 1.3 "
+        << "for Audio Profile Definitions.";
+    return false;
+  }
+
+  MEDIA_LOG(INFO, media_log)
+      << "Audio codec: mp4a.40." << static_cast<int>(profile_)
+      << ". Sampling frequency: " << frequency_ << "Hz"
+      << ". Sampling frequency(Extension): " << extension_frequency_ << "Hz"
+      << ". Channel layout: " << channel_layout_ << ".";
+
+  return true;
 }
 
 int AAC::GetOutputSamplesPerSecond(bool sbr_in_mimetype) const {
   if (extension_frequency_ > 0)
     return extension_frequency_;
 
   if (!sbr_in_mimetype)
     return frequency_;
 
   // The following code is written according to ISO 14496 Part 3 Table 1.11 and
@@ skipping 113 matching lines @@
 
     RCHECK(bit_reader->ReadBits(1, &dummy));  // extensionFlag3
   }
 
   return true;
 }
 
 }  // namespace mp4
 
 }  // namespace media