[Bug 518857] [API] GstBaseParse: new base class for parsers

GStreamer (bugzilla.gnome.org) bugzilla at gnome.org
Sun Mar 13 14:35:45 PDT 2011

  GStreamer | gstreamer (core) | git

--- Comment #23 from Mark Nauwelaerts <mnauw at users.sourceforge.net> 2011-03-13 21:35:35 UTC ---
(In reply to comment #22)
> So, I've basically moved this into libgstbase in my local core, and fixed up a
> few minor things up here and there.
> I noticed a few cosmetic issues when reviewing the API (let's just try to
> decide what we want to do and I'll fix it up locally):
>  - gst_base_parse_set_seek() => rename to ???
>    (set_seekable, set_seekability, set_seek_type, set_seek_mode?)
>  - or maybe get rid of gst_base_parse_set_seek() entirely, and just
>    have _set_bitrate()? Or add a separate _set_average_bitrate()?
>  - GstBaseParseSeekable => GstBaseParseSeekability or GstBaseParseSeekMode?
>    (see above)
>  - Name mismatch with "GstBaseParse*Seekable*" and
>    (should be renamed according to whatever is decided above)

Seems like we could get rid of seek* entirely and stick to a
set_(average_)bitrate (or set_bitrates allowing for min/max as well?).

>  - gst_base_parse_set_format() looks a bit weird API-wise ("gboolean on"),
> maybe
>    make it {set|unset}_format_flag() or something like that?

I'd go for set_format_flags (with a gboolean on), but taste varies.

>  - gst_base_parse_set_frame_props() -> gst_base_parse_set_frame_properties()?

>  - should we add some padding to GstBaseParseFrame? (better safe than sorry
> etc.)

Probably.  The idea/was is for this not to be needed by having
GstBaseParseFrame grow as needed (without interfering with existing code).
Only flacparse using gst_base_parse_frame_init (and _clear done in baseparse)
is complicating that approach, which might be alleviated by using a _new/_free
instead (though a repeated alloc/free is also preferably avoided).

>  - GstBaseParseFrame: shouldn't flags be GstBaseParseFrameFlags instead of
> guint

>  - does GstBaseParse::segment need to be exposed? (nothing uses it afaict)
>  - does GstBaseParse::pending_segment need to be exposed? (nothing uses it
> afaict)
>  - does GstBaseParse::close_segment need to be exposed? (nothing uses it
> afaict)
>  - does GstBaseParse::adapter need to be exposed? (nothing uses it afaict)
Likely not (legacy leftover).

>  - GST_BASE_PARSE_{SRC,SINK}_NAME -> unused, let's remove it (seems pointless
> too)

>  - GST_BASE_PARSE_FRAME_SYNC() -> rename to express booleaness?
>    (maybe easier if we make it negative, like LOST_SYNC)
>  - GST_BASE_PARSE_FRAME_DRAIN() -> rename to express booleaness?

>  - GST_BASE_PARSE_FRAME_{DRAIN,SYNC} - shouldn't "draining" and "in sync"
> status
>    be something on GstBaseParse rather than GstBaseParseFrame? (maybe add API
> to
>    get status for those?)

Possibly.  I am not particularly fond of a whole plethora of "little"
_get/_set, and the baseaudioencoder/*decoder approach tends to use a "context"
to side-step that (and avoid additional call(s) each parsing round in as much
that might have impact), but taste may vary (again).

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.

More information about the gstreamer-bugs mailing list