[gstreamer-bugs] [Bug 631200] flacparse: major performance improvements

[GStreamer (bugzilla.gnome.org)]

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

--- Comment #35 from Sebastian Dröge <slomo at circular-chaos.org> 2010-10-23 07:42:55 UTC ---
(In reply to comment #32)
> (In reply to comment #29)
> > (From update of attachment 171597 [details] [details])
> > Except the GValue initializations this is fixed now. I don't see what's wrong
> > with these initializations and GLib and GStreamer is doing that *everywhere*
> 
> That doesn't make it right.
> 
> See bug #577231.

Everything after the first 0 will be initialized to 0 too, that warning only
means that you didn't explicitely initialize the remaining struct members.

(In reply to comment #33)
> (In reply to comment #31)
> > (From update of attachment 171603 [details] [details])
> > I don't see anything wrong with this code either.
> > 
> > a) Every list node always contains a valid buffer reference
> 
> Not true, there are some error conditions where this doesn't happen.

Which cases?

> I know because I saw the warnings of gst_mini_object_unref(), which why I
> bothered writing the patch.
> 
> BTW. gst_mini_object_unref has a silly API, should be:
> void gst_mini_object_unref(void *mini_object);

There's a gst_buffer_unref() for the correct parameter type.

> And should accept NULL, that would make it really useful, similar to free(),
> and g_object_unref().

g_object_unref() does not accept NULL, neither does free() (in general).
g_free() accepts NULL, yes.

(In reply to comment #34)
> For the record:
> % git log --author=felipe.contreras -- gst/audioparsers
> 
> Shows basically nothing. Way to incentivize contributors.

I committed two of your patches which could be included. The caps fix and the
picture parsing fix.
I rejected other patches because they were not necessary (as explained above)
or the large rewrite of the parsing because you added unrelated changes and
also used C99 features everywhere.

Instead I've done the rewrite of the parsing myself based on your idea, which
was done faster than splitting your patches, cleaning things up, etc. But if
you look at the commit message of that change you'll see your name.

> I have more luck contributing patches to the linux kernel, where they:
>  a) actually review the patches

Not sure why you think nobody reviewed your patches

>  b) comment the exact changes needed, or what in general would they do

... and told you what was wrong with them or why I think they were not
necessary.

>  c) give a chance to the contributor to do the changes themselves

You complained that the patch submitting process wasted too much time for you
and that you didn't plan to submit the patches at all and also thought that
they might not be good for inclusion yet. I assumed that you don't plan to
improve them and as such I've simply fixed your patches myself (or have done
them in a way that is acceptable for inclusion)

>  d) leave the original author in the commit

And if you took at look at the commit message you would see that the rewritten
change still says that it's based on your patch

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