[NUT-devel] info packets/frames

[Rich Felker]

On Thu, Feb 16, 2006 at 08:01:38AM +0200, Oded Shimon wrote:
> > > I dislike it. For this purpose, a pure bitmask with special case 0 is good 
> > > enough... It's not quite a complication but it is simply not worth it...
> > > (bitmask has the slight disadvantage of overflowing, but seriously, 64 
> > > streams... Worst pathological scenario I can think of is 30 streams, 20 
> > > subtitles and 10 audio tracks)
> > 
> > a limitation to 30-60 streams is not acceptable
> 
> It's just a implementation limitation, not a spec one, but anyway, see new 
> patch.

Agree. It's easier to have a custom v-reader/writer for arbitrary
length bitflags than to have some huge mechanism to use ordinary read
to 64bit integer types for this purpose.

> > > That's somewhat a good point, but still not excuse enough IMO to put the 
> > > info types in the main header, it literally defeats the purpose - info 
> > > stuff goes in info packets, not in main headers...
> > 
> > then my vote is to simply not allow extending the table, its not needed
> > anyway to add new fields to info packets
> > 
> > you either have a fixed table or you store the table in the file
> 
> Is the problem old programs not understanding new fields? It's not a big 
> issue IMO...

Yes, I thought it was an issue before, but then realized it's probably
not. In any case I'm against forbidding this in the design, in case we
decide later that there are important fields we want to add.

> Main difference in this patch is "type" is only for binary data. I couldn't 
> decide an elegant way to design the table to accomodate this though...

Design of the table is an _implementation_ issue.

>      [Note: strings MUST be encoded in UTF-8]
> +    [Note: strings MUST NOT be encoded with their terminating NUL]

IMO this should read "the character NUL (U+0000) is not legal within
or at the end of a string."

> +        code= id&3
> +        index= id>>2
> +        name= info_table[code][index][0]
> +        if(name==NULL) name             vb
> +        if(code==0) {
>              value                       s
> -        else
> +        } else if(code==1) {
> +            nom                         s
> +            denom                       v
> +            value= nom/denom
> +        } else if(code==2) {
> +            mantissa                    s
> +            exponent                    s
> +            value= mantissa*pow(2,exp)
> +        } else {
> +            if(index) type= info_table[code][index][1]
> +            else type                   vb
>              value                       vb
> +        }

OK IMO.

> +info_table[4][][2]={
> +    {
> +        {NULL            , "i"}, // integer
> +    },
> +    {
> +        {NULL            , "r"}, // rational
> +        {"TrackTime"     , "r"},
> +    },
> +    {
> +        {NULL            , "f"}, // float
> +    },

IMO this part is silly/misleading, since "i", "r", and "f" are not
"types".

> +    {
> +        {NULL            ,  NULL },
> +        {NULL            , "UTF8"},
> +        {"Author"        , "UTF8"},
> +        {"Description"   , "UTF8"},
> +        {"Copyright"     , "UTF8"},
> +        {"Encoder"       , "UTF8"},
> +        {"Title"         , "UTF8"},
> +        {"Cover"         , "JPEG"},
> +        {"Cover"         , "PNG"},
> +        {"Source"        , "UTF8"},
> +        {"CaptureDevice" , "UTF8"},
> +        {"CreationTime"  , "UTF8"},
> +        {"Keywords"      , "UTF8"},
> +        {"Language"      , "UTF8"},
> +        {"Disposition"   , "UTF8"},
> +    }
>  };
> +    Note: No future entries will have NULL as a value.

Two comments:

1. Is it really necessary to have entry 1 ({NULL, "UTF8"}) ? It would
   be simpler if type were always coded when a custom name is.

2. Maybe this is stupid/pedantic, but the correct name is UTF-8, not
   UTF8. Is it worth wasting a byte to correct this?

Rich