[vlc-devel] [PATCH 03/16] Add variable to enable/disable dual subtitles

Rémi Denis-Courmont remi at remlab.net
Tue May 28 12:09:21 CEST 2019


I'm trying to keep things simple by not leaking stuff across UIs especially when some won't handle it.

Since separate hotkeys are already there, I don't see why you need a flag at all as far as hotkeys are concerned.

Le 28 mai 2019 12:23:58 GMT+03:00, Roland Bewick <roland.bewick at gmail.com> a écrit :
>
>On 28/05/2019 1:38 PM, Rémi Denis-Courmont wrote:
>> Hi,
>>
>> The whole point of adding a call to atomically select all SPU ES is 
>> that we don't need to keep UIs in sync.
>>
>> Keeping that stuff synched would break existing UIs that don't
>support 
>> multiple SPU, which are not going to disappear overnight.
>>
>> And it would require some code-complex and user-uninteligible 
>> transaction model... Or what will happen if a UI asks to enable two 
>> SPU's after another UI disabled multiple SPU's??
>>
>> Le 28 mai 2019 08:45:41 GMT+03:00, Roland Bewick 
>> <roland.bewick at gmail.com> a écrit :
>
>Hi, please refer to Jean-Baptiste's requirement: "If you enable
>dual-spu 
>in Qt, it must work in the hotkeys.",
>
>As to the existing UIs - They don't use the new 
>vlc_player_SelectTrackList function.
>
>I feel like you're making this over-complicated.
>
>
>
>>
>>     On 28/05/2019 12:12 PM, Rémi Denis-Courmont wrote:
>>>     This belongs in the UI, not the player as I explained in a
>>>     previous review.
>>
>>     Hi,
>>
>>     I think you are referring to this review:
>>
>>>     While it likely makes sense at the level of UI widgets and user
>>>     interactions, it seems a bit out of place, standing alone, at
>the
>>>     player API level.
>>>
>>>     In particular, I doubt that enabling multiple tracks mode in Qt
>>>     should interfere with the track selection of, say, the HTTP
>>>     remote control.
>>>
>>>     IMO, the API should just take the list of active tracks, whether
>>>     it be empty, a singleton, or larger.
>>
>>     Now that we've added the vlc_player_SelectTrackList function,
>only
>>     the first line of your review still needs to be addressed.
>>
>>     If we don't want to use the player API to enable / check if
>>     subtitles are enabled, I need a way to enable/disable dual
>>     subtitles and keep that synced between the hotkeys, QT GUI and
>any
>>     future interfaces that might allow dual subtitle selection.
>>
>>     So, if it should belong in the UI as you suggest, could you give
>>     some more information on how this might be implemented? How is
>>     dual subtitles enabled in interface A and then used to select
>>     another subtitle track in interface B or C?
>>
>>     Note: There are other subtitle-related functions
>>     (SetSubtitleEnabled, SetSubtitleDelay, GetSubtitleDelay,
>>     SetSubtitleSync, SetSubtitleTextScale, GetSubtitleTextScale, ) in
>>     the player API too. I think the two methods I added
>>     (SetDualSubtitlesEnabled, AreDualSubtitlesEnabled) could fit.
>>
>>
>>>
>>>     Le 28 mai 2019 05:06:54 GMT+03:00, Roland Bewick
>>>     <roland.bewick at gmail.com> a écrit :
>>>
>>>         So the state has to be linked across the interfaces. In this
>>>         case shouldn’t it belong to the player? If not, where?
>>>
>>>         I like Thomas’ idea: Store the bool in the vlc_player_t
>struct.
>>>
>>>         Roland
>>>
>>>         On Mon, 27 May 2019 at 11:53 PM, Rémi Denis-Courmont
>>>         <remi at remlab.net <mailto:remi at remlab.net>> wrote:
>>>
>>>             Le maanantaina 27. toukokuuta 2019, 10.19.43 EEST Thomas
>>>             Guillem a écrit :
>>>             > Hello,
>>>             >
>>>             > This should not belong to the player.
>>>             > I think you should store the internal dual subtitle
>>>             state in each
>>>             > interfaces.
>>>
>>>             Yes and I wrote something to same effect in a previous
>>>             review.
>>>
>>>             -- 
>>>             Rémi Denis-Courmont
>>>             http://www.remlab.net/
>>>
>>>
>>>
>>>             _______________________________________________
>>>             vlc-devel mailing list
>>>             To unsubscribe or modify your subscription options:
>>>             https://mailman.videolan.org/listinfo/vlc-devel
>>>
>>>
>>>     -- 
>>>     Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez
>>>     excuser ma brièveté.
>>>
>>>     _______________________________________________
>>>     vlc-devel mailing list
>>>     To unsubscribe or modify your subscription options:
>>>     https://mailman.videolan.org/listinfo/vlc-devel
>>
>>
>> -- 
>> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez 
>> excuser ma brièveté.
>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel

-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20190528/ac683f06/attachment.html>


More information about the vlc-devel mailing list