[vlc-devel] [PATCH 3/3] Add UI for setting subtitle charset detection on/off

David Fuhrmann david.fuhrmann at gmail.com
Mon Apr 8 18:45:26 CEST 2019


Hi Aleksei,

Thanks for already thinking about the UI and about adding the new preference to them.
But I think before doing that, it should be discussed what is actually needed or interesting for the normal user.

We already have a popup menu, where you can select the preferred charset encoding for your subtitle. Now, just adding another check box for an automatic option, might just confuse the user even more.

—> Is the new code reliable enough to maybe get rid of both in simple prefs already?
—> Should there be an „Automatic“ entry in the existing charset list, which is selected by default?
—> If we stick with the checkbox, should be at least grey out the popup menu to make clear its setting does not have an effect at all?

How to deal with the fact that VLC might not be compiled with the uchardetect library at all (probably mainly relevant for linux, if they do not package that lib)?

BR. David


> Am 08.04.2019 um 09:43 schrieb Thomas Guillem <thomas at gllm.fr>:
> 
> 
> 
> On Sun, Apr 7, 2019, at 20:39, pertuleha at gmail.com <mailto:pertuleha at gmail.com> wrote:
>> From: Aleksei Pertu <pertuleha at gmail.com <mailto:pertuleha at gmail.com>>
>> 
>> ---
>> modules/gui/macosx/UI/SimplePreferences.xib   | 35 +++++++++++++------
>> .../preferences/VLCSimplePrefsController.h    |  1 +
>> .../preferences/VLCSimplePrefsController.m    |  2 ++
>> .../gui/qt/components/simple_preferences.cpp  |  1 +
> 
> There sould be one commit for macOS and one commit for QT.
> 
>> modules/gui/qt/ui/sprefs_subtitles.ui         |  7 ++++
>> 5 files changed, 35 insertions(+), 11 deletions(-)
>> 
>> diff --git a/modules/gui/macosx/UI/SimplePreferences.xib 
>> b/modules/gui/macosx/UI/SimplePreferences.xib
>> index 1366b78cc7..d8f4d94c00 100644
>> --- a/modules/gui/macosx/UI/SimplePreferences.xib
>> +++ b/modules/gui/macosx/UI/SimplePreferences.xib
>> @@ -84,6 +84,7 @@
>>                 <outlet property="osdView" destination="2523" 
>> id="d1o-FZ-hXa"/>
>>                 <outlet property="osd_encodingLabel" 
>> destination="2531" id="yyW-qD-zJ0"/>
>>                 <outlet property="osd_encodingPopup" 
>> destination="2532" id="mBO-m6-lIV"/>
>> +                <outlet property="osd_encodingAutoDetectCheckbox" 
>> destination="epz-hm-jAM" id="mAn-id-Mmz"/>
>>                 <outlet property="osd_fontBox" destination="2537" 
>> id="OCf-KO-i4B"/>
>>                 <outlet property="osd_fontButton" destination="2543" 
>> id="UOT-87-z16"/>
>>                 <outlet property="osd_fontLabel" destination="2542" 
>> id="LD1-g4-6Ex"/>
>> @@ -1532,7 +1533,7 @@ Gw
>>             <point key="canvasLocation" x="883" y="158"/>
>>         </customView>
>>         <customView translatesAutoresizingMaskIntoConstraints="NO" 
>> id="2523" userLabel="Subtitles & OSD Settings">
>> -            <rect key="frame" x="0.0" y="-1" width="571" height="402"/>
>> +            <rect key="frame" x="0.0" y="-1" width="571" height="428"/>
>>             <subviews>
>>                 <box title="Display Settings" 
>> translatesAutoresizingMaskIntoConstraints="NO" id="2537">
>>                     <rect key="frame" x="17" y="16" width="537" 
>> height="214"/>
>> @@ -1770,13 +1771,13 @@ Gw
>>                     </view>
>>                 </box>
>>                 <box title="On Screen Display" 
>> translatesAutoresizingMaskIntoConstraints="NO" id="2524">
>> -                    <rect key="frame" x="17" y="328" width="537" 
>> height="54"/>
>> +                    <rect key="frame" x="17" y="352" width="537" 
>> height="56"/>
>>                     <view key="contentView" id="GNq-vl-Idl">
>> -                        <rect key="frame" x="3" y="3" width="531" 
>> height="36"/>
>> +                        <rect key="frame" x="3" y="3" width="531" 
>> height="38"/>
>>                         <autoresizingMask key="autoresizingMask" 
>> widthSizable="YES" heightSizable="YES"/>
>>                         <subviews>
>>                             <button 
>> mirrorLayoutDirectionWhenInternationalizing="always" 
>> translatesAutoresizingMaskIntoConstraints="NO" id="2526">
>> -                                <rect key="frame" x="14" y="9" 
>> width="94" height="18"/>
>> +                                <rect key="frame" x="14" y="9" 
>> width="94" height="20"/>
>>                                 <buttonCell key="cell" type="check" 
>> title="Enable OSD" bezelStyle="regularSquare" imagePosition="left" 
>> alignment="left" inset="2" id="3494">
>>                                     <behavior key="behavior" 
>> changeContents="YES" doesNotDimImage="YES" lightByContents="YES"/>
>>                                     <font key="font" 
>> metaFont="system"/>
>> @@ -1795,13 +1796,13 @@ Gw
>>                     </view>
>>                 </box>
>>                 <box title="SPU language" 
>> translatesAutoresizingMaskIntoConstraints="NO" id="1aw-Yd-yzY">
>> -                    <rect key="frame" x="17" y="235" width="537" 
>> height="88"/>
>> +                    <rect key="frame" x="17" y="235" width="537" 
>> height="112"/>
>>                     <view key="contentView" id="clp-fm-5xs">
>> -                        <rect key="frame" x="3" y="3" width="531" 
>> height="70"/>
>> +                        <rect key="frame" x="3" y="3" width="531" 
>> height="94"/>
>>                         <autoresizingMask key="autoresizingMask" 
>> widthSizable="YES" heightSizable="YES"/>
>>                         <subviews>
>>                             <textField horizontalHuggingPriority="251" 
>> verticalHuggingPriority="750" 
>> translatesAutoresizingMaskIntoConstraints="NO" id="2529">
>> -                                <rect key="frame" x="16" y="43" 
>> width="175" height="17"/>
>> +                                <rect key="frame" x="16" y="67" 
>> width="175" height="17"/>
>>                                 <textFieldCell key="cell" 
>> lineBreakMode="truncatingTail" sendsActionOnEndEditing="YES" 
>> title="Preferred Subtitle Language" usesSingleLineMode="YES" id="3495">
>>                                     <font key="font" 
>> metaFont="system"/>
>>                                     <color key="textColor" 
>> name="controlTextColor" catalog="System" colorSpace="catalog"/>
>> @@ -1809,7 +1810,7 @@ Gw
>>                                 </textFieldCell>
>>                             </textField>
>>                             <textField verticalHuggingPriority="750" 
>> translatesAutoresizingMaskIntoConstraints="NO" id="2530">
>> -                                <rect key="frame" x="206" y="41" 
>> width="308" height="22"/>
>> +                                <rect key="frame" x="206" y="65" 
>> width="308" height="22"/>
>>                                 <textFieldCell key="cell" 
>> scrollable="YES" lineBreakMode="clipping" selectable="YES" 
>> editable="YES" continuous="YES" sendsActionOnEndEditing="YES" 
>> state="on" borderStyle="bezel" drawsBackground="YES" id="3496">
>>                                     <font key="font" 
>> metaFont="system"/>
>>                                     <color key="textColor" 
>> name="controlTextColor" catalog="System" colorSpace="catalog"/>
>> @@ -1821,7 +1822,7 @@ Gw
>>                                 </connections>
>>                             </textField>
>>                             <textField horizontalHuggingPriority="251" 
>> verticalHuggingPriority="750" 
>> translatesAutoresizingMaskIntoConstraints="NO" id="2531">
>> -                                <rect key="frame" x="16" y="14" 
>> width="175" height="17"/>
>> +                                <rect key="frame" x="16" y="38" 
>> width="175" height="17"/>
>>                                 <textFieldCell key="cell" 
>> lineBreakMode="truncatingTail" sendsActionOnEndEditing="YES" 
>> title="Default Encoding" usesSingleLineMode="YES" id="3497">
>>                                     <font key="font" 
>> metaFont="system"/>
>>                                     <color key="textColor" 
>> name="controlTextColor" catalog="System" colorSpace="catalog"/>
>> @@ -1829,7 +1830,7 @@ Gw
>>                                 </textFieldCell>
>>                             </textField>
>>                             <popUpButton verticalHuggingPriority="750" 
>> translatesAutoresizingMaskIntoConstraints="NO" id="2532">
>> -                                <rect key="frame" x="204" y="9" 
>> width="313" height="25"/>
>> +                                <rect key="frame" x="204" y="33" 
>> width="313" height="25"/>
>>                                 <popUpButtonCell key="cell" 
>> type="push" bezelStyle="rounded" lineBreakMode="truncatingTail" 
>> state="on" borderStyle="borderAndBezel" inset="2" 
>> arrowPosition="arrowAtCenter" preferredEdge="maxY" selectedItem="2665" 
>> id="3498">
>>                                     <behavior key="behavior" 
>> lightByBackground="YES" lightByGray="YES"/>
>>                                     <font key="font" metaFont="menu"/>
>> @@ -1848,15 +1849,27 @@ Gw
>>                                     <action 
>> selector="osdSettingChanged:" target="-2" id="xeg-0v-Ab2"/>
>>                                 </connections>
>>                             </popUpButton>
>> +                            <button verticalHuggingPriority="750" 
>> translatesAutoresizingMaskIntoConstraints="NO" id="epz-hm-jAM">
>> +                                <rect key="frame" x="16" y="10" 
>> width="193" height="18"/>
>> +                                <buttonCell key="cell" type="check" 
>> title="Try to auto-detect encoding" bezelStyle="regularSquare" 
>> imagePosition="left" state="on" inset="2" id="7Mx-so-6d6">
>> +                                    <behavior key="behavior" 
>> changeContents="YES" doesNotDimImage="YES" lightByContents="YES"/>
>> +                                    <font key="font" 
>> metaFont="system"/>
>> +                                </buttonCell>
>> +                                <connections>
>> +                                    <action 
>> selector="osdSettingChanged:" target="-2" id="mAn-id-Yn1"/>
>> +                                </connections>
>> +                            </button>
>>                         </subviews>
>>                         <constraints>
>>                             <constraint firstItem="2529" 
>> firstAttribute="centerY" secondItem="2530" secondAttribute="centerY" 
>> id="HsK-i9-n2z"/>
>> -                            <constraint firstAttribute="bottom" 
>> secondItem="2532" secondAttribute="bottom" constant="12" 
>> id="LX5-YD-7vT"/>
>> +                            <constraint firstAttribute="bottom" 
>> secondItem="epz-hm-jAM" secondAttribute="bottom" constant="12" 
>> id="LX5-YD-7vT"/>
>>                             <constraint firstAttribute="trailing" 
>> secondItem="2530" secondAttribute="trailing" constant="17" 
>> id="NTc-Wd-fKW"/>
>>                             <constraint firstItem="2529" 
>> firstAttribute="leading" secondItem="clp-fm-5xs" 
>> secondAttribute="leading" constant="18" id="Q4r-4X-sJQ"/>
>>                             <constraint firstItem="2530" 
>> firstAttribute="leading" secondItem="2532" secondAttribute="leading" 
>> id="R1y-rp-flP"/>
>>                             <constraint firstItem="2532" 
>> firstAttribute="leading" secondItem="2531" secondAttribute="trailing" 
>> constant="17" id="Upg-N7-CHN"/>
>>                             <constraint firstItem="2530" 
>> firstAttribute="width" secondItem="2532" secondAttribute="width" 
>> id="VfS-JB-gci"/>
>> +                            <constraint firstItem="epz-hm-jAM" 
>> firstAttribute="top" secondItem="2531" secondAttribute="bottom" 
>> constant="12" id="aws-1g-8oU"/>
>> +                            <constraint firstItem="epz-hm-jAM" 
>> firstAttribute="leading" secondItem="2531" secondAttribute="leading" 
>> id="dZ3-mh-SN1"/>
>>                             <constraint firstItem="2531" 
>> firstAttribute="centerY" secondItem="2532" secondAttribute="centerY" 
>> id="lPF-fI-ol6"/>
>>                             <constraint firstItem="2529" 
>> firstAttribute="leading" secondItem="2531" secondAttribute="leading" 
>> id="vCh-Cd-x9H"/>
>>                             <constraint firstItem="2529" 
>> firstAttribute="top" secondItem="clp-fm-5xs" secondAttribute="top" 
>> constant="10" id="vT7-e0-BpD"/>
>> diff --git a/modules/gui/macosx/preferences/VLCSimplePrefsController.h 
>> b/modules/gui/macosx/preferences/VLCSimplePrefsController.h
>> index b7d0b49141..1dc0e73fb0 100644
>> --- a/modules/gui/macosx/preferences/VLCSimplePrefsController.h
>> +++ b/modules/gui/macosx/preferences/VLCSimplePrefsController.h
>> @@ -157,6 +157,7 @@
>> @property (readwrite, weak) IBOutlet NSButton *osd_forceboldCheckbox;
>> @property (readwrite, weak) IBOutlet NSBox *osd_osdBox;
>> @property (readwrite, weak) IBOutlet NSButton *osd_osdCheckbox;
>> + at property (readwrite, weak) IBOutlet NSButton 
>> *osd_encodingAutoDetectCheckbox;
>> 
>> // video pane
>> @property (readwrite, strong) IBOutlet NSView *videoView;
>> diff --git a/modules/gui/macosx/preferences/VLCSimplePrefsController.m 
>> b/modules/gui/macosx/preferences/VLCSimplePrefsController.m
>> index 26d9589022..24295f1854 100644
>> --- a/modules/gui/macosx/preferences/VLCSimplePrefsController.m
>> +++ b/modules/gui/macosx/preferences/VLCSimplePrefsController.m
>> @@ -714,6 +714,7 @@ static inline const char * 
>> __config_GetLabel(vlc_object_t *p_this, const char *p
>> 
>>     [self setupButton:_osd_encodingPopup forStringList: 
>> "subsdec-encoding"];
>>     [self setupField:_osd_langTextField forOption: "sub-language" ];
>> +    [self setupButton:_osd_encodingAutoDetectCheckbox forBoolValue: 
>> "sub-autodetect-charset"];
>> 
>>     [self setupField:_osd_fontTextField forOption: "freetype-font"];
>>     [self setupButton:_osd_font_colorPopup forIntList: 
>> "freetype-color"];
>> @@ -1009,6 +1010,7 @@ static inline void save_string_list(intf_thread_t 
>> * p_intf, id object, const cha
>>             config_PutPsz("subsdec-encoding", "");
>> 
>>         config_PutPsz("sub-language", [[_osd_langTextField 
>> stringValue] UTF8String]);
>> +        config_PutInt("sub-autodetect-charset", 
>> [_osd_encodingAutoDetectCheckbox state]);
>> 
>>         config_PutPsz("freetype-font", [[_osd_fontTextField 
>> stringValue] UTF8String]);
>>         SaveIntList(_osd_font_colorPopup, "freetype-color");
>> diff --git a/modules/gui/qt/components/simple_preferences.cpp 
>> b/modules/gui/qt/components/simple_preferences.cpp
>> index 914b92f07d..11556f38f9 100644
>> --- a/modules/gui/qt/components/simple_preferences.cpp
>> +++ b/modules/gui/qt/components/simple_preferences.cpp
>> @@ -851,6 +851,7 @@ SPrefsPanel::SPrefsPanel( intf_thread_t *_p_intf, 
>> QWidget *_parent,
>>                             encoding );
>>             CONFIG_GENERIC( "sub-language", String, ui.subLangLabel,
>>                             preferredLanguage );
>> +            CONFIG_BOOL( "sub-autodetect-charset", subDetectCharset );
>> 
>>             CONFIG_GENERIC( "freetype-rel-fontsize", IntegerList,
>>                             ui.fontSizeLabel, fontSize );
>> diff --git a/modules/gui/qt/ui/sprefs_subtitles.ui 
>> b/modules/gui/qt/ui/sprefs_subtitles.ui
>> index b9e851dc9d..12ab2d56d5 100644
>> --- a/modules/gui/qt/ui/sprefs_subtitles.ui
>> +++ b/modules/gui/qt/ui/sprefs_subtitles.ui
>> @@ -136,6 +136,13 @@
>>            </property>
>>           </widget>
>>          </item>
>> +         <item row="2" column="0">
>> +           <widget class="QCheckBox" name="subDetectCharset">
>> +             <property name="text">
>> +               <string>Try to auto-detect encoding</string>
>> +             </property>
>> +           </widget>
>> +         </item>
>>         </layout>
>>        </widget>
>>       </item>
>> -- 
>> 2.20.1
>> 
>> _______________________________________________
>> 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 <https://mailman.videolan.org/listinfo/vlc-devel>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20190408/e02dfed0/attachment.html>


More information about the vlc-devel mailing list