Fixed processing files to/from stdio

- Duration calculation - More consistent status messages - Error reporting if unable to perform requested conversion - Test cases for all combinations of file and stdio input and output - Prevent illegal seek error writing WAV output - Remove code duplication in WaveformBuffer
bbc · Aug 3, 2019 · 515a94f · 515a94f
1 parent e15fc09
commit 515a94f
Show file tree

Hide file tree

Showing 21 changed files with 438 additions and 225 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -1,6 +1,6 @@
 #-------------------------------------------------------------------------------
 #
-# Copyright 2013-2018 BBC Research and Development
+# Copyright 2013-2019 BBC Research and Development
 #
 # This file is part of Audio Waveform Image Generator.
 #

diff --git a/src/AudioProcessor.h b/src/AudioProcessor.h
@@ -1,6 +1,6 @@
 //------------------------------------------------------------------------------
 //
-// Copyright 2013-2018 BBC Research and Development
+// Copyright 2013-2019 BBC Research and Development
 //
 // Author: Chris Needham
 //
@@ -38,6 +38,8 @@ class AudioProcessor
             int buffer_size
         ) = 0;
 
+        virtual bool shouldContinue() const = 0;
+
         virtual bool process(
             const short* input_buffer,
             int input_frame_count

diff --git a/src/DurationCalculator.cpp b/src/DurationCalculator.cpp
@@ -1,6 +1,6 @@
 //------------------------------------------------------------------------------
 //
-// Copyright 2018 BBC Research and Development
+// Copyright 2019 BBC Research and Development
 //
 // Author: Chris Needham
 //
@@ -38,8 +38,16 @@ bool DurationCalculator::init(int sample_rate, int /* channels */, long frame_co
     sample_rate_ = sample_rate;
     frame_count_ = frame_count;
 
-    // Only continue processing if we don't now know the length
-    return frame_count == 0;
+    return true;
+}
+
+//------------------------------------------------------------------------------
+
+bool DurationCalculator::shouldContinue() const
+{
+    // Only continue processing if we don't now know the length from the
+    // information passed to init()
+    return frame_count_ == 0;
 }
 
 //------------------------------------------------------------------------------

diff --git a/src/DurationCalculator.h b/src/DurationCalculator.h
@@ -1,6 +1,6 @@
 //------------------------------------------------------------------------------
 //
-// Copyright 2018 BBC Research and Development
+// Copyright 2019 BBC Research and Development
 //
 // Author: Chris Needham
 //
@@ -42,6 +42,8 @@ class DurationCalculator : public AudioProcessor
             int buffer_size
         );
 
+        virtual bool shouldContinue() const;
+
         virtual bool process(
             const short* input_buffer,
             int input_frame_count

diff --git a/src/FileUtil.cpp b/src/FileUtil.cpp
@@ -43,6 +43,20 @@ bool isStdioFilename(const char* filename)
 
 //------------------------------------------------------------------------------
 
+const char* getInputFilename(const char* filename)
+{
+    return FileUtil::isStdioFilename(filename) ? "(stdin)" : filename;
+}
+
+//------------------------------------------------------------------------------
+
+const char* getOutputFilename(const char* filename)
+{
+    return FileUtil::isStdioFilename(filename) ? "(stdout)" : filename;
+}
+
+//------------------------------------------------------------------------------
+
 } // namespace FileUtil
 
 //------------------------------------------------------------------------------
diff --git a/src/FileUtil.h b/src/FileUtil.h
@@ -28,6 +28,8 @@
 
 namespace FileUtil {
     bool isStdioFilename(const char* filename);
+    const char* getInputFilename(const char* filename);
+    const char* getOutputFilename(const char* filename);
 }
 
 //------------------------------------------------------------------------------

