tuffy / python-audio-tools

A collection of audio handling programs which work from the command line.
http://audiotools.sourceforge.net
GNU General Public License v2.0
249 stars 58 forks source link

Vorbis.convert() - unable to encode file with oggenc | from_pcm() sub.wait returns -11 #10

Open kiddhustle opened 11 years ago

kiddhustle commented 11 years ago

A strange error occurred ( unable to encode file with oggenc ) when running some tests converting a FLAC file to the following formats:

.mp3/libmp3 .m4a/neroaac .ogga/vorbis

All formats work apart from the OGG/Vorbis format which fails.

I searched the source code for PAT and found that inside the Vorbis.from_pcm() method (Vorbis.py: line 352) the code is expecting sub.wait() == 0 whilst it is equal to -11.

Searching for oggenc return codes, I found that minus codes indicate that the process has been terminated, but not sure why that might be.

I have all of the binaries installed for all formats so OGG/Vorbis should work.

Any ideas what could be causing this??

kiddhustle commented 11 years ago

error occurs on line: 360 in the master development branch (line 352 in 2.19 release)

tuffy commented 11 years ago

What happens when you try to run oggenc directly, such as on a .wav file using "oggenc file.wav" ? If you're able to run it successfully, I may need to adjust the arguments I'm sending it, but if it fails when run directly there may be some issue with your oggenc install.

kiddhustle commented 11 years ago

Hey Tuffy

Yep, I tried running oggenc directly and the encoding works fine. I tried using both FLAC and .wav files as the source and both source formats work.

Here's the commands that I tried (different quality settings):

oggenc 00_audio.wav -q -1 -o 00_audio-1.ogg oggenc 00_audio.wav -q 0 -o 00_audio0.ogg oggenc 00_audio.wav -q 1 -o 00_audio1.ogg oggenc 00_audio.wav -q 2 -o 00_audio2.ogg oggenc 00_audio.wav -q 3 -o 00_audio3.ogg oggenc 00_audio.wav -q 4 -o 00_audio4.ogg oggenc 00_audio.wav -q 5 -o 00_audio5.ogg

Additional shorter-hand forms works:

oggenc 00_audio.flac -q -1 oggenc 00_audio.flac

Also oggenc was installed via vorbis-tools in Ubuntu 12.10

Hope this helps.

tuffy commented 11 years ago

That is interesting. One of my testing machines is Ubuntu 12.04 which hasn't exhibited any problems and is running vorbis-tools-1.4.0-1ubuntu2. I'm going to install 12.10 and see if I can duplicate the problem on my end.

tuffy commented 11 years ago

After installing Ubuntu 12.10, I'm getting the same error you are when attempting to encode from oggenc using data from stdin. It's a segmentation fault in vorbis-tools-1.4.0-1ubuntu3 package that doesn't occur in ubuntu2 or when encoding from a source file. After installing vorbis-tools from source and running it through valgrind, it looks like there's a bug on line 449 of oggenc.c during a file close operation that's causing it to crash, but it'll take more digging to find the whole cause.

In the meantime, I could reimplement my from_pcm() method to use temp files if you need that functionality right away, but I may have to punt a long-term oggenc fix to the Ubuntu people.

kiddhustle commented 11 years ago

Ahh OK thanks for finding the source of the problem. I only upgraded to Ubuntu 12.10 recently too so that I could get the Opus encoder/decoder from the repository. I was assuming that everything will would work OK seeing at that version was released a few months ago and any problems would be found and fixed by now.. Guess not.

I did want to use OGG enc for some bulk transcoding in a web project, is your plan to re-implement from_pcm() quite a trivial bit of work?

Out of interest how would your short-term solution work? Would you convert the source file to another file format before sending it to oggenc or is the segmentation something separate from the source file format itself?

Do you think me removing vorbis-tools-1.4.0-1ubuntu3 and installing the previous version would work if that's possible in 12.10?

tuffy commented 11 years ago

I'm not sure installing vorbis-tools from a previous version would help since installing them from source has the same error. There's either a bug in some library oggenc is using under Ubuntu 12.10, or it has some longstanding bug that just hasn't happened to be fatal until now.

The short-term solution works by decoding to a temporary .wav file on disk, then running oggenc on that file before deleting it. Its progress display will look funny, but it should work okay. The patch looks like:

diff --git a/audiotools/vorbis.py b/audiotools/vorbis.py
index 7ed219e..8b6948b 100644
--- a/audiotools/vorbis.py
+++ b/audiotools/vorbis.py
@@ -256,100 +256,37 @@ class VorbisAudio(AudioFile):
     def from_pcm(cls, filename, pcmreader, compression=None):
         """returns a PCMReader object containing the track's PCM data"""

-        from . import transfer_framelist_data
         from . import BIN
-        from . import ignore_sigint
         from . import EncodingError
