Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wave file module does not support 32bit float format #60729

Open
seebk mannequin opened this issue Nov 21, 2012 · 15 comments · May be fixed by #102574
Open

wave file module does not support 32bit float format #60729

seebk mannequin opened this issue Nov 21, 2012 · 15 comments · May be fixed by #102574
Assignees
Labels
extension-modules C modules in the Modules dir topic-IO type-feature A feature request or enhancement

Comments

@seebk
Copy link
Mannequin

seebk mannequin commented Nov 21, 2012

BPO 16525
Nosy @jcea, @mdickinson, @serhiy-storchaka, @jleedev
Files
  • wave_read_issue16525.patch
  • wave_issue16525_updated.patch
  • wave_issue16525_doc.patch: Documentation patch clearly defining the supported sample formats
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/serhiy-storchaka'
    closed_at = None
    created_at = <Date 2012-11-21.20:13:47.301>
    labels = ['extension-modules', 'type-feature', 'expert-IO']
    title = 'wave file module does not support 32bit float format'
    updated_at = <Date 2020-10-15.18:42:18.176>
    user = 'https://bugs.python.org/seebk'

    bugs.python.org fields:

    activity = <Date 2020-10-15.18:42:18.176>
    actor = 'jleedev'
    assignee = 'serhiy.storchaka'
    closed = False
    closed_date = None
    closer = None
    components = ['Extension Modules', 'IO']
    creation = <Date 2012-11-21.20:13:47.301>
    creator = 'seebk'
    dependencies = []
    files = ['29079', '29085', '34264']
    hgrepos = []
    issue_num = 16525
    keywords = ['patch']
    message_count = 14.0
    messages = ['176077', '176085', '176086', '176284', '176291', '176292', '176293', '176374', '176741', '180132', '182150', '182217', '182316', '212542']
    nosy_count = 6.0
    nosy_names = ['jcea', 'mark.dickinson', 'serhiy.storchaka', 'seebk', 'jleedev', 'harveyormston']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue16525'
    versions = ['Python 3.4']

    Linked PRs

    @seebk
    Copy link
    Mannequin Author

    seebk mannequin commented Nov 21, 2012

    The wave module cannot read audio WAV files containing 32bit float values. This is a very common file type for professional audio!

    There has already been a patch some years ago which works fine but was finally not applied. I can confirm that it does not break anything and only adds support for a new number format.

    http://bugs.python.org/issue1144504

    @seebk seebk mannequin added extension-modules C modules in the Modules dir topic-IO type-feature A feature request or enhancement labels Nov 21, 2012
    @jcea
    Copy link
    Member

    jcea commented Nov 22, 2012

    This is a new feature. Targeting Python 3.4.

    @jcea
    Copy link
    Member

    jcea commented Nov 22, 2012

    Can somebody point to a floating point WAVE specification?.

    I think the issues raised in the original bug still stands. Also, support for writing should be provided too.

    http://web.archive.org/web/20080924064943/http://ccrma.stanford.edu/CCRMA/Courses/422/projects/WaveFormat/

    http://www-mmsp.ece.mcgill.ca/Documents/AudioFormats/WAVE/WAVE.html

    @seebk
    Copy link
    Mannequin Author

    seebk mannequin commented Nov 24, 2012

    Write support is no problem, I will add this.

    From reading the spec in the link you provided I think the implementation in general is OK.

    Everything apart WAVE_FORMAT_PCM should have an extension size cbSize, that's right. But only WAVE_FORMAT_EXTENSIBLE sets cbSize=22.
    So for read access it is not mandatory to check the value when WAVE_FORMAT_IEEE_FLOAT, but for writing files I would set this to zero.

    There are several other wave formats which also use float data, but most important is WAVE_FORMAT_IEEE_FLOAT, and that can be suppoted quite easily without many changes.

    @jcea
    Copy link
    Member

    jcea commented Nov 24, 2012

    Sebastian, Could you possibly write a patch with a test?.

    Could you possibly fill a contributor agreement? Details in http://www.python.org/psf/contrib/

    @seebk
    Copy link
    Mannequin Author

    seebk mannequin commented Nov 24, 2012

    I will create a patch together with a testset of example files and also fill out the agreement.

    BTW: readframes() returns bad data for 24bit PCM if big_endian==True. Furthermore IMO it doesn't make sense to return a byte stream in little endian order on a big endian system... What do you think? At least the waves doc doesn't mention or specify the endianess which will cause trouble.

    @jcea
    Copy link
    Member

    jcea commented Nov 24, 2012

    About the 24 bit PCM bug, please fill another bug.

    @seebk
    Copy link
    Mannequin Author

    seebk mannequin commented Nov 25, 2012

    Attached to this mail you find my patch for the implementation of support for 8, 16, 24, 32 bit signed int PCM and 32, 64 bit float.

    24bit on big endian systems is buggy, but this will be reported in another ticket.

    The modified test checks all number formats apart form 24bit int. If the patch is fine and will be accepted I will also update the doc and send a seperate patch for this.

    @seebk
    Copy link
    Mannequin Author

    seebk mannequin commented Dec 1, 2012

    Contribution agreement is now attached to my account. So the review can start ;)

    @seebk
    Copy link
    Mannequin Author

    seebk mannequin commented Jan 17, 2013

    Any news or feedback regarding my patch?

    @harveyormston
    Copy link
    Mannequin

    harveyormston mannequin commented Feb 15, 2013

    I see that this issue applies to Python 3.4. Nevertheless, I am now using the submitted patch (http://bugs.python.org/file28122/wave_float_issue16525.patch) with Python 2.7.

    I find that the patched module fails when calling getwavformat(), becuase the _wavformat attribute is not set.

    I attach wave_read_issue16525.patch, which properly sets the _wavformat attribute and allows getwavformat() to succeed.

    Apologies if I have not followed the proper process here. I found this patch a while ago while looking for a solution to the limitations in the standard wave module and, upon using the patch, identified this potential issue which I have attampted to fix. I thought it would be best to share that fix.

    @seebk
    Copy link
    Mannequin Author

    seebk mannequin commented Feb 16, 2013

    Thanks for the hint Harvey!
    I have updated my patch to include your changes, but only applied the second hunk for the following reasons:

    Wave_read should not assume any wave format, as it is expected to open a file during initialization. So actually the only missing thing was the assignment of the wave format during the reading of the format chunk.

    setparams() in Wave_write already sets the wavformat a few lines later together with the other parameters. So no need to set it twice...

    Can you please check if the updated patch works for you?

    @harveyormston
    Copy link
    Mannequin

    harveyormston mannequin commented Feb 18, 2013

    Thanks Sebastian. That makes sense. I've tried the updated patch and it works just fine for me.

    @serhiy-storchaka serhiy-storchaka self-assigned this Oct 14, 2013
    @seebk
    Copy link
    Mannequin Author

    seebk mannequin commented Mar 2, 2014

    After the last changes in the development version of python 3.4 the patch cannot be applied anymore. As the the other audio file readers and the wave module share a common API it may be not desireable to simply enhance the wave module with support for floating point data. In particular the underlying audiooop module is designed ro only work with integer data.

    Therefore, and taking into account that this patch and its predecessor has not been reviewed in the last 2 years, I will not take the time to modify the patch again.

    At least please consider to apply the documentation patch I have uploaded now. This will clearly state in the documentation what sample formats are supported by the wave module.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    lkoenig added a commit to lkoenig/cpython that referenced this issue Mar 8, 2023
    This add support for reading and writing floating point wave file as
    requested by python#60729.
    
    Signed-off-by: Lionel Koenig <lionelk@google.com>
    lkoenig added a commit to lkoenig/cpython that referenced this issue Mar 10, 2023
    This adds support for floating point wav files and fix python#60729.
    lkoenig added a commit to lkoenig/cpython that referenced this issue Mar 10, 2023
    This adds support for floating point wav files and fix python#60729.
    @lkoenig
    Copy link

    lkoenig commented Mar 13, 2023

    I am making a attempt at adding support for floating point wave with PR #102574 let me know what you think !

    lkoenig added a commit to lkoenig/cpython that referenced this issue Mar 14, 2023
    This adds support for floating point wav files and fix python#60729.
    lkoenig added a commit to lkoenig/cpython that referenced this issue Mar 14, 2023
    This adds support for floating point wav files and fix python#60729.
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    extension-modules C modules in the Modules dir topic-IO type-feature A feature request or enhancement
    Projects
    None yet
    Development

    Successfully merging a pull request may close this issue.

    3 participants