https://bugzilla.gnome.org/show_bug.cgi?id=594504
GStreamer | gstreamer (core) | git
--- Comment #54 from Thiago Sousa Santos <thiago.sousa.santos@...> 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.
|