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

[Thomas Guillem]

On Tue, May 28, 2019, at 12:15, Roland Bewick wrote:
> 

> On 28/05/2019 5:09 PM, Rémi Denis-Courmont wrote:
>> 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 :
> Yes. There is no problem in that scenario. But, If you select dual subtitles in the hotkeys first, dual subtitles won't be enabled in the QT GUI.

> (Two tracks will be selected in the menu while the "Enable Dual Subtitles" option is unselected).

> Roland



Maybe you could save this parameter as a variable (like "multiple-spus") in the root vlc object.

>>> 

>>> 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> 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é. 
>> 
>> _______________________________________________
vlc-devel mailing list
To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20190528/9d27b7e2/attachment.html>