[vlc-devel] [PATCH 2/2] libvlc: add event libvlc_MediaPlayerESChanged

[Rémi Denis-Courmont]

Le 2014-07-16 15:11, Felix Abecassis a écrit :
> 2014-07-16 13:43 GMT+02:00 Rémi Denis-Courmont <remi at remlab.net>:
>> Le 2014-07-16 14:06, Felix Abecassis a écrit :
>>
>>> ---
>>>  include/vlc/libvlc_events.h |  6 ++++++
>>>  lib/event.c                 |  1 +
>>>  lib/media_player.c          | 21 +++++++++++++++++++++
>>>  3 files changed, 28 insertions(+)
>>>
>>> diff --git a/include/vlc/libvlc_events.h 
>>> b/include/vlc/libvlc_events.h
>>> index f268fb5..1d4f781 100644
>>> --- a/include/vlc/libvlc_events.h
>>> +++ b/include/vlc/libvlc_events.h
>>> @@ -74,6 +74,7 @@ enum libvlc_event_e {
>>>      libvlc_MediaPlayerLengthChanged,
>>>      libvlc_MediaPlayerVout,
>>>      libvlc_MediaPlayerScrambledChanged,
>>> +    libvlc_MediaPlayerESChanged,
>>>
>>>      libvlc_MediaListItemAdded=0x200,
>>>      libvlc_MediaListWillAddItem,
>>> @@ -230,6 +231,11 @@ typedef struct libvlc_event_t
>>>          {
>>>              libvlc_media_t * new_media;
>>>          } media_player_media_changed;
>>> +
>>> +        struct
>>> +        {
>>> +            libvlc_track_type_t i_type;
>>> +        } media_player_es_changed;
>>
>>
>> That does not seem very useful. The callback would almost certainly 
>> need to
>> know if the ES was added, changed  or removed, and since there may 
>> be more
>> than one ES of a given type, the unique ID.
>>
>
> If we want to provide only one additional piece of information for
> this event, I think we should provide the ES type instead of the ID.

The ID is unique, the type is not. It's much easier to look up the ID 
in each list, and it does not need extra state on the library user.

> This way we immediately know which list we have to fetch again. But
> this is open to discussion.

But you need to keep track of the list, which not all library users do. 
And then you need to compare the old and the new version of the adequate 
list everytime - which is inconvenient and error-prone.

With the unique ID, you need to fetch each list and look them up. That 
is "slow", but still faster than comparing lists, and much easier to 
write. Most importantly, the user does not have to replicate the track 
lists in its private state.

> If we want to provide ES type, ID and operation type, it would be
> awkward to add more variables; I've suggested different options when
> discussing the previous submitted patch for the new libVLC event.

>> What warrants that the es-type variable wwas not overriden by 
>> another
>> pending ES change?
>
> Is this possible?

Is this not possible?

> If this is the case, we might have a similar issue
> with INPUT_EVENT_STATE and the "state" variable.

I don't really follow the parallel: there is only one input, which can 
only have one state. There are (typically) more than one ES, each with 
their own state.
But regardless, if there is a problem with "state", it's not an excuse 
to reproduce it.

-- 
Rémi Denis-Courmont