tuwien-musicir / rp_extract

Rhythm Pattern music feature extractor by IFS @ TU-Vienna
GNU General Public License v3.0
111 stars 27 forks source link

temp files of audiofile_read.py are not deleted #12

Closed slychief closed 9 years ago

audiofeature commented 9 years ago

on Linux (and most likely Mac) they are deleted. Maybe its a Windows problem. Could be also related to FFMPEG: temp = tempfile.NamedTemporaryFile(suffix='.wav') opens (creates) the file already. So we had to add "-y" to the ffmpeg command to overwrite the file.

    finally:
        # Automatically cleans up (deletes) the temp file
        temp.close()

should delete the file (works in Linux/Mac). But maybe because ffmpeg overwrote the file it took it over and prevents it from being deleted.

Please double check if its a Windows or ffmepg problem (by using lame or mpg123 on windows). You could insert a particular delete in the finally part for this case (or do a generic if temp.file exists: delete after temp.close())

slychief commented 9 years ago

Don’t know if it’s only a windows problem, but I guess the problems is the same on other platforms. The workflow seems to be problematic.

The following output trace describes the problem:

  1. The temp-file is created
  2. Enter loop
  3. Try mpg123 => does not exist => Exception
  4. Finally-block closes temp-file => delete file => OK
  5. Next iteration (now with closed tempfile)
  6. Try ffmpeg => ffmpeg available, but mp3 corrupted => Exception
  7. Finally-block closes closed tempfile => tempfile exists!!! But status closed
>>> open tempfile
    filename: c:\users\schind~1\appdata\local\temp\tmpagplmo.wav
    status tempfile ', mode 'w+b' at 0x0000000003DB4B70>
---
>>> processing:  ['mpg123', '-q', '-w', 'c:\\users\\schind~1\\appdata\\local\\temp\\tmpagplmo.wav', 'E:/Data/MIR/EU_SOUNDS/2023601/oai_eu_dismarc_2GSR_371055160279.mp3']
    finally: closing tempfile
    os.path.exists(temp.name):  False
    status tempfile ', mode 'w+b' at 0x0000000003DB4B70>
---
>>> processing:  ['ffmpeg', '-y', '-v', '1', '-i', 'E:/Data/MIR/EU_SOUNDS/2023601/oai_eu_dismarc_2GSR_371055160279.mp3', 'c:\\users\\schind~1\\appdata\\local\\temp\\tmpagplmo.wav']
    finally: closing tempfile
    os.path.exists(temp.name):  True
    status tempfile ', mode 'w+b' at 0x0000000003DB4B70>
Traceback (most recent call last):
  File "C:\Work\IFS\rp_extract\audiofile_read.py", line 161, in 
    samplerate, samplewidth, wavedata = audiofile_read(file)
  File "C:\Work\IFS\rp_extract\audiofile_read.py", line 143, in audiofile_read
    return(mp3_read(filename,normalize))
  File "C:\Work\IFS\rp_extract\audiofile_read.py", line 96, in mp3_read
    raise DecoderException("Problem appeared during decoding.", command=cmd)
__main__.DecoderException: Problem appeared during decoding.

ps: sorry, ich bekomm die formatierung nicht hin.

audiofeature commented 9 years ago

I see. You are right. The temp.close() needs to be moved just before return. (no need for the finally block).

slychief commented 9 years ago

Done. Added try-finally without catch around loop

slychief commented 9 years ago

Further problem!

It only worked, because the first command failed, so that the temp-file was closed!

Temp files are generated and automatically opened, thus the file is locked and cannot be accessed by other processess. Because mpg123 is the first command, which usually nobody is using, the first command failed and the temp-file was closed. Yet, the filename of the deleted temp-file was still in the command-list handed over to ffmpeg which created it on its own.

I will implement a method that only creates temp-filenames without opening filehandles.

And that's also the reason why temp.close() did not delete the file!

slychief commented 9 years ago

added custom temp-file handling. solved problem