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

media_new_callbacks: c_char_p for foreign buffer #117

Closed
albestro opened this issue Dec 4, 2019 · 4 comments · Fixed by #125
Closed

media_new_callbacks: c_char_p for foreign buffer #117

albestro opened this issue Dec 4, 2019 · 4 comments · Fixed by #125

Comments

@albestro
Copy link
Contributor

albestro commented Dec 4, 2019

DISCLAIMER
I'm new both to libvlc and ctypes, so please correct me if I'm wrong on something.

I'm experimenting with media_new_callbacks and I addressed some problems defining the read callback read_cb.

In libvlc the signature for the read callback is defined as

typedef ssize_t(* libvlc_media_read_cb) (void *opaque, unsigned char *buf, size_t len)

where the 2nd argument unsigned char *buf points to a buffer allocated by VLC and that the callback implementation should populate.

In python-vlc this signature is mapped to vlc.CallbackDecorators.MediaReadCb that is

MediaReadCb = ctypes.CFUNCTYPE(
  ctypes.c_void_p,
  ctypes.c_void_p, ctypes.c_char_p, ctypes.c_size_t)

where the type of the 2nd argument (the 3rd of this signature, since the 1st should be the return type) is ctypes.c_char_p.

In my knowledge, ctypes.c_char_p does not allow to change the value of the pointed data, and if you request the value, you get a copy of it (usually, I see it used with immutable objects). Moreover, inside the callback, the buffer parameter is automatically translated to a <class bytes>, so I was not able to populate the VLC buffer.

After a lot of tests, I decided to use my own signature of MediaReadCb by changing c_char_p to POINTER(c_char), which now is

MediaReadCb = ctypes.CFUNCTYPE(
  ctypes.c_void_p,
  ctypes.c_void_p, ctypes.POINTER(c_char), ctypes.c_size_t)

and now from inside the read callback, I'm able to do something like

@MediaReadCb
def media_read_cb(opaque, buffer, length):
  # ...
  new_data = stream.read() # stream is a _io.BufferedReader
  bytes_read = len(new_data)
  buffer_array = ctypes.cast(buffer, ctypes.POINTER(ctypes.c_char * length))
  ctypes.memmove(buffer_array, new_data, bytes_read)

and it works!

Is this a real problem or I can do it with c_char_p (can I cast it or get the pointer? from ctypes documentation I didn't find anything in the direction of "pointer to mutable").

Thank you in advance for any feedback!

@albestro albestro changed the title problem with media_new_callbacks: c_char_p for foreign buffer problem with media_new_callbacks: c_char_p for foreign buffer Dec 4, 2019
@albestro albestro changed the title problem with media_new_callbacks: c_char_p for foreign buffer media_new_callbacks: c_char_p for foreign buffer Dec 4, 2019
@daniel5gh
Copy link

Just for reference, this looks like it is similar to what I encountered with callbacks for video_set_callbacks

#17

@oaubert
Copy link
Owner

oaubert commented Dec 17, 2019

Your solution seems appropriate. It could be generalized into a helper function, to use in all relevant locations. The issue here is to be able to distinguish in the generator between the cases where c_char_p is appropriate, and the cases where a cast/memmove is necessary. This does not seem possible by introspecting the code alone, some heuristics have to be found. I will gladly integrate contributions if you get something.

@albestro
Copy link
Contributor Author

After a quick look at the code, I see that constness information for parameters is discarded

def parse_param(self, param):
"""Parse a C parameter expression.
It is used to parse the type/name of functions
and type/name of the function parameters.
@return: a Par instance.
"""
t = param.replace('const', '').strip()

You may have already evaluated to use this information to distinguish between immutable/mutable data, and so to use it to decide whether to use c_char_p or POINTER(c_char).

What do you think about this?

@oaubert
Copy link
Owner

oaubert commented Jan 25, 2020

Well spotted, it would be a first step to allow for proper ctypes mapping. This could be stored as a new property of the Par class, which would allow to do specific mapping. Do you want to try it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants