jiaaro / pydub

Manipulate audio with a simple and easy high level interface
http://pydub.com
MIT License
8.84k stars 1.04k forks source link

Both AudioSegment.from_file and mediainfo_json Close the File Provided to Them #333

Open nathanielatom opened 5 years ago

nathanielatom commented 5 years ago

Steps to reproduce

from pydub import AudioSegment
from pydub.utils import mediainfo_json
filename = '04 The Mary Ellen Carter (Live).m4a' # any file
with open(filename, 'rb') as disk_file: # let's say this is expensive
    # each of these closes the file
    audio = AudioSegment.from_file(disk_file, format='m4a')
    media = mediainfo_json(disk_file)['format']

Expected behaviour

Same behaviour as:

from pydub import AudioSegment 
from pydub.utils import mediainfo_json 
filename = '04 The Mary Ellen Carter (Live).m4a' 
with open(filename, 'rb') as disk_file:
    audio = AudioSegment.from_file(disk_file, format='m4a') 
with open(filename, 'rb') as disk_file:  
    media = mediainfo_json(disk_file)['format'] 

where both audio and media get populated.

Actual behaviour

ValueError: seek of closed file is raised.

Your System configuration

Is there an audio file you can include to help us reproduce?

From my testing this happens with any file.

Discussion

I have a requirement to use a file-like object, which can be very expensive to download (using a custom context manager), so I can't use more than one context manager. Similarly I need to avoid making any unnecessary copies of the buffer.

This may make sense to discuss in another issue, but I noticed mediainfo_json gets called inside AudioSegment.from_file, but is only stored in a local variable info. If this were an AudioSegment attribute then it wouldn't even be necessary to call AudioSegment.from_file and mediainfo_json together. Since custom tags can be exported directly with AudioSegment.export, IMHO it would be more natural if they could also be accessed directly from the AudioSegment.

nathanielatom commented 5 years ago

I could work on a pull request to

  1. Add close=True parameter to AudioSegment.from_file which controls whether it closes the file passed in. OR
  2. Store info local variable into an AudioSegment attribute, .info, .media, ._info, or ._media, etc.

if either of these would be acceptable changes. Thoughts?

Hiyorimi commented 4 years ago

Really annoying behaviour. Any chances close param will be added?