Un bug particulier avec Enna/libplayer

Hello,

comme je l’ai plus d’une fois expliqué, la vidéo sous Enna passe par libplayer. Et depuis quelque temps, Ben et Nico m’ont à nouveau sensibilisé sur un problème d’aspect avec les vidéos.
Le plus simple des aspects correspond à une stupide fraction:

\textbf{aspect}=\frac{\textbf{largeur}}{\textbf{hauteur}}

Mais celle-ci n’est pas toujours vraie, ainsi l’aspect peut être directement enregistré en tant que propriété dans le fichier vidéo. Un exemple courant étant les films DVD. La définition de l’image est de 720×576 (PAL) néanmoins l’aspect peut être du 4:3, 16:9 ou du cinémascope 2.39:1. Pourtant l’image est toujours enregistrée afin d’utiliser un maximum de surface et donc les pixels ne sont pas carrés. Considérer le pixel carré est le dernier recours au cas où la vidéo ne donnerait aucune information sur son aspect.

Le bug…

EnnaLe problème rencontré part Ben et Nico n’était pas reproductible chez moi. Et étant principalement responsable de libplayer depuis toutes les modifications que j’ai apporté, et donc connaissant de fond en comble sa structure et son fonctionnement actuel, il est forcément de mon devoir que de corriger les problèmes connus.
Quand ils utilisaient libplayer avec Enna, ils avaient certaines vidéos qui s’affichaient avec un aspect de 1.00. Ce qui engendre donc une image parfaitement carrée. S’ils testaient cette même vidéo avec le logiciel test-player qui utilise libplayer, l’aspect était correcte. Là où ça devenait encore plus étrange, c’est quand j’ai testé la même vidéo que Nico et j’avais un aspect correct aussi bien sous Enna que sous test-player. Je ne savais donc pas comment reproduire le bug, même que j’avais de plus en plus de doute sur l’existence de ce bug. Non pas que je n’ai pas confiance en leurs manipulations (surtout qu’ils sont deux à être victime de ce problème) mais plutôt que je fais toujours une grande quantité de testes lorsque j’ajoute ou modifie des fonctionnalités dans la bibliothèque. Les problèmes d’aspect je les ai particulièrement travaillé quand j’ai rajouté le support de la navigation DVD pour les wrappers MPlayer et xine. Car il y a des conversions de coordonnées à effectuer pour que la souris agissent au bon endroit sur la surface de l’image et cela en tenant compte de l’aspect et des offsets en x et y.

Nico m’a alors transmis les logs Enna lorsque le problème survient et j’ai pu identifier le problème indirectement. libplayer écrit dans ses logs certaines informations sur les modifications qu’il effectue sur la fenêtre vidéo. Il apparait que la ligne qui indique l’aspect de l’image utilisait une virgule pour séparer les décimales. Sur mon ordinateur cette même ligne ne s’écrit pas avec une virgule, mais avec un point.

Un problème de LOCALE

libplayer utilise abondamment la fonction atof() pour la conversion des valeurs réelles transmissent par MPlayer. Une de ces valeurs est l’aspect. MPlayer retourne la valeur dans un champ ID_VIDEO_ASPECT=1.85 par exemple. Et ce champ a toujours un point comme délimiteur dans le cas d’un MPlayer utilisé avec libplayer (MPlayer doit toujours être en anglais). Là où ça devient un problème c’est le comportement de la fonction atof(). En fonction de la LOCALE du système, elle considère que le délimiteur est une virgule et non un point. De ce fait, (et surtout parce que la fonction atof() ne peut pas échouer) l’aspect lu n’était pas 1.85, mais 1. atof() ignorait tous les caractères depuis le point.

