[Bug 642690] [baseaudioencoder] GstBaseAudioEncoder class

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=642690
  GStreamer | gst-plugins-bad | git

Stefan Kost (gstreamer, gtkdoc dev) <ensonic> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ensonic at sonicpulse.de

--- Comment #1 from Stefan Kost (gstreamer, gtkdoc dev) <ensonic at sonicpulse.de> 2011-03-08 12:56:39 UTC ---
Cool stuff! A patch set for the basecalsses would be easier to review though. I
hope you can match the context.

vorbiscommon seems to be only used by vorbis.

gstbaseaudioencoder.h
 101  * GstAudioState:
this needs a one liner describing the purpose below the fields. Is it really a
state or GstAudioFormat. Should that go to gst-plugin-base/libs/audio (could
ev. be used in the GstRingBuffer too).

 102  * @xint: whether sample data is int or float
"is_int" looks better to me.


 178  * GstBaseAudioEncoderClass:
 200  * @pre_push:       Optional.
not clear when one would need it. The ported encoders don't seem to use it.

gstbaseaudioencoder.c
 104  * <link linkend="GstBaseAudioEncoder--perfect-ts">perfect-ts</link>.
 108  * than <link linkend="GstBaseAudioEncoder--tolerance">tolerance</link>,
 116  * (see <link
linkend="GstBaseAudioEncoder--hard-resync">hard-resync</link>)
you can write "GstBaseAudioEncoder:perfect-ts" "GstBaseAudioEncoder:tolerance"
and so on

 231 GType
 232 gst_base_audio_encoder_get_type (void)
shoudl this be using G_DEFINE_ABSTRACT_TYPE_WITH_CODE() ?

 310       g_param_spec_boolean ("perfect-ts", "perfect-ts",
in the name we can spell it out more "perfect timestamps". Also on other
properties.

The whole granule business might need some more explanation. People who don't
know ogg might not be familiar with it.

 431 /** gst_base_audio_encoder_finish_frame:
the gtk-doc syntax is (several occurrences):
 /**
  * gst_base_audio_encoder_finish_frame:


 451  * Returns: a #GstFlowReturn that should be escalated to caller (of
caller)
typo at the end of the line?

 600     // TODO return value ?
ha - its not only me doing that (c++ comments) :) (there are more of them)

 907 #define CHECK_VALUE(res, var, val) \
Use G_STMT_START/END, in the code below you create several ;; right now - just
to avoid suprises if some one adds an if() there later on. Also either drop
'res' from the args (and use it implicitely) or also pass 'changed' :)

1181   enc = GST_BASE_AUDIO_ENCODER (gst_pad_get_parent (pad));
event handlers are protected by the streaming lock and thus the pad cannot
disappear. enc = GST_BASE_AUDIO_ENCODER (GST_PAD_PARENT (pad)); should be
enough.

-- 
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.