[gstreamer-bugs] [Bug 594504] Need a GType of "Date AND Time AND Timezone"

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=594504
  GStreamer | gstreamer (core) | git

--- Comment #54 from Thiago Sousa Santos <thiago.sousa.santos at collabora.co.uk> 2010-07-21 21:55:12 UTC ---
(In reply to comment #48)
> > Added its prototype to gst_private.h and it is now private.
> 
> Make sure to prefix it with priv_ or remove the _*gst prefix, otherwise it'll
> still get exported (see: grep GST_LIB_LDFLAGS Makefile).

Fixed.

> 
> 
> > > Why do we need a datetime getter for GstStructure?
> > 
> > Added those to be used on datetime serialization/deserialization tests (in
> > gstvalue tests).
> > 
> > All other tests uses the GstStructure functions, including GstDate tests.
> > 
> > But I guess I can update the tests to use the GstTagList functions to get them.
> > Is this preferable?
> 
> Ah, I didn't realize we had a gst_structure_get_date(), even though it was me
> who added it :-). I'm not sure what that's good for, but I guess we may just as
> well add a _get_date_time() for consistency and ease of use in unit tests then.

Ok.

> 
> 
> > >  - why do we have gst_date_time_get_dmy(), but
> > >    for the time fields it's _get_hour(), get_minute()
> > >    etc.? Instead of one function?
> > 
> > Because for days you need to get the month first, and for the month, you'd need
> > the year. Getting all 3 at once is faster than getting one by one. For the time
> > functions this doesn't happen.
> 
> Ah, I see. That makes sense, but sounds very implementation-driven. The
> question is what is the better/nicer/more minimal API IMHO.
> 
> We could just have a _get_dmy_and_time() for example, and people can then just
> pass NULL for stuff they're not interested in. Since they need to do a
> _get_foo() for the date anyway, that doesn't seem particularly inconvenient (if
> it was all _get_day() etc. then one could argue that it would be convenient in
> print functions etc. if you can just use the getters there directly, but that's
> not an option anyway now).
> 
> I can live with _get_{hour,minute,second,etc.}() though (in case we do go with
> this, I wonder if it should be plural, ie. get_hours(), get_minutes(),
> get_seconds()?).

Changed internal representation to separate fields and added a get_ for each of
them fields, removing the get_dmy. 

Kept the time functions at singular, a think both make sense and have no strong
opinion.

> 
> ---
> 
> Next: how about:
> 
>   gst_date_time_new_now() -> gst_date_time_new_now_local_time()
> 
> and
> 
>   gst_date_time_new_utc_now() -> gst_date_time_new_now_utc()
> 
> As for the order, I'm not sure which one sounds nicer, best to ask a native
> speaker probably. I think it'd be good to make it explicit in _new_now() that
> it is local time, so people using this API get what they want.

Changed to your suggestions.

> 
> ---
> 
> How about:
> 
>     gst_date_time_get_utc_offset() -> gst_date_time_get_time_zone_offset() ?
> 
> Too long?

Done.

> 
> ----
> 
> gst_date_time_new() + _getters: why do we use gint here when the values are
> alwyas unsigned? To allow passing -1 in future in case we change our minds and
> want to allow some fields to remain unset (e.g. for less accuracy?)

Exactly.

> 
> And I still think that it would be more natural to specify the tz_offset to
> gst_date_time_new() in hours as a gdouble rather than in minutes. 1/2 and 1/4
> can be perfectly expressed in floats, so I don't think there'll be any rounding
> issues.
> 

Ok. Changed to float.

> ---
> 
> Another thing: what about accuracy? Do we want that/need that? (I somewhat do)

XMP uses it too, so I'll need it at some point.

> 
> ---
> 
> The gstdatetime.h file should have a gtk-doc blurb for GstDateTime telling
> people that it's opaque etc., plus since marker.

Done.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.