[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [virt-tools-list] [libosinfo] API to query required user avatar format



On Tue, Nov 20, 2012 at 12:02 PM, Christophe Fergeau
<cfergeau redhat com> wrote:
> On Thu, Nov 15, 2012 at 12:44:47AM +0200, Zeeshan Ali (Khattak) wrote:
>> From: "Zeeshan Ali (Khattak)" <zeeshanak gnome org>
>>
>> Add a new API for apps to be able to query in which format do the user
>> avatar file need to be in.
>>
>> Based on a patch from Fabiano FidĂȘncio <fabiano fidencio org>.
>> ---
>>  data/install-scripts/windows-cmd.xml |   6 +
>>  data/schemas/libosinfo.rng           |  30 +++++
>>  osinfo/Makefile.am                   |   2 +
>>  osinfo/libosinfo.syms                |   8 +-
>>  osinfo/osinfo.h                      |   1 +
>>  osinfo/osinfo_avatar_format.c        | 250 +++++++++++++++++++++++++++++++++++
>>  osinfo/osinfo_avatar_format.h        |  95 +++++++++++++
>>  osinfo/osinfo_install_script.c       |  53 ++++++++
>>  osinfo/osinfo_install_script.h       |   6 +
>>  osinfo/osinfo_loader.c               |  44 ++++++
>>  po/POTFILES.in                       |   1 +
>>  11 files changed, 495 insertions(+), 1 deletion(-)
>>  create mode 100644 osinfo/osinfo_avatar_format.c
>>  create mode 100644 osinfo/osinfo_avatar_format.h
>>
>> diff --git a/data/install-scripts/windows-cmd.xml b/data/install-scripts/windows-cmd.xml
>> index 020a493..3c6ba23 100644
>> --- a/data/install-scripts/windows-cmd.xml
>> +++ b/data/install-scripts/windows-cmd.xml
>> @@ -12,6 +12,12 @@
>>        <param name="target-disk" policy="optional"/>
>>        <param name="script-disk" policy="optional"/>
>>      </config>
>> +    <avatar-format>
>> +        <mime-type>image/bmp</mime-type>
>> +        <depth>24</depth>
>> +        <width>48</width>
>> +        <height>48</height>
>> +    </avatar-format>
>>      <template>
>>        <xsl:stylesheet
>>          xmlns:xsl="http://www.w3.org/1999/XSL/Transform";
>> diff --git a/data/schemas/libosinfo.rng b/data/schemas/libosinfo.rng
>> index 3c57c36..c27c680 100644
>> --- a/data/schemas/libosinfo.rng
>> +++ b/data/schemas/libosinfo.rng
>> @@ -478,6 +478,9 @@
>>            </element>
>>          </optional>
>>          <optional>
>> +          <ref name='avatar-format'/>
>> +        </optional>
>> +        <optional>
>>            <element name='config'>
>>              <oneOrMore>
>>                <element name='param'>
>> @@ -509,6 +512,33 @@
>>      </element>
>>    </define>
>>
>> +  <define name='avatar-format'>
>> +    <element name='avatar-format'>
>> +      <interleave>
>> +        <optional>
>> +          <element name="mime-type">
>> +            <text/>
>> +          </element>
>> +        </optional>
>> +        <optional>
>> +          <element name="width">
>> +            <ref name='num'/>
>> +          </element>
>> +        </optional>
>> +        <optional>
>> +          <element name="height">
>> +            <ref name='num'/>
>> +          </element>
>> +        </optional>
>> +        <optional>
>> +          <element name="depth">
>> +            <ref name='num'/>
>> +          </element>
>> +        </optional>
>> +      </interleave>
>> +    </element>
>> +  </define>
>> +
>>    <define name="customElement">
>>      <element>
>>        <anyName/>
>> diff --git a/osinfo/Makefile.am b/osinfo/Makefile.am
>> index abaa78c..e85adb7 100644
>> --- a/osinfo/Makefile.am
>> +++ b/osinfo/Makefile.am
>> @@ -55,6 +55,7 @@ libosinfo_1_0_includedir = $(includedir)/libosinfo-1.0/osinfo
>>
>>  libosinfo_1_0_include_HEADERS = \
>>    osinfo.h                   \
>> +  osinfo_avatar_format.h     \
>>    osinfo_db.h                        \
>>    osinfo_loader.h            \
>>    osinfo_device.h            \
>> @@ -88,6 +89,7 @@ libosinfo_1_0_include_HEADERS = \
>>    $(NULL)
>>
>>  libosinfo_1_0_la_SOURCES =   \
>> +  osinfo_avatar_format.c     \
>>    osinfo_entity.c            \
>>    osinfo_enum_types.c                \
>>    osinfo_filter.c            \
>> diff --git a/osinfo/libosinfo.syms b/osinfo/libosinfo.syms
>> index de33a70..271e3d7 100644
>> --- a/osinfo/libosinfo.syms
>> +++ b/osinfo/libosinfo.syms
>> @@ -317,6 +317,12 @@ LIBOSINFO_0.2.1 {
>>
>>  LIBOSINFO_0.2.2 {
>>      global:
>> +     osinfo_avatar_format_get_type;
>> +     osinfo_avatar_format_get_mime_type;
>> +     osinfo_avatar_format_get_width;
>> +     osinfo_avatar_format_get_height;
>> +     osinfo_avatar_format_get_depth;
>> +
>>       osinfo_install_config_param_policy_get_type;
>>       osinfo_media_error_get_type;
>>       osinfo_product_relationship_get_type;
>> @@ -336,10 +342,10 @@ LIBOSINFO_0.2.2 {
>>       osinfo_install_config_get_script_disk;
>>       osinfo_install_config_set_script_disk;
>>
>> +     osinfo_install_script_get_avatar_format;
>>       osinfo_install_script_get_path_format;
>>  } LIBOSINFO_0.2.1;
>>
>> -
>>  /* Symbols in next release...
>>
>>    LIBOSINFO_0.0.2 {
>> diff --git a/osinfo/osinfo.h b/osinfo/osinfo.h
>> index 81ed1cc..559eaa8 100644
>> --- a/osinfo/osinfo.h
>> +++ b/osinfo/osinfo.h
>> @@ -39,6 +39,7 @@
>>  #include <osinfo/osinfo_install_config.h>
>>  #include <osinfo/osinfo_install_config_param.h>
>>  #include <osinfo/osinfo_install_script.h>
>> +#include <osinfo/osinfo_avatar_format.h>
>>  #include <osinfo/osinfo_install_scriptlist.h>
>>  #include <osinfo/osinfo_productlist.h>
>>  #include <osinfo/osinfo_product.h>
>> diff --git a/osinfo/osinfo_avatar_format.c b/osinfo/osinfo_avatar_format.c
>> new file mode 100644
>> index 0000000..5f6a4e3
>> --- /dev/null
>> +++ b/osinfo/osinfo_avatar_format.c
>> @@ -0,0 +1,250 @@
>> +/*
>> + * libosinfo:
>> + *
>> + * Copyright (C) 2009-2012 Red Hat, Inc.
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>> + *
>> + * Authors:
>> + *   Fabiano FidĂȘncio <fabiano fidencio org>
>> + *   Zeeshan Ali (Khattak) <zeeshanak gnome org>
>> + */
>> +
>> +#include <config.h>
>> +
>> +#include <osinfo/osinfo.h>
>> +#include <glib/gi18n-lib.h>
>> +
>> +G_DEFINE_TYPE (OsinfoAvatarFormat, osinfo_avatar_format, OSINFO_TYPE_ENTITY);
>> +
>> +#define OSINFO_AVATAR_FORMAT_GET_PRIVATE(obj)  \
>> +        (G_TYPE_INSTANCE_GET_PRIVATE ((obj),                \
>> +         OSINFO_TYPE_AVATAR_FORMAT,            \
>> +         OsinfoAvatarFormatPrivate))
>> +
>> +#define DEFAULT_MIME_TYPE "image/png"
>> +
>> +/**
>> + * SECTION: osinfo_avatar_format
>> + * @short_description: The required format of avatar for an install script
>> + * @see_also: #OsinfoInstallScript
>> + */
>> +
>> +enum {
>> +    PROP_0,
>> +
>> +    PROP_MIME_TYPE,
>> +    PROP_WIDTH,
>> +    PROP_HEIGHT,
>> +    PROP_DEPTH,
>> +};
>> +
>> +static void
>> +osinfo_avatar_format_get_property(GObject *object,
>> +                               guint property_id,
>> +                                  GValue *value,
>> +                                  GParamSpec *pspec)
>> +{
>> +    OsinfoAvatarFormat *avatar = OSINFO_AVATAR_FORMAT (object);
>> +
>> +    switch (property_id)
>> +        {
>> +        case PROP_MIME_TYPE:
>> +        {
>> +            const gchar *mime_type;
>> +
>> +            mime_type = osinfo_avatar_format_get_mime_type(avatar);
>> +            g_value_set_string(value, mime_type);
>> +            break;
>> +        }
>> +        case PROP_WIDTH:
>> +            g_value_set_int(value,
>> +                            osinfo_avatar_format_get_width(avatar));
>> +            break;
>> +        case PROP_HEIGHT:
>> +            g_value_set_int(value,
>> +                            osinfo_avatar_format_get_height(avatar));
>> +            break;
>> +        case PROP_DEPTH:
>> +            g_value_set_int(value,
>> +                            osinfo_avatar_format_get_depth(avatar));
>> +            break;
>> +        default:
>> +            /* We don't have any other property... */
>> +            G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
>> +            break;
>> +        }
>> +}
>> +
>> +/* Init functions */
>> +static void
>> +osinfo_avatar_format_class_init (OsinfoAvatarFormatClass *klass)
>> +{
>> +    GObjectClass *g_klass = G_OBJECT_CLASS (klass);
>> +    GParamSpec *pspec;
>> +
>> +    g_klass->get_property = osinfo_avatar_format_get_property;
>> +
>> +    /**
>> +     * OsinfoAvatarFormat:mime-type:
>> +     *
>> +     * The required mime-type of the avatar.
>
>
> What if several formats are supported?

Hm.. didn't think of that. Usually I've seen either the image is
required to be in a particular format or there is no restrictions (all
common image formats are supported) so in case of later we an simply
return NULL? If we want to be very future safe, well this could be a
regex then?

>> +     **/
>> +    pspec = g_param_spec_string("mime-type",
>> +                                "MIME Type",
>> +                                _("The required mime-type of the avatar"),
>> +                                DEFAULT_MIME_TYPE,
>> +                                G_PARAM_READABLE |
>> +                                G_PARAM_STATIC_STRINGS);
>> +    g_object_class_install_property(g_klass,
>> +                                    PROP_MIME_TYPE,
>> +                                    pspec);
>> +
>> +    /**
>> +     * OsinfoAvatarFormat:width:
>> +     *
>> +     * The required width (in pixels) of the avatar.
>
> What does 'required' means here? That no other width will work?
> Installer are not able to scale the avatar?

Yes. If we want max/min width for some installer, we can add extra
props for those.

>> +     **/
>> +    pspec = g_param_spec_int("width",
>> +                             "Width",
>> +                             _("The required width (in pixels) of the avatar"),
>> +                             -1,
>> +                             G_MAXINT,
>> +                             -1,
>> +                             G_PARAM_READABLE |
>> +                             G_PARAM_STATIC_STRINGS);
>> +    g_object_class_install_property(g_klass,
>> +                                    PROP_WIDTH,
>> +                                    pspec);
>> +
>> +    /**
>> +     * OsinfoAvatarFormat:height:
>> +     *
>> +     * The required height (in pixels) of the avatar.
>> +     **/
>> +    pspec = g_param_spec_int("height",
>> +                             "Height",
>> +                             _("The required height (in pixels) of the avatar."),
>> +                             -1,
>> +                             G_MAXINT,
>> +                             -1,
>> +                             G_PARAM_READABLE |
>> +                             G_PARAM_STATIC_STRINGS);
>> +    g_object_class_install_property(g_klass,
>> +                                    PROP_HEIGHT,
>> +                                    pspec);
>> +
>> +    /**
>> +     * OsinfoAvatarFormat:depth:
>> +     *
>> +     * The required depth (in bits) of the avatar.
>
> Is that really required?

Yes. Check the changes to data/install-scripts/windows-cmd.xml in this
patch. It was a boolean 'alpha' prop in fidencio's original patch to
indicate whether alpha channel is allowed or not.

> I don't think such a property is really useful as
> the number of bits per pixel (I guess this is what this means?) is not
> expressive enough, you need to know the bits used by each component (R, G,
> B) as well as their bit position.

Is that a usual scenerio to have anything other the R, G and B? About
position of bits, I doubt we'll need to know that or have any
restrictions on those.

> I'd rather not have this property at all
> for now, and add it when it's really needed.

Thats now. :)

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]