[Ardour-Dev] Feedback on string-convert branch

[Paul Davis]

I think a straightforward way to think about this branch is:

   * it fixes a number of current issues
   * it fixes a number of potential future issues (by preventing them from
happening by accident)
   * it doesn't fix all problems associated with i18n
   * it appears to have somewhat improved performance

Those alone suggest the branch is worth merging/rebasing. So what remains
is:

   * is there anything that the changes in this branch will make worse, in
terms of:
          - complexity
          - performance
          - forward-compatibility
          - back-compatibility
          - maintainance

>From where I sit (or stand), the answer to this question seems to be "no".
If that's our final answer, we should merge.


On Fri, Jan 6, 2017 at 2:13 PM, Tim Mayberry <mojofunk at gmail.com> wrote:

> On Fri, Jan 6, 2017 at 12:36 AM, Robin Gareus <robin at gareus.org> wrote:
>
>> On 01/05/2017 12:12 AM, Tim Mayberry wrote:
>> > On Thu, Jan 5, 2017 at 7:14 AM, Paul Davis <paul at linuxaudiosystems.com>
>> > wrote:
>> >
>> >>
>> >>
>> >> On Wed, Jan 4, 2017 at 12:52 PM, Tim Mayberry <mojofunk at gmail.com>
>> wrote:
>> >>
>> >>> I've pushed a branch called string-convert to the main repository that
>> >>> I'd like
>> >>> some feedback on from the other developers when they get a chance. It
>> >>> represents a fair bit of work over the last year or so and is attempt
>> to
>> >>> make
>> >>> improvements to the de/serialization code in Ardour.
>> >>>
>> >>
>> >> I still don't understand how this is supposed to work with plugin
>> data. If
>> >> you leave the global locale setting untouched, how do we know what to
>> do
>> >> when reloading plugin data across two systems where the global locale
>> may
>> >> be different?
>> >>
>> >>
>> >
>> > Of the 100 or so LocaleGuard's in master, the 10 or so that remain are
>> in
>> > the Plugin get/set_state methods, which is why I mentioned removing
>> > LocaleGuard's that guard internal string conversion.
>>
>> Since changing the locale is expensive, so we should also retain a
>> single LocaleGuard in save_state() rather than switch forth/back in
>> every Plugin get/set state.
>>
>> With some luck, plugins don't change the locale and every save only
>> needs to change the locale once.
>>
>>
> And then remove all the other obvious LocaleGuards in the Session state
> call tree? That is an option for that particular issue I guess.
>
>
>>
>> > That some plugins depend on a specific global locale for correct
>> operation
>> > or worse modify the global locale is dissappointing but seems to be a
>> > reality and it is accounted for in the branch.
>>
>> ..and it's worse. Some plugins expect to also *run* in the "C" or en_US
>> locale. e.g. NI Kontakt fails in numeric-locales that use a comma, and
>> NI isn't alone.
>>
>>
>> If we add a custom system, I think it's better to
>>
>>  - don't change the locale at all, keep using the default "C" locale for
>> C++/std::locale
>>
>>  - keep relying on standard functions for string-conversion
>>
>>
> I'm not sure I understand, this is how it is currently and does not really
> address any of the issues I raised, so I'd be interested in hearing why you
> think it is better than what I have proposed.
>
> If you keep the current method of serialization, you would still need to
> identify and individually fix the many incorrect numeric conversions that I
> have fixed in the branch (which may not be too hard as I had to find/audit
> them and most are mentioned in the commit log AFAIR) but that really only
> fixes the current issues with round trip equality. It doesn't address the
> other issues and it doesn't help to reduce incorrect serialization in the
> future. Part of the intent with changing the API's rather than just fixing
> all the individual issues is to fix it once and in the future all
> serialization is correct by default/automatically.
>
>
>> and instead
>>
>>  - come up with a custom message-translation system that does not depend
>> on the std::locale at the time the message is formatted.
>>
>>
> I'm not sure what you mean here either. Are you referring to message
> translation i.e gettext or numeric conversion via
> snprintf/sscanf/iostream/etc?
>
> Tim
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ardour.org/pipermail/ardour-dev-ardour.org/attachments/20170109/cd709563/attachment-0001.htm>