Skip to content

Conversation

@yanns1
Copy link
Contributor

@yanns1 yanns1 commented Feb 18, 2025

I identified and fixed two bugs:

  1. During bindings generation (PythonGenerator.generate_funcs and PythonGenerator.generate_func_pointer_decorator), Func.docs_in_sphinx_format was called after Par.flags, when the former fills the out member of Func, which determines the output of Par.flags. This resulted in out parameters being ignored, even though they are properly gathered by Func.docs_in_sphinx_format. Actually, some are still taken into account, because in Par.flags, some pointer types are hard-coded to be considered out parameters (this hard-coding may not be necessary anymore).

    This issue is fixed by commit "Partially fix regression in [OUT] parameter handling".

    The functions concerned by the fix are:

    • libvlc_log_get_context
    • libvlc_log_get_object
  2. Func.docs_in_sphinx_format did not properly gather all out parameters. It failed to gather those that have documentation spanning multiple lines. As the [OUT] flag is often (always?) present at the end of a parameter's documentation, it was missed in those cases.

    This issue is fixed by commit "Fix regression in [OUT] parameters handling".

    The functions concerned by the fix are:

    • libvlc_media_discoverer_list_get
    • libvlc_media_player_get_full_chapter_descriptions
    • libvlc_media_player_get_full_title_descriptions
    • libvlc_media_slaves_get
    • libvlc_media_tracks_get
    • libvlc_renderer_discoverer_list_get

    What I have not done is change generator/templates/override.py, which has calls to:

    • libvlc_media_player_get_full_chapter_descriptions
    • libvlc_media_player_get_full_title_descriptions
    • libvlc_media_tracks_get

    that now take an incorrect number of arguments.
    Tests fail for this reason.

Otherwise, the first commit ("Fix dumping of parameters that are Func and not Par") fixes a small bug in the code for dumping in debug mode.

Part of the problem was that `Func.docs_in_sphinx_format`
is called after `Par.flags` during generation, when the former
method fills the `out` member of `Func`, on which the output
of `Par.flags` depends.

It resulted in out parameters being incorrectly flagged as in
parameters (because it is the default).

The concerned functions are:

- `libvlc_log_get_context`
- `libvlc_log_get_object`.
Out parameters that have documentation spanning multiple lines were
not flagged as out parameters, because only the presence of `[OUT]`
in the _first_ documentation line was checked.

This commit fixes the issue be checking the presence of `[OUT]` in
other lines as well.

The concerned functions are:

- `libvlc_media_discoverer_list_get`
- `libvlc_media_player_get_full_chapter_descriptions`
- `libvlc_media_player_get_full_title_descriptions`
- `libvlc_media_slaves_get`
- `libvlc_media_tracks_get`
- `libvlc_renderer_discoverer_list_get`
@oaubert oaubert merged commit b917fb7 into oaubert:master Feb 18, 2025
2 checks passed
@oaubert
Copy link
Owner

oaubert commented Feb 18, 2025

Great, thanks! I had a look at the media_tracks_get issue, but we will need to fix the ctypes code before updating the override code: calling the vlc.libvlc_media_tracks_get(m) returns a single <vlc.LP_MediaTrack object> (OUT parameter), losing the actual return value (number of elements).

To solve this, I think we will have to write a list_result errcheck method (similar to class_result) that will convert the OUT value as a python list with the appropriate number of elements, and update the generate_funcs code (which is where the errcheck code is injected), basically to do in ctypes the conversion that was done in the override wrapper previously.

I got to something like:

def list_result(result, func, arguments):
    """Errcheck function. Returns a function that creates the specified list."""
    # result is the actual result of the function (an uint)
    # func is a reference to the CFunctionType instance
    # arguments is the list of arguments, as specified by flags (i.e. with the input one if it was specified)
    if not result:
        return []
    else:
        arraytype = arguments[1]._type_ * result
        return ctypes.cast(arguments[1], arraytype)

but the last conversion needs some thinking, and then some more investigation to make sure that the correct argument is used. To be continued...

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 this pull request may close these issues.

2 participants