oaubert / python-vlc

Python vlc bindings
GNU Lesser General Public License v2.1
381 stars 108 forks source link

video_set_marquee_int not interpreting _Enum type of VideoMarqueeOption.Position properly, resulting in crash #243

Open devdevgoat opened 1 year ago

devdevgoat commented 1 year ago

Hi All,

Trying to implement text overlays in using player.video_set_marquee_int, but found the following fix me note:

# FIXME: This crashes the module - it should be investigated
# player.video_set_marquee_int(VideoMarqueeOption.Position, Position.bottom)

Error Received :

typeserror

Temporary Fix 1

I have no experience in ctype binding, so not sure how to implement the fix properly, but it looks like the class type _Enum of VideoMarqueeOption.Position isn't resolving to an INT type properly? When I use the native VLC integer assignments (found here) I'm able to set the overlay position as expected, for example, setting it display the text on the bottom:

# Marquee position: 0=center, 1=left, 2=right, 4=top, 8=bottom,
# you can also use combinations of these values, eg 6 = top-right. default value: -1
self.player.video_set_marquee_int(4, 8)

Temporary Fix 2

Alternatively, if I set the enum entries without the Position() wrapper (not sure what to call this), it allows for using the enum and works:

Position.bottom       = 8  
Position.bottom_left  = 9  
Position.bottom_right = 10 
Position.center       = 0  
Position.disable      = -1
Position.left         = 1 
Position.right        = 2  
Position.top          = 4  
Position.top_left     = 5  
Position.top_right    = 6 
...
player.video_set_marquee_int(VideoMarqueeOption.Position, Position.bottom)

Not sure why the other enums seem to work natively, but since I don't see any open issues discussing this, decided to start this thread with my findings.

Misc Comments

Additionally, I noticed two small discrepancies:

  1. It looks like the casing of the enums has changed and is out of sync with the example implementations which expect Position.Bottom, vs Position.bottom
  2. The enum values don't appear to match the VLC source. The below is the same fix above, pasted with the old mappings in comment form for comparison

src block


# Old mappings are in comment form to compare before/after

Position.bottom       = 8  #Position(6)
Position.bottom_left  = 9  #Position(7)
Position.bottom_right = 10 #Position(8)
Position.center       = 0  #Position(0)
Position.disable      = -1 #Position(-1)
Position.left         = 1  #Position(1)
Position.right        = 2  #Position(2)
Position.top          = 4  #Position(3)
Position.top_left     = 5  #Position(4)
Position.top_right    = 6  #Position(5)

Marq_demonstration_-_VLC_3 0 6_Linux img src

Hope this helps in troubleshooting.

mrJean1 commented 1 year ago

In addition to the incorrect Position enum values in vlc.py, there is the TypeError.

Class Position is a subclass of _Enum which in turn is a subclass of ctype.c_uint. But the i_val argument of function libvlc_video_set_marquee_int is defined as ctypes.c_int. Changing the definition of the latter inside that function to ctypes.c_uint would resolve the TypeError.

Using vlc.MediaPlayer.video_set_marquee_int(..., vlc.Position.bottom.value) also avoids the TypeError. __ PS) The examples/cocoavlc.py script suffers the same crashes for the marquee (and the logo) option issue. A workaround for cocoavlc.py is forthcoming.

mrJean1 commented 1 year ago

Investigating the TypeError further, the C functions

void libvlc_video_set_marquee_int(libvlc_media_player_t *p_mi, unsigned option, int i_val)
int  libvlc_video_get_marquee_int(libvlc_media_player_t *p_mi, unsigned option)

both define the value as int. In other words, the generated Python functions and methods are correct.

The issue is Python class Position(_Enum) generated from C enum libvlc_position_t with _Enum being a subclass of ctypes.c_uint. The Position class should be a subclass of ctypes.c_int.

There are several other _Enum subclasses** with the same type conflict. For example LogLevel(_Enum), but defined as ctypes.c_int argument level in class CallbackDecorators.LogCb. Similarly, AudioOutputDeviceTypes(_Enum) with i_device defined as ctypes.c_int in several libvlc_audio_output_device... functions and methods.

It is unclear how to resolved this in the generator.py. One option might be: define a signed base class in Python, say class _Inum(ctypes.c_int): ... and use that for C enums which include a negative enum value, like Position.disable=-1 in Position. That would only cover the obviously signed C enums.

