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

Fix bug in the shutdown behaviour of the decoder #26

Merged
merged 8 commits into from
Apr 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
flake8:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
- name: Lint the Python code
uses: TrueBrain/actions-flake8@master
with:
Expand Down
14 changes: 7 additions & 7 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,22 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
python-version: ["3.8", "3.9", "3.10", "3.11"]
python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"]

steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
- name: Install dependencies
run: |
sudo apt-get update -y
sudo apt-get install libsndfile1
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v4
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}
- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install coverage
pip install coverage setuptools
- name: Run tests
run: coverage run setup.py test
- name: Run coveralls
Expand All @@ -42,9 +42,9 @@ jobs:
macos:
runs-on: macos-latest
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
- name: Set up Python
uses: actions/setup-python@v4
uses: actions/setup-python@v5
with:
python-version: 3.11
- name: Install dependencies
Expand All @@ -58,7 +58,7 @@ jobs:
windows:
runs-on: windows-latest
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
with:
submodules: recursive
- name: Check install
Expand Down
8 changes: 8 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
pyFLAC Changelog
----------------

**v3.0.0**

* Fixed bug in the shutdown behaviour of the `StreamDecoder` (see #22 and #23).
* Automatically detect bit depth of input data in the `FileEncoder`, and
raise an error if not 16-bit or 32-bit PCM (see #24).
* Added a new `OneShotDecoder` to decode a buffer of FLAC data in a single
blocking operation, without the use of threads. Courtesy of @GOAE.

**v2.2.0**

* Updated FLAC library to v1.4.3.
Expand Down
2 changes: 2 additions & 0 deletions docs/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ data directly from a file or process in real-time.
:undoc-members:
:inherited-members:

.. autoclass:: pyflac.OneShotDecoder

State
-----

Expand Down
11 changes: 10 additions & 1 deletion examples/passthrough.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,16 @@ def __init__(self, args):
self.idx = 0
self.total_bytes = 0
self.queue = queue.SimpleQueue()
self.data, self.sr = sf.read(args.input_file, dtype='int16', always_2d=True)

info = sf.info(str(args.input_file))
if info.subtype == 'PCM_16':
dtype = 'int16'
elif info.subtype == 'PCM_32':
dtype = 'int32'
else:
raise ValueError(f'WAV input data type must be either PCM_16 or PCM_32: Got {info.subtype}')

self.data, self.sr = sf.read(args.input_file, dtype=dtype, always_2d=True)

self.encoder = pyflac.StreamEncoder(
write_callback=self.encoder_callback,
Expand Down
6 changes: 4 additions & 2 deletions pyflac/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@
#
# pyFLAC
#
# Copyright (c) 2020-2021, Sonos, Inc.
# Copyright (c) 2020-2024, Sonos, Inc.
# All rights reserved.
#
# ------------------------------------------------------------------------------

__title__ = 'pyFLAC'
__version__ = '2.2.0'
__version__ = '3.0.0'
__all__ = [
'StreamEncoder',
'FileEncoder',
Expand All @@ -19,6 +19,7 @@
'EncoderProcessException',
'StreamDecoder',
'FileDecoder',
'OneShotDecoder',
'DecoderState',
'DecoderInitException',
'DecoderProcessException'
Expand Down Expand Up @@ -55,6 +56,7 @@
from .decoder import (
StreamDecoder,
FileDecoder,
OneShotDecoder,
DecoderState,
DecoderInitException,
DecoderProcessException
Expand Down
2 changes: 0 additions & 2 deletions pyflac/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ def get_args():
parser.add_argument('-c', '--compression-level', type=int, choices=range(0, 9), default=5,
help='0 is the fastest compression, 5 is the default, 8 is the highest compression')
parser.add_argument('-b', '--block-size', type=int, default=0, help='The block size')
parser.add_argument('-d', '--dtype', default='int16', help='The encoded data type (int16 or int32)')
parser.add_argument('-v', '--verify', action='store_false', default=True, help='Verify the compressed data')
args = parser.parse_args()
return args
Expand All @@ -45,7 +44,6 @@ def main():
input_file=args.input_file,
output_file=args.output_file,
blocksize=args.block_size,
dtype=args.dtype,
compression_level=args.compression_level,
verify=args.verify
)
Expand Down
118 changes: 108 additions & 10 deletions pyflac/decoder.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#
# pyFLAC decoder
#
# Copyright (c) 2020-2021, Sonos, Inc.
# Copyright (c) 2020-2024, Sonos, Inc.
# All rights reserved.
#
# ------------------------------------------------------------------------------
Expand Down Expand Up @@ -93,7 +93,8 @@
Flushes the decoding buffer, releases resources, resets the decoder
settings to their defaults, and returns the decoder state to `DecoderState.UNINITIALIZED`.

A well behaved program should always call this at the end.
A well behaved program should always call this at the end, otherwise the processing
thread will be left running, awaiting more data.
"""
_lib.FLAC__stream_decoder_finish(self._decoder)

Expand Down Expand Up @@ -121,6 +122,9 @@
blocks of raw uncompressed audio is passed back to the user via
the `callback`.

The `finish` method must be called at the end of the decoding process,
otherwise the processing thread will be left running.

Args:
write_callback (fn): Function to call when there is uncompressed
audio data ready, see the example below for more information.
Expand Down Expand Up @@ -159,6 +163,8 @@

self._done = False
self._buffer = deque()
self._event = threading.Event()
self._lock = threading.Lock()
self.write_callback = write_callback

rc = _lib.FLAC__stream_decoder_init_stream(
Expand Down Expand Up @@ -200,7 +206,10 @@
Args:
data (bytes): Bytes of FLAC data
"""
self._lock.acquire()
self._buffer.append(data)
self._lock.release()
self._event.set()

def finish(self):
"""
Expand All @@ -225,8 +234,9 @@
# Instruct the decoder to finish up and wait until it is done
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The above while loop can be removed if the timeout in the below join is also removed. This way we're closer to being purely event driven; that above while loop is polling.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually nevermind, I see that _done forces the worker to exit even if the buffer's still not empty without that loop. Could make the worker not exit until the buffer's flushed though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do agree with removing the timeout. If join hangs indefinitely it might help spot any shutdown issues before releasing.

# --------------------------------------------------------------
self._done = True
self._event.set()
self._thread.join()
super().finish()
self._thread.join(timeout=3)
if self._error:
raise DecoderProcessException(self._error)

Expand Down Expand Up @@ -303,6 +313,84 @@
self.__output.write(data)


class OneShotDecoder(_Decoder):
"""
A pyFLAC one-shot decoder converts a buffer of FLAC encoded
bytes back to raw audio data. Unlike the `StreamDecoder` class,
the one-shot decoder operates on a single block of data, and
runs in a blocking manner, as opposed to in a background thread.

The compressed data is passed in via the constructor, and
blocks of raw uncompressed audio is passed back to the user via
the `callback`.

Args:
write_callback (fn): Function to call when there is uncompressed
audio data ready, see the example below for more information.
buffer (bytes): The FLAC encoded audio data

Examples:
An example callback which writes the audio data to file
using SoundFile.

.. code-block:: python
:linenos:

import soundfile as sf

def callback(self,
audio: np.ndarray,
sample_rate: int,
num_channels: int,
num_samples: int):

# ------------------------------------------------------
# Note: num_samples is the number of samples per channel
# ------------------------------------------------------
if self.output is None:
self.output = sf.SoundFile(
'output.wav', mode='w', channels=num_channels,
samplerate=sample_rate
)
self.output.write(audio)

Raises:
DecoderInitException: If initialisation of the decoder fails
"""
def __init__(self,
write_callback: Callable[[np.ndarray, int, int, int], None],
buffer: bytes):
super().__init__()
self._done = False
self._buffer = deque()
self._buffer.append(buffer)
self._event = threading.Event()
self._event.set()
self._lock = threading.Lock()
self.write_callback = write_callback

rc = _lib.FLAC__stream_decoder_init_stream(
self._decoder,
_lib._read_callback,
_ffi.NULL,
_ffi.NULL,
_ffi.NULL,
_ffi.NULL,
_lib._write_callback,
_ffi.NULL,
_lib._error_callback,
self._decoder_handle
)
if rc != _lib.FLAC__STREAM_DECODER_INIT_STATUS_OK:
raise DecoderInitException(rc)

while len(self._buffer) > 0:
_lib.FLAC__stream_decoder_process_single(self._decoder)

self._done = True
super().finish()


@_ffi.def_extern(error=_lib.FLAC__STREAM_DECODER_READ_STATUS_ABORT)
def _read_callback(_decoder,
byte_buffer,
Expand All @@ -314,6 +402,12 @@
If an exception is raised here, the abort status is returned.
"""
decoder = _ffi.from_handle(client_data)

# ----------------------------------------------------------
# Wait until there is something in the buffer, or an error
# occurs, or the end of the stream is reached.
# ----------------------------------------------------------
decoder._event.wait()
if decoder._error:
# ----------------------------------------------------------
# If an error has been issued via the error callback, then
Expand All @@ -329,18 +423,13 @@
num_bytes[0] = 0
return _lib.FLAC__STREAM_DECODER_READ_STATUS_END_OF_STREAM

maximum_bytes = int(num_bytes[0])
while len(decoder._buffer) == 0:
# ----------------------------------------------------------
# Wait until there is something in the buffer
# ----------------------------------------------------------
time.sleep(0.01)

# --------------------------------------------------------------
# Ensure only the maximum bytes or less is taken from
# the thread safe queue.
# --------------------------------------------------------------
data = bytes()
maximum_bytes = int(num_bytes[0])
decoder._lock.acquire()
if len(decoder._buffer[0]) <= maximum_bytes:
joetoddsonos marked this conversation as resolved.
Show resolved Hide resolved
data = decoder._buffer.popleft()
maximum_bytes -= len(data)
Expand All @@ -349,6 +438,14 @@
data += decoder._buffer[0][0:maximum_bytes]
decoder._buffer[0] = decoder._buffer[0][maximum_bytes:]

# --------------------------------------------------------------
# If there is no more data to process from the buffer, then
# clear the event, the thread will await more data to process.
# --------------------------------------------------------------
if len(decoder._buffer) == 0 or (len(decoder._buffer) > 0 and len(decoder._buffer[0]) == 0):
decoder._event.clear()
decoder._lock.release()

actual_bytes = len(data)
num_bytes[0] = actual_bytes
_ffi.memmove(byte_buffer, data, actual_bytes)
Expand Down Expand Up @@ -403,7 +500,7 @@
bytes_per_frame = frame.header.blocksize * np.dtype(np.int32).itemsize

if frame.header.bits_per_sample not in (16, 32):
raise ValueError('Only int16/int32 data type is supported')

Check failure on line 503 in pyflac/decoder.py

View workflow job for this annotation

GitHub Actions / macos

Only int16/int32 data type is supported

Check failure on line 503 in pyflac/decoder.py

View workflow job for this annotation

GitHub Actions / linux (3.8)

Only int16/int32 data type is supported

Check failure on line 503 in pyflac/decoder.py

View workflow job for this annotation

GitHub Actions / linux (3.9)

Only int16/int32 data type is supported

Check failure on line 503 in pyflac/decoder.py

View workflow job for this annotation

GitHub Actions / linux (3.10)

Only int16/int32 data type is supported

Check failure on line 503 in pyflac/decoder.py

View workflow job for this annotation

GitHub Actions / linux (3.11)

Only int16/int32 data type is supported

Check failure on line 503 in pyflac/decoder.py

View workflow job for this annotation

GitHub Actions / linux (3.12)

Only int16/int32 data type is supported

# --------------------------------------------------------------
# The buffer contains an array of pointers to decoded channels
Expand Down Expand Up @@ -449,3 +546,4 @@
_lib.FLAC__StreamDecoderErrorStatusString[status]).decode()
decoder.logger.error(f'Error in libFLAC decoder: {message}')
decoder._error = message
decoder._event.set()
16 changes: 10 additions & 6 deletions pyflac/encoder.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#
# pyFLAC encoder
#
# Copyright (c) 2020-2021, Sonos, Inc.
# Copyright (c) 2020-2024, Sonos, Inc.
# All rights reserved.
#
# ------------------------------------------------------------------------------
Expand Down Expand Up @@ -335,6 +335,8 @@ class FileEncoder(_Encoder):
The pyFLAC file encoder reads the raw audio data from the WAV file and
writes the encoded audio data to a FLAC file.

Note that the input WAV file must be either PCM_16 or PCM_32.

Args:
input_file (pathlib.Path): Path to the input WAV file
output_file (pathlib.Path): Path to the output FLAC file, a temporary
Expand All @@ -345,8 +347,6 @@ class FileEncoder(_Encoder):
blocksize (int): The size of the block to be returned in the
callback. The default is 0 which allows libFLAC to determine
the best block size.
dtype (str): The data type to use in the FLAC encoder, either int16 or int32,
defaults to int16.
streamable_subset (bool): Whether to use the streamable subset for encoding.
If true the encoder will check settings for compatibility. If false,
the settings may take advantage of the full range that the format allows.
Expand All @@ -365,13 +365,17 @@ def __init__(self,
output_file: Path = None,
compression_level: int = 5,
blocksize: int = 0,
dtype: str = 'int16',
streamable_subset: bool = True,
verify: bool = False):
super().__init__()

if dtype not in ('int16', 'int32'):
raise ValueError('FLAC encoding data type must be either int16 or int32')
info = sf.info(str(input_file))
if info.subtype == 'PCM_16':
dtype = 'int16'
elif info.subtype == 'PCM_32':
dtype = 'int32'
else:
raise ValueError(f'WAV input data type must be either PCM_16 or PCM_32: Got {info.subtype}')

self.__raw_audio, sample_rate = sf.read(str(input_file), dtype=dtype)
if output_file:
Expand Down
Loading