[pulseaudio-discuss] [PATCH 7/7] format: Add API to query a property's type

[Arun Raghavan]

On Tue, 2012-02-07 at 10:26 +0100, David Henningsson wrote:
> On 02/07/2012 09:20 AM, Arun Raghavan wrote:
> > Since a given property can be single-valued, an array or (in the case of
> > ints) a range, clients need an API to figure out what type of value a
> > property holds. This adds such an API. The actual property type
> > enumeration is kept in the PA_PROP_* namespace and not the
> > PA_FORMAT_INFO* namespace so that it can later be reused for properties
> > generically if required.
> > ---
> >   src/map-file            |    1 +
> >   src/pulse/format.c      |   74 +++++++++++++++++++++++++++++++++++++++++++++++
> >   src/pulse/format.h      |   24 +++++++++++++++
> >   src/tests/format-test.c |    5 +++
> >   4 files changed, 104 insertions(+), 0 deletions(-)
> >
> > diff --git a/src/map-file b/src/map-file
> > index ee3d31d..5730fdf 100644
> > --- a/src/map-file
> > +++ b/src/map-file
> > @@ -168,6 +168,7 @@ pa_format_info_free;
> >   pa_format_info_free2;
> >   pa_format_info_from_string;
> >   pa_format_info_from_sample_spec;
> > +pa_format_info_get_prop_type;
> >   pa_format_info_get_prop_int;
> >   pa_format_info_get_prop_int_range;
> >   pa_format_info_get_prop_int_array;
> > diff --git a/src/pulse/format.c b/src/pulse/format.c
> > index 6176846..aee3dfc 100644
> > --- a/src/pulse/format.c
> > +++ b/src/pulse/format.c
> > @@ -280,6 +280,80 @@ int pa_format_info_to_sample_spec_fake(pa_format_info *f, pa_sample_spec *ss) {
> >       return 0;
> >   }
> >
> > +pa_prop_type_t pa_format_info_get_prop_type(pa_format_info *f, const char *key) {
> > +    const char *str;
> > +    json_object *o, *o1;
> > +    pa_prop_type_t type;
> > +
> > +    pa_assert(f);
> > +    pa_assert(key);
> > +
> > +    str = pa_proplist_gets(f->plist, key);
> > +    if (!str)
> > +        return PA_PROP_TYPE_INVALID;
> > +
> > +    o = json_tokener_parse(str);
> > +    if (is_error(o))
> > +        return PA_PROP_TYPE_INVALID;
> > +
> > +    switch (json_object_get_type(o)) {
> > +        case json_type_int:
> > +            type = PA_PROP_TYPE_INT;
> > +            break;
> > +
> > +        case json_type_string:
> > +            type = PA_PROP_TYPE_STRING;
> > +            break;
> > +
> > +        case json_type_array:
> > +            if (json_object_array_length(o) == 0) {
> > +                /* Unlikely, but let's account for this anyway. We need at
> > +                 * least one element to figure out the array type. */
> > +                type = PA_PROP_TYPE_INVALID;
> > +                break;
> > +            }
> > +
> > +            o1 = json_object_array_get_idx(o, 1);
> > +
> > +            if (json_object_get_type(o1) == json_type_int)
> > +                type = PA_PROP_TYPE_INT_ARRAY;
> > +            else if (json_object_get_type(o1) == json_type_string)
> > +                type = PA_PROP_TYPE_STRING_ARRAY;
> > +            else
> > +                type = PA_PROP_TYPE_INVALID;
> > +
> > +            json_object_put(o1);
> > +            break;
> > +
> > +        case json_type_object:
> > +            /* We actually know at this point that it's a int range, but let's
> > +             * confirm. */
> > +            o1 = json_object_object_get(o, PA_JSON_MIN_KEY);
> > +            if (!o1) {
> > +                type = PA_PROP_TYPE_INVALID;
> > +                break;
> > +            }
> > +            json_object_put(o1);
> > +
> > +            o1 = json_object_object_get(o, PA_JSON_MIN_KEY);
> 
> Shouldn't this be PA_JSON_MAX_KEY - and if it should, how come that the 
> test succeed anyway? (Assuming you have actually tested that it does 
> before submitting?)

The test succeeds because we're testing the same thing twice in this
code, so it's true both times. Crafting a case that causes this test to
fail would be quite painful (the actual condition for the range case. as
I've documented, is more out of paranoia than actual necessity).

-- Arun