Timtam / Bass4Py

A simple and object-like interface for the bass audio library from un4seen.com.
0 stars 1 forks source link

Remove the length parameter for functions like Channel.get_data and Channel.put_data #6

Open Keithcat1 opened 2 years ago

Keithcat1 commented 2 years ago

If you don't want to pass the entire buffer to BASS, then pass a slice, no extra work necessary for Cython. If you want to pass the equivalent of NULL to BASS then do:

    def get_data(Channel self, unsigned char[:] data):
        BASS_ChannelGetData(self.channel, &data[0] if data is not None and data.shape[0] > 0 else NULL, <QWORD>data.shape[0]) # if the memoryview has a length of 0 or is None, pass NULL

You can efficiently retrieve the length of a 1D memoryview with:

data.shape[0]

which is of type Py_ssize_t, which is 32-bits on 32-bit systems and 64-bit on 64-bit systems I believe.

Timtam commented 2 years ago

You're right for any put methods like put_data(), but get_data() is meant to retrieve data, there is no buffer to hand over to this method, thus the length parameter has to stay to indicate the amount of data you want to get from the stream. i'm currently working through all objects, adding todo hints wherever I see fit for later and fixing stuff on the fly, i'll deffinitely rework any put methods anyway, thanks for the hint.

Keithcat1 commented 2 years ago

No, you can define get_data in the following way, it will allow the user to pass in any mutable bytes-like object like a bytearray

def get_data(Channel self, unsigned char[:] data not None): # we did not mark data as const, so it will be a writable memoryview. Thus, it's safe to pass it to BASS, at least for the duration of this function call
  BASS_ChannelGetData(self.channel, &data[0], data.shape[0])
Timtam commented 2 years ago

Thats absolutely not pythonic here, more like C-ish. Imagine file reading to work like this in Python. Nope, the get data method is meant to return the data, just like any other file reader method too, you just need to tell it how much its supposed to read. Things get that much more complicated if the users need to learn about creating empty buffers, keep them alive, how to actually extract the data so that they can pass it on to other methods and so on. We could probably make the length parameter optional to let it read as much data as it can (if thats not already the case), but your solution really doesn't follow the Pythonic way of life as far as I can tell, sorry.

Keithcat1 commented 2 years ago

Example code of getting all data from a decode stream would look like this:

channel = my_output_device.create_stream_from_file("myfile.wav", flags = constants.STREAM.DECODE)
# get the length of the stream
length = channel.get_length()
# make a bytearray big enough to contain all the data
data_buffer = bytearray(length)
# tell BASS to copy data into the buffer
channel.get_data(data_buffer)
# do something with the data

There is no need to keep the data buffer alive because it's a Python object and BASS_ChannelGetData does not need to hold on to the provided pointer. Also see the readinto() method which is implemented by bytes-like file objects like io.BytesIO and any file object returned by open(file, "rb")

Timtam commented 2 years ago

I never used bytearrays before you started this discussion, and i'm programming in Python since 2013 :D. Still doesn't feel pythonic to me. Plus, we wouldn't save any parameter. For me it just looks like you're changing the length (int) against a buffer (bytearray), you'd just have to code more due to initializing the bytearray. For me, this code looks shorter:

length = strm.get_length()
data = strm.get_data(length)

Your example is longer and more complicated as far as I can tell. I also see no obvious advantages here.

We can have both ways however. You can extend get_data() to instead of accepting the length attribute, accept a bytearray or integer instead. If a bytearray is handed as parameter, you just do what you proposed and if its the length as an int, we'll just use the logic that is already there. We can deffinitely handle typing via overloads in that case I guess. I can do it too, but it'd deffinitely not be of high priority to me right now.

Keithcat1 commented 2 years ago

The advantage here is performance. Since the user is providing the buffer, we don't need to allocate any memory. Also, you only need to initialize the bytearray once, then you can replace the data inside it using BASS_ChannelGetData as much as you like.

Timtam commented 2 years ago

I'd be interested in seeing a performance comparison here as soon as we got both implementations ready.