Bien qu’étant de langue francophone, en Suisse-Romande nous n’avons pas du tout la même LOCALE que la France. Notre clavier est également différent (disposition QWERTZ). Nous utilisons le point comme séparateur de décimale et non la virgule et la LOCALE est “fr_CH”. En France, la LOCALE est “fr_FR” et le séparateur de décimale est une virgule. Ceci explique pourquoi je ne pouvais pas reproduire l’erreur. Qui pense aux LOCALEs quand il y a un bug sur l’aspect des vidéos? Et bien je tâcherais d’y penser à partir de maintenant.
Quoi qu’il en soit, de nombreux français devaient avoir des problème, mais je n’ai pas eu d’autres retours que les autres développeurs. Je suppose que les éventuels utilisateurs d’Enna se sont dit qu’étant donné qu’aucune version stable existe, ça finirait par être fixé pour la finale? Sachez qu’il y a un site où vous pouvez rapporter les problèmes: enna.geexbox.org, vérifiez si le bug est déjà listé et dans le cas contraire n’hésitez pas à vous inscrire et à soumettre un rapport de bug.

Correction

Pour y remédier il suffit donc de changer de LOCALE avec libplayer. Mais il n’y a pas de raison apparente de changer la LOCALE pour toute la bibliothèque. Ainsi il suffit de réécrire une fonction atof() qui s’exécute dans une autre LOCALE. En principe un programme utilise la LOCALE “C”. Qui est celle par défaut. Enna étant internationalisé, la LOCALE change selon la langue. Ceci explique donc pourquoi le bug apparaissait sous Enna mais pas sous test-player.

En principe, la LOCALE se change avec la fonction setlocale(). Mais avant de dire youpie il faut garder à l’esprit que libplayer utilise abondamment les threads. Lorsque la fonction atof() est appelée, Enna peut être en train de faire autre chose. Et cet autre chose peut très bien être influencé par la LOCALE en cours. Le fait d’utiliser la fonction setlocale() modifie la LOCALE pour tout le processus.
Il est donc nécessaire d’avoir recours à une fonction qui ne modifie la LOCALE que sur le thread où atof() est exécuté. Cette fonction s’appelle uselocale(), le nouveau atof() se présente donc ainsi (patch libplayer):

#define _GNU_SOURCE
#include <locale.h>
#include <stdlib.h>

double
my_atof (const char *nptr)
{
  double res;
  locale_t new_locale, prev_locale;

  new_locale = newlocale (LC_NUMERIC_MASK, "C", NULL);
  prev_locale = uselocale (new_locale);
  res = atof (nptr);
  uselocale (prev_locale);
  freelocale (new_locale);

  return res;
}

Afin de simplifier la fonction et d’éviter atof() qui a une réputation d’obsolescence je l’ai remplacé par strtod_l(). Du même coup il est possible d’éliminer uselocale() et donc de ne plus changer la LOCALE du thread.

#define _GNU_SOURCE
#include <locale.h>
#include <stdlib.h>

double
my_atof (const char *nptr)
{
  double res;
  locale_t new_locale;

  new_locale = newlocale (LC_NUMERIC_MASK, "C", NULL);
  res = strtod_l (nptr, NULL, new_locale);
  freelocale (new_locale);

  return res;
}

On notera que strtod() peut retourner HUGE_VAL contrairement à atof().

A bientôt,

Mathieu SCHROETER

2 thoughts on “Un bug particulier avec Enna/libplayer

  1. Salut

    Heu juste comme ça, s’il y a plusieurs threads je pense que tu devrais rajouter un lock.
    Sinon, plusieurs threads pourraient modifier la locale simultanément.
    Je pense que le plus simple et le plus propre serait de réécrire atof. Ce n’est pas bien compliqué et il y a pleins d’exemples sur le net. Au moins tu sera sur que le séparateur est un point.
    Ce n’est pas toujours idéal de modifier l’environnement utilisateur comme la locale, le path, etc…

    just my 2 cents

    cyberic.

  2. Salut,
    justement, je n’utilise pas setlocale() qui agit globalement mais uselocale() qui agit uniquement sur le thread. Il peut y avoir donc 50 threads qui font la manipulation en même temps car il y aura 50 LOCALEs différentes “créées”. Le système est donc sûre. uselocale() par contre n’est pas standard.
    Et atof() n’est plus MT-Safe uniquement si on utilise setlocale(), ce qui n’est pas le cas ici. Le lock est donc inutile.

    EDIT: J’ai réécris l’article avec également la solution via strtod_l() qui a le mérite d’éviter le changement de LOCALE du thread.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google photo

You are commenting using your Google account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s