-        from . import DecodingError
-        from . import UnsupportedChannelMask
         from . import __default_quality__
+        from . import WaveAudio
         import subprocess
         import os
+        import tempfile

         if (((compression is None) or
              (compression not in cls.COMPRESSION_MODES))):
             compression = __default_quality__(cls.NAME)

         devnull = file(os.devnull, 'ab')
-
-        sub = subprocess.Popen([BIN['oggenc'], '-Q',
-                                '-r',
-                                '-B', str(pcmreader.bits_per_sample),
-                                '-C', str(pcmreader.channels),
-                                '-R', str(pcmreader.sample_rate),
-                                '--raw-endianness', str(0),
-                                '-q', compression,
-                                '-o', filename, '-'],
-                               stdin=subprocess.PIPE,
-                               stdout=devnull,
-                               stderr=devnull,
-                               preexec_fn=ignore_sigint)
-
-        if ((pcmreader.channels <= 2) or (int(pcmreader.channel_mask) == 0)):
-            try:
-                transfer_framelist_data(pcmreader, sub.stdin.write)
-            except (IOError, ValueError), err:
-                sub.stdin.close()
-                sub.wait()
-                cls.__unlink__(filename)
-                raise EncodingError(str(err))
-            except Exception, err:
-                sub.stdin.close()
-                sub.wait()
-                cls.__unlink__(filename)
-                raise err
-
-        elif (pcmreader.channels <= 8):
-            if (int(pcmreader.channel_mask) in
-                (0x7,      # FR, FC, FL
-                 0x33,     # FR, FL, BR, BL
-                 0x37,     # FR, FC, FL, BL, BR
-                 0x3f,     # FR, FC, FL, BL, BR, LFE
-                 0x70f,    # FL, FC, FR, SL, SR, BC, LFE
-                 0x63f)):  # FL, FC, FR, SL, SR, BL, BR, LFE
-
-                standard_channel_mask = ChannelMask(pcmreader.channel_mask)
-                vorbis_channel_mask = VorbisChannelMask(standard_channel_mask)
-            else:
-                raise UnsupportedChannelMask(filename,
-                                             int(pcmreader.channel_mask))
-
-            try:
-                from . import ReorderedPCMReader
-
-                transfer_framelist_data(
-                    ReorderedPCMReader(
-                        pcmreader,
-                        [standard_channel_mask.channels().index(channel)
-                         for channel in vorbis_channel_mask.channels()]),
-                    sub.stdin.write)
-            except (IOError, ValueError), err:
-                sub.stdin.close()
-                sub.wait()
-                cls.__unlink__(filename)
-                raise EncodingError(str(err))
-            except Exception, err:
-                sub.stdin.close()
-                sub.wait()
-                cls.__unlink__(filename)
-                raise err
-
-        else:
-            raise UnsupportedChannelMask(filename,
-                                         int(pcmreader.channel_mask))
-
+        tempwavefile = tempfile.NamedTemporaryFile(suffix=".wav")
         try:
-            pcmreader.close()
-        except DecodingError, err:
-            raise EncodingError(err.error_message)
+            tempwave = WaveAudio.from_pcm(tempwavefile.name, pcmreader)

-        sub.stdin.close()
-        devnull.close()
+            sub = subprocess.Popen([BIN["oggenc"], "-Q",
+                                    "-q", compression,
+                                    "-o", filename,
+                                    tempwavefile.name],
+                                   stdout=devnull,
+                                   stderr=devnull)
+            devnull.close()

-        if (sub.wait() == 0):
-            return VorbisAudio(filename)
-        else:
-            raise EncodingError(u"unable to encode file with oggenc")
+            if (sub.wait() == 0):
+                return VorbisAudio(filename)
+            else:
+                raise EncodingError(u"unable to encode file with oggenc")
+        finally:
+            tempwavefile.close()

     def update_metadata(self, metadata):
         """takes this track's current MetaData object
kiddhustle commented 11 years ago

OK cool thanks. Will try that out and let you know how it goes.

Also I've filed a bug in the Ubuntu repository. Not sure how quickly they respond to bug reports mind..

https://bugs.launchpad.net/ubuntu/+source/vorbis-tools/+bug/1096037

kiddhustle commented 11 years ago

just tried this patch and i get this error:

error: patch failed: audiotools/vorbis.py:256 error: audiotools/vorbis.py: patch does not apply

I tried this with the latest code in on the master branch. Do I need to use another branch at all?

tuffy commented 11 years ago

The patch was made against v2.19 commit 2cca95d4fc99c9ec427a0d7f6444b581c982e677. I was able to patch it okay from the audiotools root directory via "patch -p1 < vorbis.patch"

kiddhustle commented 11 years ago

Aah I was using git apply rather than patch..

Yep, thanks that worked a treat. Vorbis encoding is working as expected. Cheers!