TeamPyOgg / PyOgg

Simple OGG Vorbis, Opus and FLAC bindings for Python
The Unlicense
63 stars 27 forks source link

OpusFile and OpusFileStream: Open Opus files from memory. #90

Open ijc8 opened 3 years ago

ijc8 commented 3 years ago

It'd be nice if OpusFile (and, for consistency, OpusFileStream) supported opening Opus files from memory. The underlying opusfile library supports this via op_open_memory(), and this function is already wrapped in opus.py.

I think this would involve tweaking OpusFile.__init__() and OpusFileStream.__init__() to take path: str OR something like data: memoryview (or maybe collections.abc.ByteString).

(For what it's worth, the opposite direction already works nicely for in-memory usage, as OggOpusWriter accepts memoryview in write() and can take io.BytesIO in __init__().)

Let me know if this seems like a reasonable feature; I'd be happy to submit a PR.

Thanks for the useful bindings! Ian

mattgwwalker commented 3 years ago

Hey Ian,

Sounds like a great idea to me. Thanks for the offer.

Please add tests in to demonstrate the functionality.

Cheers,

Matthew

On Sat, 11 Sep 2021 at 08:51, Ian Clester @.***> wrote:

It'd be nice if OpusFile (and, for consistency, OpusFileStream) supported opening Opus files from memory. The underlying opusfile library supports this via op_open_memory() https://opus-codec.org/docs/opusfile_api-0.7/group__stream__open__close.html#gaffc5769a1e5977f186f77a1fb08cb248, and this function is already wrapped in opus.py https://github.com/TeamPyOgg/PyOgg/blob/6871a4f234e8a3a346c4874a12509bfa02c4c63a/pyogg/opus.py#L1026 .

I think this would involve tweaking OpusFile.init() and OpusFileStream.init() to take path: str OR something like data: memoryview (or maybe collections.abc.ByteString).

(For what it's worth, the opposite direction already works nicely for in-memory usage, as OggOpusWriter accepts memoryview in write() and can take io.BytesIO in init().)

Let me know if this seems like a reasonable feature; I'd be happy to submit a PR.

Thanks for the useful bindings! Ian

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/TeamPyOgg/PyOgg/issues/90, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA653XEIOOOL4NYWWYRWOLDUBJVW3ANCNFSM5D2C2BRQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

ijc8 commented 3 years ago

Hi Matthew,

Great! I went ahead and submitted #91 for this. A little ctypes finagling was required to avoid an extra copy, but everything typechecks and passes (including the new tests). Let me know if you'd like to see any changes (in style, API, tests, etc.).

Best, Ian

mattgwwalker commented 3 years ago

Awesome :o) Thanks Ian. I’ll try to get to it this week.

Cheers,

Matthew

On Tue, 14 Sep 2021 at 10:56, Ian Clester @.***> wrote:

Hi Matthew,

Great! I went ahead and submitted #91 https://github.com/TeamPyOgg/PyOgg/pull/91 for this. A little ctypes finagling was required to avoid an extra copy, but everything typechecks and passes (including the new tests). Let me know if you'd like to see any changes (in style, API, tests, etc.).

Best, Ian

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/TeamPyOgg/PyOgg/issues/90#issuecomment-918642954, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA653XAVY74XCW3VDN4ZYPTUBZ6TBANCNFSM5D2C2BRQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

AndreasGocht commented 2 years ago

Hey,

any updates? I accidentally implemented the same functionality 😄

Best,

Andreas