[Bug 735085] y4mencode : port y4m encoder to use GstVideoEncoder base class
GStreamer (bugzilla.gnome.org)
bugzilla at gnome.org
Mon Aug 25 12:05:56 PDT 2014
https://bugzilla.gnome.org/show_bug.cgi?id=735085
GStreamer | gst-plugins-good | git
Thiago Sousa Santos <thiagossantos> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #283953|none |needs-work
status| |
--- Comment #1 from Thiago Sousa Santos <thiagossantos at gmail.com> 2014-08-25 19:05:51 UTC ---
Review of attachment 283953:
--> (https://bugzilla.gnome.org/review?bug=735085&attachment=283953)
Thanks for taking time to do this port, a few suggestions below.
::: gst/y4m/gsty4mencode.c
@@ +94,3 @@
{
+ GstElementClass *element_class = GST_ELEMENT_CLASS (klass);
+ GstVideoEncoderClass *venc_class = (GstVideoEncoderClass *) klass;
For consistency, use GST_VIDEO_ENCODER_CLASS or a direct cast for the
GST_ELEMENT_CLASS.
@@ +116,3 @@
{
filter->sinkpad =
gst_pad_new_from_static_template (&y4mencode_sink_factory, "sink");
Creating the pads is done in the base class, this pad is just being stored in
memory and never will be used.
@@ +174,3 @@
{
+ GST_ELEMENT_ERROR (y4menc, CORE, NEGOTIATION, (NULL), ("Invalid format"));
+ return ret;
GST_ERROR_OBJECT is a debug log statement, while GST_ELEMENT_ERROR will post an
error to the bus. It should only return FALSE here directly and print the log
to let base class post the error if needed.
@@ -280,3 @@
}
- /* join with data, FIXME, strides are all wrong etc */
- outbuf = gst_buffer_append (outbuf, buf);
I'd keep using gst_buffer_append here for 3 reasons.
1) Makes it easier to review only the porting patch, as no other changes would
be involved.
2) I'm not sure there is a use case that would justify needing special
allocated memory for this encoder
3) The code is much simpler with gst_buffer_append
allocating buffers with downstream memory makes, usually, more sense for
decoders.
::: gst/y4m/gsty4mencode.h
@@ +47,1 @@
GstPad *sinkpad,*srcpad;
sinkpad and srcpad aren't needed anymore as the base class already has those.
--
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