#115 SIGSEGV in title compositing

trunk
closed
Dan Dennedy
framework (5)
5
2014-06-30
2011-05-26
jtlebi
No

When rendering a video containing at least two consecutive titles using an image and a key-framed fade-in, fade-out, melt crashes in libwscale passing a NULL pointer.

This specific "NULL pointer not checked" stuff has already been reported and fixed in ffmpeg but the original cause still remains.

It appears that the root of this issue takes place in the function "mlt_geometry_parse_item" of file "src/framework/mlt_geometry.c". This is the code in charge of parsing a geometry instruction. It uses the function "strtod" which is **local dependant**. In this function, it is assumed that the decimal separator is a "." whereas it is a "," in french. The "," is also a field separator and it causes the parser to read weird values :D

Adding "setlocale(LC_NUMERIC, "C");" at the beginning of the function appears to be a quite good workaround at the moment. This is the one i am currently using.

Anyway, i still have a silly questions:
=> why does this bug appears only on the SECOND title using an image ???

Note that fixing the sigsegv in the libwscale code helps the video the play, without the title using images but all the other following titles are fine !
=> Is it possible that the local is changed at some point in the program ?

Discussion

  • jtlebi
    jtlebi
    2011-05-26

    It appears that the real bug is
    => data parsing is localized. Should be fixed to "C" or "POSIX"

    For example, running melt with LC_NUMERIC=C solves all issues, including aspect ratio reading from xml bug that i didn't notice before...

    i will attach a partial fix to this report ASAP but it solves only the title issue.

     
  • jtlebi
    jtlebi
    2011-05-26

    title issue fix. current git head

     
    Attachments
  • Dan Dennedy
    Dan Dennedy
    2011-05-26

    Perhaps the problem is that you are using a comma as an item separator in a numeric locale that uses it for a decimal point. I do not consider the locale support to be a bug. The geometry parsing code is not specifically looking for a comma. The only special token it looks for is ";" and the usage of commas in the function description and demos is just for example, and perhaps something safer should be chosen such as "|". I specifically avoided ":" in that statement and will do so in future revised examples because that will be used in future support for timecode values.

     
  • jtlebi
    jtlebi
    2011-05-26

    We completely agree on that point. MLT is not supposed to support locales. Indeed, this prevent files from being shared by people having different locales...

    I double checked, all the fields are well formed and the decimal separator is "dot", not a "comma".

    The fact is that the function strtod on which geometry parsing relies is locale dependent. This is the reason why i suggested to force the local to "C" or "POSIX" for numerical values. This would ensure MLT is fully local independent.

    I can try to provide you with an example file if you wish.

    Btw, the file was generated by kdenlive and the localization problem also appears in the "aspect ratio" field for instance.

    An even better fix is to run the client application with 'LC_NUMERIC="C" ' as a command line prefix.

     
  • Dan Dennedy
    Dan Dennedy
    2011-05-26

    > MLT is not supposed to support locales.

    Maybe you misunderstood. Why should MLT should break the locale support built into libc? I consider adding hard-coded setlocale() in the code an improper solution. If your locale requires comma as the decimal separator, then you should use that in the values you supply to MLT, not a dot.

    > Indeed, this prevent files from being shared by people having different
    locales...

    I currently have a lot higher priority concerns with MLT than people with different locales sharing files. Maybe a solution to that problem is the addition of locale attributes in the XML, but I am not going to work on that soon, and this capability must suffer until myself or someone contributes a proper fix.

    > I double checked, all the fields are well formed and the decimal separator is "dot", not a "comma".

    So what is the problem? That tells me nothing; I do not know your locale. Is dot a good or bad thing?

     
  • Dan Dennedy
    Dan Dennedy
    2011-07-17

    I added some special things for locale support in the latest release including changing all examples to use '/' instead of comma to separate X and Y. Kdenlive is making changes too. I do not yet know if your specific problem is fixed. Certainly old projects that lack a LC_NUMERIC attribute in the XML will not work. But new projects should have a LC_NUMERIC attribute in the root mlt element of the XML, and hopefully Kdenlive stops using comma as a separator in the mlt_geometry values.

     
  • Dan Dennedy
    Dan Dennedy
    2014-06-30

    • status: open --> closed