[gst-devel] Re: [gst-cvs] alima gst-plugins-good: gst-plugins-good/ gst-plugins-good/gst/auparse/

[Andy Wingo]

Hi Edgard,

Just some reviews on auparse that I never got around to doing. Pretty
good but a couple of nits. Also it seems you are tackling some very
crufty plugins, so they need to be updated to the 21st century before
being updated to 2005 ;-)

On Thu, 2005-09-22 at 15:39 -0700, Edgard Nicéas Arcoverde Gusmão Lima
wrote:

[in gst_auparse_chain]
> @@ -180,9 +174,9 @@
>    layout[0] = 0;
> -  g_return_if_fail (pad != NULL);
> -  g_return_if_fail (GST_IS_PAD (pad));
> -  g_return_if_fail (buf != NULL);
> +  g_return_val_if_fail (pad != NULL, GST_FLOW_ERROR);
> +  g_return_val_if_fail (GST_IS_PAD (pad), GST_FLOW_ERROR);
> +  g_return_val_if_fail (buf != NULL, GST_FLOW_ERROR);

These checks are bogus, and were bogus before. I would just remove them.
If you want to keep them I'd change them to g_assert() because if they
fail it means GStreamer core has a bug that needs fixing.

> @@ -352,47 +348,45 @@
>            NULL);
> -    if (!gst_pad_set_explicit_caps (auparse->srcpad, tempcaps)) {
> -      GST_ELEMENT_ERROR (auparse, CORE, NEGOTIATION, (NULL), (NULL));
> -      gst_object_unref (GST_OBJECT (auparse->srcpad));
> -      auparse->srcpad = NULL;
> +    gst_pad_set_active (auparse->srcpad, TRUE);
> +    gst_pad_set_caps (auparse->srcpad, tempcaps);
> +    gst_element_add_pad (GST_ELEMENT (auparse), auparse->srcpad);
> +    if ((ret = gst_pad_alloc_buffer (auparse->srcpad, GST_BUFFER_OFFSET_NONE,
> +                size - (auparse->offset),
> +                GST_PAD_CAPS (auparse->srcpad), &newbuf)) != GST_FLOW_OK) {
> +      printf ("failed gst_pad_alloc_buffer\n");
> +      return ret;
> +    ret = GST_FLOW_OK;

Why add a pad in the chain function? It would seem better just to add it
in the instance init.

Also, printf? ;)

You have a change_state function but it doesn't do anything. It should
clear its state when going READY->NULL so that it tries to typefind
again if you bring it back to PLAYING.

Do you mind fixing these issues?

Regards,
-- 
Andy Wingo
http://wingolog.org/