Another option might be to use the signed _Inum base class for all C enums which have at least one int enum value listed, but excluding shifted and hex ones. That would handle the LogLevel case.

Simply making _Enum signed by changing its definition on line 1206 in function generate_enums in the generator.py to class _Enum(ctypes.c_int) does not work. That produces a TypeError for the option argument of the libvlc_video_set_marquee_int(p_mi, option, i_val) and certainly other ones. __ **) Following are all C enums the parser found (in VLC 3.0.18 on macOS):

enum libvlc_audio_output_channel_t libvlc_AudioChannel_Error   = -1, libvlc_AudioChannel_Stereo  =  1, libvlc_AudioChannel_RStereo =  2, libvlc_AudioChannel_Left    =  3, libvlc_AudioChannel_Right   =  4, libvlc_AudioChannel_Dolbys  =  5 
enum libvlc_audio_output_device_types_t libvlc_AudioOutputDevice_Error  = -1, libvlc_AudioOutputDevice_Mono   =  1, libvlc_AudioOutputDevice_Stereo =  2, libvlc_AudioOutputDevice_2F2R   =  4, libvlc_AudioOutputDevice_3F2R   =  5, libvlc_AudioOutputDevice_5_1    =  6, libvlc_AudioOutputDevice_6_1    =  7, libvlc_AudioOutputDevice_7_1    =  8, libvlc_AudioOutputDevice_SPDIF  = 10 
enum libvlc_dialog_question_type LIBVLC_DIALOG_QUESTION_NORMAL, LIBVLC_DIALOG_QUESTION_WARNING, LIBVLC_DIALOG_QUESTION_CRITICAL, 
enum libvlc_event_e libvlc_MediaMetaChanged=0, libvlc_MediaSubItemAdded, libvlc_MediaDurationChanged, libvlc_MediaParsedChanged, libvlc_MediaFreed, libvlc_MediaStateChanged, libvlc_MediaSubItemTreeAdded,  libvlc_MediaPlayerMediaChanged=0x100, libvlc_MediaPlayerNothingSpecial, libvlc_MediaPlayerOpening, libvlc_MediaPlayerBuffering, libvlc_MediaPlayerPlaying, libvlc_MediaPlayerPaused, libvlc_MediaPlayerStopped, libvlc_MediaPlayerForward, libvlc_MediaPlayerBackward, libvlc_MediaPlayerEndReached, libvlc_MediaPlayerEncounteredError, libvlc_MediaPlayerTimeChanged, libvlc_MediaPlayerPositionChanged, libvlc_MediaPlayerSeekableChanged, libvlc_MediaPlayerPausableChanged, libvlc_MediaPlayerTitleChanged, libvlc_MediaPlayerSnapshotTaken, libvlc_MediaPlayerLengthChanged, libvlc_MediaPlayerVout, libvlc_MediaPlayerScrambledChanged, libvlc_MediaPlayerESAdded, libvlc_MediaPlayerESDeleted, libvlc_MediaPlayerESSelected, libvlc_MediaPlayerCorked, libvlc_MediaPlayerUncorked, libvlc_MediaPlayerMuted, libvlc_MediaPlayerUnmuted, libvlc_MediaPlayerAudioVolume, libvlc_MediaPlayerAudioDevice, libvlc_MediaPlayerChapterChanged,  libvlc_MediaListItemAdded=0x200, libvlc_MediaListWillAddItem, libvlc_MediaListItemDeleted, libvlc_MediaListWillDeleteItem, libvlc_MediaListEndReached,  libvlc_MediaListViewItemAdded=0x300, libvlc_MediaListViewWillAddItem, libvlc_MediaListViewItemDeleted, libvlc_MediaListViewWillDeleteItem,  libvlc_MediaListPlayerPlayed=0x400, libvlc_MediaListPlayerNextItemSet, libvlc_MediaListPlayerStopped,  libvlc_MediaDiscovererStarted=0x500, libvlc_MediaDiscovererEnded,  libvlc_RendererDiscovererItemAdded, libvlc_RendererDiscovererItemDeleted,  libvlc_VlmMediaAdded=0x600, libvlc_VlmMediaRemoved, libvlc_VlmMediaChanged, libvlc_VlmMediaInstanceStarted, libvlc_VlmMediaInstanceStopped, libvlc_VlmMediaInstanceStatusInit, libvlc_VlmMediaInstanceStatusOpening, libvlc_VlmMediaInstanceStatusPlaying, libvlc_VlmMediaInstanceStatusPause, libvlc_VlmMediaInstanceStatusEnd, libvlc_VlmMediaInstanceStatusError 
enum libvlc_log_level LIBVLC_DEBUG=0, LIBVLC_NOTICE=2, LIBVLC_WARNING=3, LIBVLC_ERROR=4 
enum libvlc_media_discoverer_category_t libvlc_media_discoverer_devices, libvlc_media_discoverer_lan, libvlc_media_discoverer_podcasts, libvlc_media_discoverer_localdirs, 
enum libvlc_media_parse_flag_t libvlc_media_parse_local    = 0x00, libvlc_media_parse_network  = 0x01, libvlc_media_fetch_local    = 0x02, libvlc_media_fetch_network  = 0x04, libvlc_media_do_interact    = 0x08, 
enum libvlc_media_parsed_status_t libvlc_media_parsed_status_skipped = 1, libvlc_media_parsed_status_failed, libvlc_media_parsed_status_timeout, libvlc_media_parsed_status_done, 
enum libvlc_media_player_role libvlc_role_None = 0, libvlc_role_Music, libvlc_role_Video, libvlc_role_Communication, libvlc_role_Game, libvlc_role_Notification, libvlc_role_Animation, libvlc_role_Production, libvlc_role_Accessibility, libvlc_role_Test 
enum libvlc_media_slave_type_t libvlc_media_slave_type_subtitle, libvlc_media_slave_type_audio, 
enum libvlc_media_type_t libvlc_media_type_unknown, libvlc_media_type_file, libvlc_media_type_directory, libvlc_media_type_disc, libvlc_media_type_stream, libvlc_media_type_playlist, 
enum libvlc_meta_t libvlc_meta_Title, libvlc_meta_Artist, libvlc_meta_Genre, libvlc_meta_Copyright, libvlc_meta_Album, libvlc_meta_TrackNumber, libvlc_meta_Description, libvlc_meta_Rating, libvlc_meta_Date, libvlc_meta_Setting, libvlc_meta_URL, libvlc_meta_Language, libvlc_meta_NowPlaying, libvlc_meta_Publisher, libvlc_meta_EncodedBy, libvlc_meta_ArtworkURL, libvlc_meta_TrackID, libvlc_meta_TrackTotal, libvlc_meta_Director, libvlc_meta_Season, libvlc_meta_Episode, libvlc_meta_ShowName, libvlc_meta_Actors, libvlc_meta_AlbumArtist, libvlc_meta_DiscNumber, libvlc_meta_DiscTotal 
enum libvlc_navigate_mode_t libvlc_navigate_activate = 0, libvlc_navigate_up, libvlc_navigate_down, libvlc_navigate_left, libvlc_navigate_right, libvlc_navigate_popup 
enum libvlc_playback_mode_t libvlc_playback_mode_default, libvlc_playback_mode_loop, libvlc_playback_mode_repeat 
enum libvlc_position_t libvlc_position_disable=-1, libvlc_position_center, libvlc_position_left, libvlc_position_right, libvlc_position_top, libvlc_position_top_left, libvlc_position_top_right, libvlc_position_bottom, libvlc_position_bottom_left, libvlc_position_bottom_right 
enum libvlc_state_t libvlc_NothingSpecial=0, libvlc_Opening, libvlc_Buffering, libvlc_Playing, libvlc_Paused, libvlc_Stopped, libvlc_Ended, libvlc_Error 
enum libvlc_teletext_key_t libvlc_teletext_key_red = 'r' << 16, libvlc_teletext_key_green = 'g' << 16, libvlc_teletext_key_yellow = 'y' << 16, libvlc_teletext_key_blue = 'b' << 16, libvlc_teletext_key_index = 'i' << 16, 
enum libvlc_track_type_t libvlc_track_unknown   = -1, libvlc_track_audio     = 0, libvlc_track_video     = 1, libvlc_track_text      = 2 
enum libvlc_video_adjust_option_t libvlc_adjust_Enable = 0, libvlc_adjust_Contrast, libvlc_adjust_Brightness, libvlc_adjust_Hue, libvlc_adjust_Saturation, libvlc_adjust_Gamma 
enum libvlc_video_logo_option_t libvlc_logo_enable, libvlc_logo_file, libvlc_logo_x, libvlc_logo_y, libvlc_logo_delay, libvlc_logo_repeat, libvlc_logo_opacity, libvlc_logo_position 
enum libvlc_video_marquee_option_t libvlc_marquee_Enable = 0, libvlc_marquee_Text, libvlc_marquee_Color, libvlc_marquee_Opacity, libvlc_marquee_Position, libvlc_marquee_Refresh, libvlc_marquee_Size, libvlc_marquee_Timeout, libvlc_marquee_X, libvlc_marquee_Y 
enum libvlc_video_orient_t libvlc_video_orient_top_left, libvlc_video_orient_top_right, libvlc_video_orient_bottom_left, libvlc_video_orient_bottom_right, libvlc_video_orient_left_top, libvlc_video_orient_left_bottom, libvlc_video_orient_right_top, libvlc_video_orient_right_bottom 
enum libvlc_video_projection_t libvlc_video_projection_rectangular, libvlc_video_projection_equirectangular,  libvlc_video_projection_cubemap_layout_standard = 0x100, 
mrJean1 commented 1 year ago

