* Bug 15981 fixed [sound]: wavread() locked .wav on error, etc 88/20888/6
Samuel GOUGEON [Tue, 26 Feb 2019 19:55:44 +0000 (20:55 +0100)]
  http://bugzilla.scilab.org/15981

  * Opened wav file was kept open and locked when returning on error.
  * The file extension and existence were poorly tested.
  * Some errors messages referred to find_cktype() instead of to the
    current macro
  * Some wav formats were claimed as invalid instead of as unsupported:
    The supported wav formats were not indicated.

  The wavread() page is overhauled when processing the bug 15977:
    with indications about supported wav formats:
    https://codereview.scilab.org/#/c/20914/

  See also the same work for auread() and auwrite():
    https://codereview.scilab.org/21031

Change-Id: Ib51ff10799654767a61dd8ff7a3fba8516747e3a

scilab/CHANGES.md
scilab/modules/sound/macros/wavread.sci
scilab/modules/sound/tests/nonreg_tests/bug_15981.tst [new file with mode: 0644]
scilab/modules/sound/tests/nonreg_tests/mu_law_format_7.wav [new file with mode: 0644]

index 29b62bf..163bbd3 100644 (file)
@@ -254,6 +254,7 @@ Bug Fixes
 * [#15523](http://bugzilla.scilab.org/show_bug.cgi?id=15523): `%ODEOPTIONS(1)=2` didn't work with solvers 'rk' and 'rkf'
 * [#15577](http://bugzilla.scilab.org/show_bug.cgi?id=15577): `edit` did not accept a line number as text, as with `edit linspace 21`.
 * [#15580](http://bugzilla.scilab.org/show_bug.cgi?id=15580): `det(sparse([],[]))` yielded an error.
+* [#15981](http://bugzilla.scilab.org/show_bug.cgi?id=15981): `wavread()` kept the wav file open and locked when returning on errors. It weakly managed the input file name. It claimed for invalid data formats instead of unsupported ones, with poor information about the current format vs the supported ones. Several error messages refered to a wrong function.
 * [#15595](http://bugzilla.scilab.org/show_bug.cgi?id=15595): `unique()` was not able to return distinct values in their original order, without sorting them. A `keepOrder` option now allows it.
 * [#15668](http://bugzilla.scilab.org/show_bug.cgi?id=15668): `save(filename)` saved all predefined Scilab constants %e %pi etc.. (regression)
 * [#15715](http://bugzilla.scilab.org/show_bug.cgi?id=15715): `%nan` indices crashed Scilab.
index 173e947..c69cb51 100644 (file)
@@ -2,8 +2,8 @@
 // Copyright (C) ???? - INRIA - Scilab
 // Copyright (C) ???? - ENPC
 // Copyright (C) 2008-2011 - DIGITEO - Allan CORNET
-//
 // Copyright (C) 2012 - 2016 - Scilab Enterprises
+// Copyright (C) 2019 - Samuel GOUGEON
 //
 // This file is hereby licensed under the terms of the GNU GPL v2.0,
 // pursuant to article 5.3.4 of the CeCILL v.2.1.
@@ -41,17 +41,14 @@ function [y, Fs, bits] = wavread(wavfile, ext)
     //                 nBitsPerSample, cbSize, nChannels, samples]
 
 
+    // INPUT ARGUMENTS CHECKING
+    // ========================
     // Append .wav extension if necessary
-    if ( strindex(wavfile, ".") == [] ) then
+    tmp = fileparts(wavfile,"extension")
+    if tmp=="" | (~isfile(wavfile) & tmp<>".wav")
         wavfile = wavfile + ".wav";
     end
 
-    // Open the file
-    [fid, err] = mopen(wavfile, "rb", 1);    // Little-endian
-    if (err < 0) then
-        error(msprintf(gettext("%s: Cannot open file %s.\n"), "wavread", wavfile));
-    end
-
     // Handle ext optional argument
     if (argn(2) < 2) then
         ext = [];
@@ -60,12 +57,14 @@ function [y, Fs, bits] = wavread(wavfile, ext)
     if (type(ext) == 10) then
         ext = convstr(ext);
         if (ext <> "size") & (ext <> "info") then
-            error(msprintf(gettext("%s: Wrong value for input argument #%d: Must be ""%s"" or ""%s"", an integer or a vector of %d integers.\n"), "wavread", 2, "size", "info", 2));
+            msg = gettext("%s: Wrong value for input argument #%d: Must be ""%s"" or ""%s"", an integer or a vector of %d integers.\n")
+            error(msprintf(msg, "wavread", 2, "size", "info", 2));
         end
     elseif (type(ext) == 1) then
         exts = size(ext, "*");
         if (exts > 2) then
-            error(msprintf(gettext("%s: Wrong value for input argument: Index range must be specified as a scalar or %d-element vector.\n"), "wavread", 2));
+            msg = gettext("%s: Wrong value for input argument: Index range must be specified as a scalar or %d-element vector.\n")
+            error(msprintf(msg, "wavread", 2));
         end
         if (exts == 1) then
             if (ext == 0) then
@@ -75,22 +74,41 @@ function [y, Fs, bits] = wavread(wavfile, ext)
             end
         end
     else
-        error(msprintf(gettext("%s: Wrong value for input argument #%d: Must be ""%s"" or ""%s"", an integer or a vector of %d integers.\n"), "wavread", 2, "size", "info", 2));
+        msg = gettext("%s: Wrong value for input argument #%d: Must be ""%s"" or ""%s"", an integer or a vector of %d integers.\n")
+        error(msprintf(msg, "wavread", 2, "size", "info", 2));
     end
 
+    // Open the file
+    // -------------
+    if ~isfile(wavfile) then
+        msg = _("%s: The file ''%s'' does not exist.\n")
+        error(msprintf(msg, "wavread", wavfile));
+    end
+    [fid, err] = mopen(wavfile, "rb", 1);    // Little-endian
+    if (err < 0) then
+        error(msprintf(gettext("%s: Cannot open file %s.\n"), "wavread", wavfile));
+    end
+
+    // Checking for the RIFF and wav identifiers
+    // -----------------------------------------
     Data = [];
     ID = stripblanks(ascii(mget(4, "c", fid)));
     Size = mget(1, "ui", fid);
     if (convstr(ID) ~= "riff") then
-        error(msprintf(gettext("%s: An error occurred: %s\n"), "wavread", gettext(".wav file does not contain the RIFF identifier.")));
+        mclose(fid)
+        msg2 = gettext(".wav file does not contain the RIFF identifier.")
+        error(msprintf(gettext("%s: An error occurred: %s\n"), "wavread", msg2));
     end
     rifftype = mget(4, "c", fid);
     dtype = convstr(ascii(rifftype)');
     if (dtype ~= "wave") then
-        error(msprintf(gettext("%s: An error occurred: %s\n"), "wavread", gettext(".wav file does not contain the wave identifier.")));
+        mclose(fid)
+        msg2 = gettext(".wav file does not contain the wave identifier.")
+        error(msprintf(gettext("%s: An error occurred: %s\n"), "wavread", msg2));
     end
 
     // Find optional chunks
+    // --------------------
     found_fmt = 0;
     found_data = 0;
     while ~found_data then
@@ -102,26 +120,32 @@ function [y, Fs, bits] = wavread(wavfile, ext)
             nbytes = 4;
             // # of required bytes in <fact-ck> header
             if total_bytes < nbytes then
-                error(msprintf(gettext("%s: An error occurred: %s\n"), "wavread", gettext("Error reading .wav file.")));
+                mclose(fid)
+                msg2 = gettext("Error reading .wav file.")
+                error(msprintf(gettext("%s: An error occurred: %s\n"), "wavread", msg2));
             end
             factdata = mget(1, "ui", fid); // Samples per second
             rbytes = total_bytes - (mtell(fid) - orig_pos);
             if rbytes then
                 mseek(rbytes, fid, "cur");
                 if (merror(fid) <> 0) then
-                    error(msprintf(gettext("%s: An error occurred: %s\n"), "wavread", gettext("Error reading <fact-ck> chunk.")));
+                    mclose(fid)
+                    msg2 = gettext("Error reading <fact-ck> chunk.")
+                    error(msprintf(gettext("%s: An error occurred: %s\n"), "wavread", msg2));
                 end
             end
 
             // bug 4037
         case "bext" then
-            error(msprintf(gettext("%s: An error occurred: %s is not supported.\n"), "wavread", "Broadcast Wave Format"));
-            return
+            mclose(fid)
+            msg = gettext("%s: An error occurred: %s is not supported.\n")
+            error(msprintf(msg, "wavread", "Broadcast Wave Format"));
 
             // bug 4832 - Sampler Chunk
         case "smpl" then
-            error(msprintf(gettext("%s: An error occurred: invalid file format. Error reading <%s> chunk.\n"), "wavread", ID));
-            return
+            mclose(fid)
+            msg = gettext("%s: An error occurred: invalid file format. Error reading <%s> chunk.\n")
+            error(msprintf(msg, "wavread", ID));
 
         case "fmt" then
             found_fmt = 1;
@@ -131,12 +155,13 @@ function [y, Fs, bits] = wavread(wavfile, ext)
 
             found_data = 1;
             if ~found_fmt then
-                error(msprintf(gettext("%s: An error occurred: %s\n"), "wavread", gettext("Invalid .wav file: found data before format information.")));
+                mclose(fid)
+                msg2 = gettext("Invalid .wav file: found data before format information.")
+                error(msprintf(gettext("%s: An error occurred: %s\n"), "wavread", msg2));
             end
             if (ext == "size") | (ext == "info") | (~(ext == [])) & and(ext == 0) then
                 // Caller just wants data size:
                 samples = read_wavedat(fid, Size ,wFormatTag, nChannels, nBitsPerSample, -1);
-                mclose(fid);
                 if (ext == "info") then
                     y = [wFormatTag, nChannels, nSamplesPerSec, nAvgBytesPerSec, nBlockAlign, nBitsPerSample, cbSize, nChannels, samples];
                 else // "size"
@@ -144,13 +169,15 @@ function [y, Fs, bits] = wavread(wavfile, ext)
                 end
             else
                 y = read_wavedat(fid, Size ,wFormatTag, nChannels, nBitsPerSample, ext);
-                mclose(fid);
             end
+            mclose(fid);
 
         else
             mseek(Size, fid, "cur")
             if (merror(fid) <> 0) then
-                error(msprintf(gettext("%s: An error occurred: %s\n"), "wavread", gettext("Incorrect chunk size information in RIFF file.")))
+                mclose(fid);
+                msg2 = gettext("Incorrect chunk size information in RIFF file.")
+                error(msprintf(gettext("%s: An error occurred: %s\n"), "wavread", msg2))
             end
         end
     end
@@ -173,7 +200,9 @@ function [wFormatTag, nChannels, nSamplesPerSec, nAvgBytesPerSec, nBlockAlign, n
     nbytes = 14; // # of required bytes in  header
 
     if total_bytes < nbytes then
-        error(msprintf(gettext("%s: An error occurred: %s\n"), "read_wavefmt", gettext("Error reading .wav file.")));
+        mclose(fid)
+        msg2 = gettext("Error reading .wav file.")
+        error(msprintf(gettext("%s: An error occurred: %s\n"), "read_wavefmt", msg2));
     end
 
     // Read wav data:
@@ -182,8 +211,11 @@ function [wFormatTag, nChannels, nSamplesPerSec, nAvgBytesPerSec, nBlockAlign, n
     nSamplesPerSec = mget(1, "ui", fid); // Samples per second
     nAvgBytesPerSec = mget(1, "ui", fid); // Avg transfer rate
     nBlockAlign = mget(1, "us", fid); // Block alignment
-    if (and(wFormatTag ~= [1 3])) then
-        error(msprintf(gettext("%s: An error occurred: %s\n"), "find_cktype", gettext("Invalid wav format.")));
+    supportedFormats = [1 3];         // codes of supported wav formats
+    if (and(wFormatTag ~= supportedFormats)) then
+        mclose(fid)
+        msg = gettext("%s: wav format #%d detected. Only wav formats #%s are supported.\n")
+        error(msprintf(msg, "read_wavefmt", wFormatTag, strcat(msprintf("%d\n",supportedFormats'),"|")));
     else
         [cbSize, nBitsPerSample] = read_fmt_pcm(fid, total_bytes);
     end
@@ -192,7 +224,9 @@ function [wFormatTag, nChannels, nSamplesPerSec, nAvgBytesPerSec, nBlockAlign, n
     if rbytes then
         mseek(rbytes, fid, "cur")
         if (merror(fid) <> 0) then
-            error(msprintf(gettext("%s: An error occurred: %s\n"), "read_wavefmt", gettext("Error reading .wav file.")));
+            mclose(fid)
+            msg2 = gettext("Error reading .wav file.")
+            error(msprintf(gettext("%s: An error occurred: %s\n"), "read_wavefmt", msg2));
         end
     end
 endfunction
@@ -203,7 +237,9 @@ function [cbSize, nBitsPerSample] = read_fmt_pcm(fid, total_bytes)
     nBitsPerSample = [];
     // # of bytes already read
     if (total_bytes < nbytes + 2) then
-        error(msprintf(gettext("%s: An error occurred: %s\n"), "find_cktype", gettext("Error reading wav file.")));
+        mclose(fid)
+        msg2 = gettext("Error reading wav file.")
+        error(msprintf(gettext("%s: An error occurred: %s\n"), "read_fmt_pcm", msg2));
     end
     nBitsPerSample = mget(1, "us", fid);
     nbytes = nbytes + 2;
@@ -215,7 +251,9 @@ function [cbSize, nBitsPerSample] = read_fmt_pcm(fid, total_bytes)
         if (total_bytes > nbytes) then
             mseek(total_bytes - nbytes, fid, "cur")
             if (merror(fid) <> 0) then
-                error(msprintf(gettext("%s: An error occurred: %s\n"), "find_cktype", gettext("Error reading wav file.")));
+                mclose(fid)
+                msg2 = gettext("Error reading wav file.")
+                error(msprintf(gettext("%s: An error occurred: %s\n"), "read_fmt_pcm", msg2));
             end
         end
     end
@@ -254,11 +292,13 @@ function Data = read_wavedat(fid, Size, wFormatTag, nChannels, nBitsPerSample, e
         fmt_msg = "Format #" + string(wFormatTag);
     end
     if ~(fmt_msg == []) then
-        error(msprintf(gettext("%s: An error occurred: Data compression format %s is not supported.\n"), "read_wavedat", fmt_msg));
+        mclose(fid)
+        msg = gettext("%s: An error occurred: Data compression format %s is not supported.\n")
+        error(msprintf(msg, "read_wavedat", fmt_msg));
     end
 endfunction
 // =============================================================================
-function Data = read_dat_pcm(fid,total_bytes , nChannels, nBitsPerSample, ext, wFormatTag)
+function Data = read_dat_pcm(fid, total_bytes , nChannels, nBitsPerSample, ext, wFormatTag)
     // Determine # bytes/sample - format requires rounding
     //  to next integer number of bytes:
     BytesPerSample = ceil(nBitsPerSample / 8);
@@ -277,7 +317,9 @@ function Data = read_dat_pcm(fid,total_bytes , nChannels, nBitsPerSample, ext, w
             dtype = "i";
         end
     else
-        error(msprintf(gettext("%s: An error occurred: %s\n"), "read_dat_pcm", gettext("Cannot read .wav file  with more than 16 bits per sample.")));
+        mclose(fid)
+        msg2 = gettext("Cannot read .wav file  with more than 16 bits per sample.")
+        error(msprintf(gettext("%s: An error occurred: %s\n"), "read_dat_pcm", msg2));
     end//  select BytesPerSample
 
     // # bytes in this chunk
@@ -293,14 +335,19 @@ function Data = read_dat_pcm(fid,total_bytes , nChannels, nBitsPerSample, ext, w
         ext = [1, SamplesPerChannel];
     else
         if (prod(size(ext)) ~= 2) then
-            error(msprintf(gettext("%s: An error occurred: %s\n"), "read_dat_pcm", gettext("Sample limit vector must have 2 entries.")));
-            return
+            mclose(fid)
+            msg2 = gettext("Sample limit vector must have 2 entries.")
+            error(msprintf(gettext("%s: An error occurred: %s\n"), "read_dat_pcm", msg2));
         end
         if (ext(1) < 1) | (ext(2) > SamplesPerChannel) then
-            error(msprintf(gettext("%s: An error occurred: %s\n"), "read_dat_pcm", gettext("Sample limits out of range.")));
+            mclose(fid)
+            msg2 = gettext("Sample limits out of range.")
+            error(msprintf(gettext("%s: An error occurred: %s\n"), "read_dat_pcm", msg2));
         end
         if (ext(1) > ext(2)) then
-            error(msprintf(gettext("%s: An error occurred: %s\n"), "read_dat_pcm", gettext("Invalid sample limits (use ascending order).")));
+            mclose(fid)
+            msg2 = gettext("Invalid sample limits (use ascending order).")
+            error(msprintf(gettext("%s: An error occurred: %s\n"), "read_dat_pcm", msg2));
         end
     end
 
diff --git a/scilab/modules/sound/tests/nonreg_tests/bug_15981.tst b/scilab/modules/sound/tests/nonreg_tests/bug_15981.tst
new file mode 100644 (file)
index 0000000..68e9c73
--- /dev/null
@@ -0,0 +1,44 @@
+// =============================================================================
+// Scilab ( http://www.scilab.org/ ) - This file is part of Scilab
+// Copyright (C) 2019 - Samuel GOUGEON
+// Copyright (C) 2019 - Federico MIYARA      // mu-law sample file
+//
+//  This file is distributed under the same license as the Scilab package.
+// =============================================================================
+//
+// <-- CLI SHELL MODE -->
+// <-- NO CHECK REF -->
+// <-- ENGLISH IMPOSED -->
+//
+// <-- Non-regression test for bug 15981 -->
+//
+// <-- Bugzilla URL -->
+// https://bugzilla.scilab.org/15981
+//
+// <-- Short Description -->
+// wavread()
+//  - weakly managed the input filename (extension, existence).
+//  - never closed the wav opened file before emitting any error and quitting
+//    wavread(), leaving the file locked.
+//  - yielded some errors messages referring to find_cktype() instead of to the current macro
+//  - claimed that some wav formats are invalid instead of are unsupported.
+//    The supported wav formats were not indicated in the error message.
+
+// Input file name management
+// --------------------------
+File = SCI+"/modules/sound/demos/chimes";  // should be completed to chimes.wav
+assert_checktrue(execstr("wavread(File)", "errcatch")==0);
+
+File2 = TMPDIR+"/chimes.test"         // should be completed to chimes.test.wav
+copyfile(File+".wav", File2);
+assert_checktrue(execstr("wavread(File2)", "errcatch")==0);
+
+copyfile(File+".wav", File2+".wav");  // should be read as is = chimes.test.wav
+assert_checktrue(execstr("wavread(File2)", "errcatch")==0);
+
+// Management of unsupported wav format
+// ------------------------------------
+File = "SCI/modules/sound/tests/nonreg_tests/mu_law_format_7.wav";
+refmsg = "read_wavefmt: wav format #7 detected. Only wav formats #1|3 are supported."
+assert_checkerror("wavread(File)", refmsg);
+
diff --git a/scilab/modules/sound/tests/nonreg_tests/mu_law_format_7.wav b/scilab/modules/sound/tests/nonreg_tests/mu_law_format_7.wav
new file mode 100644 (file)
index 0000000..89e71da
Binary files /dev/null and b/scilab/modules/sound/tests/nonreg_tests/mu_law_format_7.wav differ