Created attachment 97436 [details] [review]
cvs_diff_pu


ok.

gst_structure_to_string add ';' to the end
gst_caps_to_string do not add ';' to the end of each field anymore

ideally the gst_structure_to_string_add should use '(' and ')' to delimiter the structure because the 'open' and 'close' delimiters would be in just one place.
currently, the function that calls gst_structure_to_string is in charge of write the open delimiter which is "(structure)".
so, gst_structure_from_string does NOT look for "(structure)" open delimeiter, but looks for the structure name, followed by fields and them followed by ';' (currently it could find \0 at the end)

a nested gst_structure looks like this:

"Exif, Camera=(structure)Camera, XResolution=(int)72, YResolution=(int)73;, Image Data=(structure)Image-Data, Orientation=(string)top-left;;"

*notice the difference beteween the fisrt and second "Image Data" != "Image-Data"
the fisrt is the name of the structure field of Exif structure
the second is the name of the structure Image-Data (which can not have ' ' in it)

....pls just assigne me...I'm waiting for comment and improvements and a ok to commit it



BR,
Edgard

I'm still a bit uneasy with moving the ; generation from GstCaps to GstStructure.

I guess it's the best way to serialize and deserialize the struct because then it generates and parses its own delimiter instead of relying on the one GstCaps created around the struct. It does however subtly break the testsuite, not a biggie maybe...

Alternatively, when GstStructure serializes a GstStructure, it can create the ; at the end and GstCaps can then also create the ; as it does now. This would not break the testsuite. It adds a bit more special casing but allows us to easily change it so that there are () around nested structs.

Ok, not problem if GstCaps add an ';' after each of fields (which, currently, are just GstStructure) (*1) (*2)

Yes, GstStructure MUST add a delimiter, otherwise, we are forcing all of types that possible are using a GstValue of type GstStructure to have something like { if ( field.type == structure ) append(str, ';'); }

(*1) Yes, it is correct to use a delimiter after each caps field, but remember, field delimiter (at least regarding GstStructure) is ',' not ';'.....but, yes, it would break current applications so its is not a big problem to use ';' as field delimiter for caps.

(*2) There is a requirement that any 'Type' MUST follow, it has to know when it ends, could be '\"', ')', ';', '\0' or whatever 'cause the 'Thing' that owners this 'Field' doesn't know what is inside the 'Field', so, the 'Thing' can't separate the fields and create a new string ended with '\0' to be parsed by the field_type_from_string. It is because the field can have inside it the same character that the 'Thing' uses as delimiter.

-