diff --git a/src/GdImageRenderer.cpp b/src/GdImageRenderer.cpp
@@ -470,16 +470,17 @@ bool GdImageRenderer::saveAsPng(
     else {
         output_file = fopen(filename, "wb");
 
-        if (output_file != nullptr) {
-            error_stream << "Writing PNG file: " << filename << '\n';
-        }
-        else {
-            error_stream << "Failed to write PNG file: " << filename << '\n'
-                    << strerror(errno) << '\n';
+        if (output_file == nullptr) {
+            error_stream << "Failed to write PNG file: "
+                         << filename << '\n'
+                         << strerror(errno) << '\n';
             return false;
         }
     }
 
+    error_stream << "Output file: "
+                 << FileUtil::getOutputFilename(filename) << '\n';
+
     gdImagePngEx(image_, output_file, compression_level);
 
     if (output_file != stdout) {

diff --git a/src/Mp3AudioFileReader.cpp b/src/Mp3AudioFileReader.cpp
@@ -360,15 +360,17 @@ bool Mp3AudioFileReader::open(const char* filename, bool show_info)
 
         if (!getFileSize()) {
             error_stream << "Failed to determine file size: " << filename << '\n'
-                         << strerror(errno);
+                         << strerror(errno) << '\n';
         }
 
-        error_stream << "Input file: " << filename << '\n';
     }
 
+    error_stream << "Input file: "
+                 << FileUtil::getInputFilename(filename) << '\n';
+
     if (!skipId3Tags()) {
-        error_stream << "Failed to read file: " << filename << '\n'
-                     << strerror(errno);
+        error_stream << "Failed to read file: "
+                     << strerror(errno) << '\n';
         return false;
     }
 
@@ -709,6 +711,11 @@ bool Mp3AudioFileReader::run(AudioProcessor& processor)
                 break;
             }
 
+            if (!processor.shouldContinue()) {
+                status = STATUS_PROCESS_ERROR;
+                break;
+            }
+
             showProgress(0, file_size_);
 
             started = true;
@@ -761,9 +768,7 @@ bool Mp3AudioFileReader::run(AudioProcessor& processor)
 
                 const int frames = OUTPUT_BUFFER_SIZE / channels;
 
-                bool success = processor.process(output_buffer, frames);
-
-                if (!success) {
+                if (!processor.process(output_buffer, frames)) {
                     status = STATUS_PROCESS_ERROR;
                     break;
                 }
@@ -779,9 +784,7 @@ bool Mp3AudioFileReader::run(AudioProcessor& processor)
     if (output_ptr != output_buffer && status != STATUS_PROCESS_ERROR) {
         int buffer_size = static_cast<int>(output_ptr - output_buffer);
 
-        bool success = processor.process(output_buffer, buffer_size / channels);
-
-        if (!success) {
+        if (!processor.process(output_buffer, buffer_size / channels)) {
             status = STATUS_PROCESS_ERROR;
         }
     }

diff --git a/src/OptionHandler.cpp b/src/OptionHandler.cpp
@@ -27,6 +27,7 @@
 #include "DurationCalculator.h"
 #include "Error.h"
 #include "FileFormat.h"
+#include "FileUtil.h"
 #include "GdImageRenderer.h"
 #include "Mp3AudioFileReader.h"
 #include "Options.h"
@@ -182,6 +183,12 @@ static std::pair<bool, double> getDuration(
 
     error_stream << "Duration: " << duration << " seconds\n";
 
+    if (FileUtil::isStdioFilename(input_filename.string().c_str())) {
+        if (fseek(stdin, 0, SEEK_SET) != 0) {
+            return std::make_pair(false, 0);
+        }
+    }
+
     return std::make_pair(true, duration);
 }
 
@@ -362,6 +369,7 @@ bool OptionHandler::renderWaveformImage(
             auto result = getDuration(input_filename, input_format);
 
             if (!result.first) {
+                error_stream << "Failed to get audio duration\n";
                 return false;
             }
 
@@ -513,8 +521,11 @@ bool OptionHandler::run(const Options& options)
             );
         }
         else {
-            error_stream << "Can't generate " << output_filename
-                         << " from " << input_filename << '\n';
+            error_stream << "Can't generate "
+                         << FileFormat::toString(output_format)
+                         << " format output from "
+                         << FileFormat::toString(input_format)
+                         << " format input\n";
             success = false;
         }
     }

diff --git a/src/SndFileAudioFileReader.cpp b/src/SndFileAudioFileReader.cpp
@@ -69,14 +69,11 @@ bool SndFileAudioFileReader::open(const char* input_filename, bool show_info)
     if (FileUtil::isStdioFilename(input_filename)) {
         input_file_ = sf_open_fd(fileno(stdin), SFM_READ, &info_, 0);
 
-        if (input_file_ != nullptr) {
-            if (show_info) {
-                showInfo(error_stream, info_);
-            }
-        }
-        else {
+        if (input_file_ == nullptr) {
             error_stream << "Failed to read input: "
                          << sf_strerror(nullptr) << '\n';
+
+            return false;
         }
     }
     else {
@@ -85,17 +82,19 @@ bool SndFileAudioFileReader::open(const char* input_filename, bool show_info)
         if (input_file_ == nullptr) {
             error_stream << "Failed to read file: " << input_filename << '\n'
                          << sf_strerror(nullptr) << '\n';
-        }
-        else {
-            error_stream << "Input file: " << input_filename << '\n';
 
-            if (show_info) {
-                showInfo(error_stream, info_);
-            }
+            return false;
         }
     }
 
-    return input_file_ != nullptr;
+    error_stream << "Input file: "
+                 << FileUtil::getInputFilename(input_filename) << '\n';
+
+    if (show_info) {
+        showInfo(error_stream, info_);
+    }
+
+    return true;
 }
 
 //------------------------------------------------------------------------------
@@ -135,7 +134,7 @@ bool SndFileAudioFileReader::run(AudioProcessor& processor)
 
     success = processor.init(info_.samplerate, info_.channels, info_.frames, BUFFER_SIZE);
 
-    if (success) {
+    if (success && processor.shouldContinue()) {
         showProgress(0, info_.frames);
 
         while (success && frames_read == frames_to_read) {

diff --git a/src/WavFileWriter.cpp b/src/WavFileWriter.cpp
@@ -22,11 +22,13 @@
 //------------------------------------------------------------------------------
 
 #include "WavFileWriter.h"
+#include "FileUtil.h"
 #include "Streams.h"
 
 #include <cassert>
 #include <cstring>
 #include <iostream>
+#include <unistd.h>
 
 //------------------------------------------------------------------------------
 
@@ -53,7 +55,8 @@ bool WavFileWriter::init(
     const long /* frame_count */,
     const int buffer_size)
 {
-    error_stream << "Output file: " << output_filename_ << '\n';
+    error_stream << "Output file: "
+                 << FileUtil::getOutputFilename(output_filename_.c_str()) << '\n';
 
     channels_    = channels;
     buffer_size_ = buffer_size;
@@ -67,7 +70,19 @@ bool WavFileWriter::init(
     info.channels   = channels;
     info.format     = SF_FORMAT_WAV | SF_FORMAT_PCM_16;
 
-    output_file_ = sf_open(output_filename_.c_str(), SFM_WRITE, &info);
+    if (FileUtil::isStdioFilename(output_filename_.c_str())) {
+        // Prevent illegal seek error
+        if (isatty(fileno(stdout))) {
+            error_stream << "Cannot write WAV audio to the terminal\n";
+            return false;
+        }
+        else {
+            output_file_ = sf_open_fd(fileno(stdout), SFM_WRITE, &info, 0);
+        }
+    }
+    else {
+        output_file_ = sf_open(output_filename_.c_str(), SFM_WRITE, &info);
+    }
 
     if (output_file_ == nullptr) {
         error_stream << sf_strerror(output_file_) << '\n';
@@ -78,6 +93,13 @@ bool WavFileWriter::init(
 
 //------------------------------------------------------------------------------
 
+bool WavFileWriter::shouldContinue() const
+{
+    return true;
+}
+
+//------------------------------------------------------------------------------
+
 void WavFileWriter::close()
 {
     if (output_file_ != nullptr) {

diff --git a/src/WavFileWriter.h b/src/WavFileWriter.h
@@ -1,6 +1,6 @@
 //------------------------------------------------------------------------------
 //
-// Copyright 2013-2018 BBC Research and Development
+// Copyright 2013-2019 BBC Research and Development
 //
 // Author: Chris Needham
 //
@@ -52,6 +52,8 @@ class WavFileWriter : public AudioProcessor
             int buffer_size
         );
 
+        virtual bool shouldContinue() const;
+
         virtual bool process(
             const short* input_buffer,
             int input_frame_count