[vlc-devel] [PATCH][SoC] Qt improvements

[Pierre d'Herbemont]

Hi Lukas,

On Sep 7, 2008, at 10:10 PM, Lukas Durfina wrote:

> + typedef enum libvlc_video_filter_names_t {
> +     ADJUST,          /**< Adjust filter - brightness, contrast,  
> gamma, hue, saturation */
> +     ATMOLIGHT,
....
> +     ROTATE,
> +     SHARPEN,
> +     WAVE
> + } libvlc_video_filter_names_t;

I would rather have libvlc_AdjustFilter, libvlc_AtmoLight, etc for  
enum name, following libvlc naming scheme. (Same for  
libvlc_audio_device_t and libvlc_audio_preset_names_t).

> +typedef enum libvlc_audio_device {
> +    DEFAULT,
> +    ALSA,
> +    OSS,
> +    SDL,
> +    DIRECTX
> +} libvlc_audio_device_t;

It doesn't feel right to export that on every platform. This should be  
an opaque type queried by libvlc_audio_device_count(),  
libvlc_audio_device_at_index().

For libvlc_audio_alsa_device_longname etc. This should be exported  
through libvlc_audio_device_(s|g)et_param( audio_device, param_char,  
val ).

Or remove the alsa from the function name and makes that high level  
enough so that it's no more alsa specific.

> +typedef union
> +{
> +    int             i_int;
> +    bool            b_bool;
> +    float           f_float;
> +    char *          psz_string;
> +} libvlc_value_t;

This should be an opaque type, created by  
libvlc_value_new_from_string(), etc. And should probably contains the  
type inside the structure. Do we really need that?

Also, I don't think it is a good idea to expose structure such as  
libvlc_track_description_t, because it could be replaced by a simple  
accessor.

In a different topic,
This adds even more complexity in libvlc, and probably duplicates so  
vlccore's code. That makes me think that we should probably merge some  
part of the two (libvlccore and libvlc) APIs back...

Pierre.