So my suggestion is: (I'm attaching a patch with it)

1- structure put a ';' in the end (it is just a structure mark of end, currently we could accept also \0 as end mark), in the future we could use '(' and ')' (*5)

2- caps put a ';' as field delimiter, in the future we could change to ','

3- add an additional ';' as Caps mark of end (for now it is still acceptable to find \0 for backward compatibility reason)

-

(*3) there is something still not solved, what if some type wants to use GstCaps inside it? How GstCaps knows where it ends? So again...it would be a good design to use ',' (or whatever) as field separator and ';' (or whatever) as and of list mark.

(*4) So, each type the has a variable lenght MUST know where it ends

(*5) The only advantage I can see to use '(' and ')' for variable length is 'cause it is more readably by Humans ... but may be I'm missing something here and it is also better for parsers

[]s
Edgard

please assign me

Created attachment 97474 [details] [review]
cvs_diff_pu3

This patch make possible GstStructure nested (or even GstCaps nested)

1- structure put a ';' in the end 

2- caps put a ',' as field delimiter

3- caps add an additional ';' in the end

4- for backward compatibility reasons caps_from_string and structure_from_string accepts '\0' instead of and delimiter ';' in the end

5- for backward compatibility reasons caps_from_string accept the absence of ',' between each structure

* I'm just waint the OK to commit

BR
Edgard

so, the serialized caps will looks like this:

"video/x-raw-yuv, widht=(int)3, height=(int)2;, video/x-raw-rgb, width=(int)4, height=(int)5;;"

first ';' is the structure end delimiter
first ',' (next to ';') is caps field delimiter (between each structure)
second ';' is the structure end delimiter
third ';' is the caps end delimiter

BR

Not ok.

With this patch, I did this, in python:

import gst
c = gst.caps_from_string("video/x-raw-yuv,width=640,height=480")
print c.to_string()

Which output:
video/x-raw-yuv, width=(int)640, height=(int)480;;

Then took that string, and went back to my installed gstreamer (0.10.14), and ran this:

import gst
c = gst.caps_from_string("video/x-raw-yuv, width=(int)640, height=(int)480;;")
if not c:
    print "You broke compatibility!"

Which unfortunately printed out the "You broke compatibility!" message.

We serialise caps in various places, and changing the serialisation of them will break things. So, we MUST avoid doing so.

It's getting silly now. No extra comma between structs in caps is needed. No extra ; at the end of caps is needed.

MikeS,

1- of course such test print this

...when I said compatibility, it means the current GStreamer can create caps to/from string or structure to/from string by using the 'old' strings (without delimiters)

of course the old gstreamer version can't parser the new created string

Wim,

If you don't like to put field separators beteween caps, OK.
so, no comma between caps fields (structure)...fortunately it will work 'cause the structures knows its end (';')

Ok, to not have extra ';' at the end of caps (you just have to keep in mind that caps can't be inside of values unless something are assumed...)

....lets forget the change by now and think what happens with the following code with currently GStreamer version:

caps - new_caps_with_some_structures_inside();
strucuture = new_strucure()

str1 = caps_to_str(caps);
str2 = structure_to_str(structure);

write(file, str1);
write(file, str2);

...then...

str = read(file);

caps = caps_from_str(); /* update str pointer to the end */
structure = structure_from_str();

So, the caps will have one more 'structure' and structure will be null

- - - - - - - - - - - - - - - -

But ok,

so, no additional ',' for caps and no additional ';' at the and of caps

so , in 5 min I will attach another patch

1- ';' will still be printed by satructure
2- 'the latest ';' will be removed by caps

Mike,

now the patch I'll attach will be 100% compatibly (even with only GstStructure that now print a new ';' in the end cause in current GStreamer version it already looks for a ';')

btw: How can GstStructure look for an end ';' if it doens't write it ??!?! So, it is not a well designed API, GstStructure forces it parent to write ';' ....so it should be at least better documented, in gst_structure_to_string documentation should say /* please put a ';' or '\0' after me so that I know handle gst_structure_from_string */

...but anyway 5 min will put the patch

BR

Created attachment 97480 [details] [review]
cvs_diff_pu3_

the is almost the same as the previous patch attached (just removind ',' and removing latest ';' from caps)

please, lets decide how to do it

1- structure print ';' at the end
2- caps will not print ';' between structures (already printed by structures) and will remove latest ';'
* then we have structure nested and 100% backward compatibility

MikeS and Wim, please tell me if it is OK, if not:
1- tell me what to do
2- decide to nome make GstStructure nested anymore

Thanks

yes, this sounds right.

I just tested to see if the old structure deserialize code can deserialise the (new) structure ending in ; and it can. This is good.

Also tested the removal of the last ; for GstCaps, which also seems to work fine.

Then some tests on caps deserialisation:

c = gst.caps_from_string("video/x-raw-yuv, width=(int)640, height=(int)480") seems to still work. 

c = gst.caps_from_string("video/x-raw-yuv, width=(int)640, height=(int)480;") seems to work fine.

c = gst.caps_from_string("video/x-raw-yuv, width=(int)640, height=(int)480;;") faild, which is also expected.

Seems ok to me.

just to avoid confusion...the caps with this latest 'patch' will look like this:

"video/x-raw-yuv, width=(int)640, height=(int)480"

the same as previous, because the 'caps_to_string' remove the latest ';'

btw: So, the only difference is for GstStructure alone that always will have a ';'in the end

MikeS, __tim, ensonic, what about you ???

BR
Edgard

applied to gstreamer/

2007-10-22  Edgard Lima  <edgard.lima@indt.org.br>

       * gst/gstcaps.c: (gst_caps_to_string),
        (gst_caps_from_string_inplace):
        * gst/gststructure.c: (gst_structure_get_abbrs),
        (gst_structure_to_string), (gst_structure_from_string):
        * gst/gstvalue.c: (gst_value_set_structure),
        (gst_value_get_structure), (gst_value_serialize_structure),
        (gst_value_deserialize_structure), (_gst_value_initialize):
        * gst/gstvalue.h:
        * tests/check/gst/gststructure.c: (GST_START_TEST),
        (gst_structure_suite):
        * tests/check/gst/gstvalue.c: (GST_START_TEST):
         Added GstStructure to gst_value_table and its related functions.
         Changed gst_structure_to_string to print ';' in the end.
         Changed gst_caps_to_string to not print ';' beteween its
         fields (structures) anymore and remove the lastes ';' from latest
         structure. Now it is possible to have nested structures.
         In addition, backward compatibilty is assured by accepting '\0' as
         end delimiter. Fixes: #487969.
         API: add gst_value_set_structure()
         API: add gst_value_get_structure()