None of the options mentioned above are feasible. Better would to select the unsigned _Enum or signed _Enim class based on the C enum name.

Below is are the changes made in class PythonGenerator of generator.py to test the idea. The table enum2Enim is only partially filled, but at least the LogLevel and Position enums not longer cause a TypeError.

class PythonGenerator(_Generator):
    """Generate Python bindings.
    """
    ....

    # C enums become an unsigned _Enum or signed
    # _Enim class in Python to avoid a TypeError
    # in libvlc_... function call arguments
    enum2Enim = (  # signed C enum classes  # ###
        'libvlc_audio_output_channel_t',
        'libvlc_audio_output_device_types_t',
#       'libvlc_dialog_question_type',
#       'libvlc_event_e',
        'libvlc_log_level',
#       'libvlc_media_discoverer_category_t',
#       'libvlc_media_parse_flag_t',
#       'libvlc_media_parsed_status_t',
#       'libvlc_media_player_role',
#       'libvlc_media_slave_type_t',
#       'libvlc_media_type_t',
#       'libvlc_meta_t',
#       'libvlc_navigate_mode_t',
#       'libvlc_playback_mode_t',
        'libvlc_position_t',
#       'libvlc_state_t',
#       'libvlc_teletext_key_t',
        'libvlc_track_type_t',
#       'libvlc_video_adjust_option_t',
#       'libvlc_video_logo_option_t',
#       'libvlc_video_marquee_option_t',
#       'libvlc_video_orient_t',
#       'libvlc_video_projection_t'
    )

    ....

    def generate_enums(self):
        """Generate classes for all enum types.
        """
        self.output("""
class _EnumBase():  # ###
    '''(INTERNAL) Base class
    '''
    _enum_names_ = {}

    def __str__(self):
        n = self._enum_names_.get(self.value, '') or ('FIXME_(%r)' % (self.value,))
        return '.'.join((self.__class__.__name__, n))

    def __hash__(self):
        return self.value

    def __repr__(self):
        return '.'.join((self.__class__.__module__, self.__str__()))

    def __eq__(self, other):
        return ( (isinstance(other, (_Enum, _Enim)) and self.value == other.value)  # ###
              or (isinstance(other,  _Ints)         and self.value == other) )

    def __ne__(self, other):
        return not self.__eq__(other)

class _Enum(_EnumBase, ctypes.c_uint):  # ###
    '''(INTERNAL) Unsigned C enum.
    '''
    pass

class _Enim(_EnumBase, ctypes.c_int):  # ###
    '''(INTERNAL) Signed C enum.
    '''
    pass
""")
        for e in self.parser.enums:

            cls = self.class4(e.name)  # ###
            u_i = 'i' if e.name in self.enum2Enim else 'u'  # ###
            self.output("""class %s(_En%sm):
    '''%s
    '''
    _enum_names_ = {""" % (cls, u_i, e.epydocs() or _NA_))  # ###

            for v in e.vals:
                self.output("        %s: '%s'," % (v.value, v.name))
            self.output('    }')

            # align on '=' signs
            w = -max(len(v.name) for v in e.vals)
            t = ['%s.%*s = %s(%s)' % (cls, w,v.name, cls, v.value) for v in e.vals]

            self.output(_NL_.join(sorted(t)), nt=2)
mrJean1 commented 1 year ago

See pull request for generator.py with proposed fixes for both issues, the Position enum values and un/signed enums.