Commit 1729147d authored by Dan Minor's avatar Dan Minor
Browse files

Bug 1379836 - Fix AEC Logging; r=jesup 

This enables apm logging by setting the apm_debug_dump variable in gyp.mozbuild.
Prior to this change, some files were including apm_data_dumper.h with logging
enabled and some were not.

This also removes the AEC* C interface and calls into webrtc::Trace directly.
Whatever historical reasons for having a C interface into these calls no
longer seems to apply. In addition reserving a buffer for the base file name
and then ensuring it was null terminated caused an ASAN "stack-buffer-overflow"
while testing. This was because it was not handling an empty base file
name properly. This would not normally happen if AEC logging was enabled through
about:webrtc, but it still seems safer to use std::string.

MozReview-Commit-ID: Ikz5xO74syA
parent 818be8be

build/gyp.mozbuild

+2 −0
Original line number Diff line number Diff line
@@ -88,6 +88,8 @@ gyp_vars.update({ 
    'include_pcm16b': 1,



    #'rtc_opus_variable_complexity': 1,



    'apm_debug_dump': 1,

})



if os == 'Android':

media/webrtc/trunk/webrtc/modules/audio_processing/logging/apm_data_dumper.cc

+5 −7
Original line number Diff line number Diff line
@@ -29,11 +29,6 @@ std::string FormFileName(const char* name, 
                         int instance_index,

                         int reinit_index,

                         const std::string& suffix) {

  char path[1024];

  AECDebugFilenameBase(path, sizeof(path));



  char* end = path + strlen(path) - 1;



#ifdef WEBRTC_WIN

  char sep = '\\';

#else
@@ -41,10 +36,13 @@ std::string FormFileName(const char* name, 
#endif



  std::stringstream ss;

  ss << path;

  if (*end != sep) {

  std::string base = webrtc::Trace::aec_debug_filename();

  ss << base;



  if (base.length() && base.back() != sep) {

    ss << sep;

  }



  ss << name << "_" << instance_index << "-" << reinit_index << suffix;

  return ss.str();

}

media/webrtc/trunk/webrtc/modules/audio_processing/logging/apm_data_dumper.h

+8 −16
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ 
#include "webrtc/base/array_view.h"

#include "webrtc/base/constructormagic.h"

#include "webrtc/common_audio/wav_file.h"

#include "webrtc/system_wrappers/include/trace.h"



// Check to verify that the define is properly set.

#if !defined(WEBRTC_APM_DEBUG_DUMP) || \
@@ -28,15 +29,6 @@ 
#error "Set WEBRTC_APM_DEBUG_DUMP to either 0 or 1"

#endif



#if WEBRTC_APM_DEBUG_DUMP == 1

extern "C" {

  extern int AECDebug();

  extern uint32_t AECDebugMaxSize();

  extern void AECDebugEnable(uint32_t enable);

  extern void AECDebugFilenameBase(char *buffer, size_t size);

}

#endif



namespace webrtc {



#if WEBRTC_APM_DEBUG_DUMP == 1
@@ -70,7 +62,7 @@ class ApmDataDumper { 
  // various formats.

  void DumpRaw(const char* name, int v_length, const float* v) {

#if WEBRTC_APM_DEBUG_DUMP == 1

    if (AECDebug()) {

    if (webrtc::Trace::aec_debug()) {

      FILE* file = GetRawFile(name);

      if (file) {

        fwrite(v, sizeof(v[0]), v_length, file);
@@ -87,7 +79,7 @@ class ApmDataDumper { 


  void DumpRaw(const char* name, int v_length, const bool* v) {

#if WEBRTC_APM_DEBUG_DUMP == 1

    if (AECDebug()) {

    if (webrtc::Trace::aec_debug()) {

      FILE* file = GetRawFile(name);

      if (file) {

        for (int k = 0; k < v_length; ++k) {
@@ -107,7 +99,7 @@ class ApmDataDumper { 


  void DumpRaw(const char* name, int v_length, const int16_t* v) {

#if WEBRTC_APM_DEBUG_DUMP == 1

    if (AECDebug()) {

    if (webrtc::Trace::aec_debug()) {

      FILE* file = GetRawFile(name);

      if (file) {

        fwrite(v, sizeof(v[0]), v_length, file);
@@ -124,7 +116,7 @@ class ApmDataDumper { 


  void DumpRaw(const char* name, int v_length, const int32_t* v) {

#if WEBRTC_APM_DEBUG_DUMP == 1

    if (AECDebug()) {

    if (webrtc::Trace::aec_debug()) {

      FILE* file = GetRawFile(name);

      if (file) {

        fwrite(v, sizeof(v[0]), v_length, file);
@@ -145,7 +137,7 @@ class ApmDataDumper { 
               int sample_rate_hz,

               int num_channels) {

#if WEBRTC_APM_DEBUG_DUMP == 1

    if (AECDebug()) {

    if (webrtc::Trace::aec_debug()) {

      WavWriter* file = GetWavFile(name, sample_rate_hz, num_channels);

      file->WriteSamples(v, v_length);

      // Cheat and use aec_near as a stand-in for "size of the largest file"
@@ -183,8 +175,8 @@ class ApmDataDumper { 
  void updateDebugWritten(uint32_t amount) {

    debug_written_ += amount;

    // Limit largest files to a specific (rough) size, to avoid filling up disk.

    if (debug_written_ >= AECDebugMaxSize()) {

      AECDebugEnable(0);

    if (debug_written_ >= webrtc::Trace::aec_debug_size()) {

      webrtc::Trace::set_aec_debug(false);

    }

  }

media/webrtc/trunk/webrtc/system_wrappers/include/trace.h

+1 −8
Original line number Diff line number Diff line
@@ -60,7 +60,7 @@ class Trace { 
  static void set_aec_debug_size(uint32_t size) { aec_debug_size_ = size; }

  static bool aec_debug() { return aec_debug_; }

  static uint32_t aec_debug_size() { return aec_debug_size_; }

  static void aec_debug_filename(char *buffer, size_t size);

  static std::string aec_debug_filename();

  static void set_aec_debug_filename(const char* filename) {

    aec_filename_base_ = filename;

  }
@@ -98,13 +98,6 @@ class Trace { 
  static std::string aec_filename_base_;

};



extern "C" {

  extern int AECDebug();

  extern uint32_t AECDebugMaxSize();

  extern void AECDebugEnable(uint32_t enable);

  extern void AECDebugFilenameBase(char *buffer, size_t size);

}



}  // namespace webrtc



#endif  // WEBRTC_SYSTEM_WRAPPERS_INCLUDE_TRACE_H_

media/webrtc/trunk/webrtc/system_wrappers/source/trace_impl.cc

+2 −12
Original line number Diff line number Diff line
@@ -29,15 +29,6 @@ 
#pragma warning(disable:4355)

#endif  // _WIN32



extern "C" {

  int AECDebug() { return (int) webrtc::Trace::aec_debug(); }

  uint32_t AECDebugMaxSize() { return webrtc::Trace::aec_debug_size(); }

  void AECDebugEnable(uint32_t enable) { webrtc::Trace::set_aec_debug(!!enable); }

  void AECDebugFilenameBase(char *buffer, size_t size) {

    webrtc::Trace::aec_debug_filename(buffer, size);

  }

}



namespace webrtc {



const int Trace::kBoilerplateLength = 71;
@@ -48,9 +39,8 @@ bool Trace::aec_debug_ = false; 
uint32_t Trace::aec_debug_size_ = 4*1024*1024;

std::string Trace::aec_filename_base_;



void Trace::aec_debug_filename(char *buffer, size_t size) {

  strncpy(buffer, aec_filename_base_.c_str(), size-1);

  buffer[size-1] = '\0';

std::string Trace::aec_debug_filename() {

  return aec_filename_base_;

}



// Construct On First Use idiom. Avoids "static initialization order fiasco".