On Tue, Nov 20, 2012 at 4:59 PM, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote:
> On Tue, Nov 20, 2012 at 04:37:13PM +0200, Zeeshan Ali (Khattak) wrote:
>> On Tue, Nov 20, 2012 at 12:02 PM, Christophe Fergeau
>> <cfergeau@xxxxxxxxxx> wrote:
>> >> +
>> >> +/* 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?
>
> Define "common image formats" ? ;) is JPEG2000 common?
JPEG, GIF, PNG and BMP are the common formats.
> What about BMP or
> PCX?
BMP is, PCX, not.
>Can we make this a GStrv or a GList or GArray, ... ?
That will not be as easy to implement as entity params can't be lists
right now. I think regex does the trick.
>> >> + /**
>> >> + * 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.
>
>
> Well, you mentioned alpha, and then R, G, B. If bit depth is 32, are we
> supposed to be using ARGB, RGBA, BGRA. ... ?
Who is we? libosinfo doesn't need any of this info, its the apps that needs it.
> If the only thing this hints
> at is whether the avatar should have an alpha channel or not, a boolean
> would probably be better, especially as the image format used will probably
> encode the image bit depth, channel order, ... for us, so we probably don't
> really need to care about it.
We don't need to care about it anyways. :) But yeah, i guess that
makes more sense. We can add this depth (actually, it should be named
max-depth) property if needed later.
--
Regards,
Zeeshan Ali (Khattak)
FSF member#5124
