(answering both mails in one, as they call back each other)
Michael Smith <msmith at xiph.org> writes:> Can you file bugs and attach the patches to them in our bugtracking
> system? http://trac.xiph.org/
Sorry, I really don't feel like creating 3x tickets right now =)
> Mail dumps of patches are pretty much guaranteed to not get merged.
Understandable.
> From a very quick look at a couple of them: in amongst the actual
> changes, there are lots of formatting changes, and things like
> additions of "XXX: ..." comments that don't say anything
helpful -
> these will need removing before the patches are really reviewable.
hum, I tried to keep those as separated as I could, but might be, it's
about 2 months of developement that is dumped here.
> A higher-level description of what you're attempting to accomplish
> with these patchsets would also be very helpful - but much more detail
> than "performance and reliability".
I actually dumped these patches at my last day working for SmartJog, but
they already have a new guy (Remi) to take over maintainership. The main
idea was to be able to continue working on IceCast while out of
contract. I still have a lot of vectors where I'd like to see the
IceCast code go:
* Add refbuff pools.
* Use libCURL for everything (drop httpp).
* Implement a better stats system (stats are actually a HUGE perf
bottleneck). We can do stats without locking.
* Merge connection.c and slave.c (it's not so far off now).
There are a few more patches needing review to add new formats, but
these need to be released later on. The first thing I tried to do was to
get get_buffer() to have some context stability when called from a
source or a slave (especially the first call), especially for formats
that need to parse a header there. Before the connection re-vamp, what
we got in the refbuf at first call was more sheer luck than anything.
Re-working the connection path, got me to break the way ShoutCast was
handled, I then re-implemented the support in a less hackery way.
A bit later down the track, I needed POST support for testing, that was
easily implemented (patch 14).
Then I got interested in improving the number of accept() per second. In
order to do so, I splited out the connection process in 2:
* accept() and push to a queue.
* process connection and pass it over once auth'd and validated (2 pass:
http validation, and logical validation)
right now, the process phase is timeouted and re-entrant for the first
pass only, the parts of the second one that can be slow (I'm not sure
there are) should get a liftup.
> On Fri, Jul 30, 2010 at 10:43 AM, Romain Beauxis <toots at
rastageeks.org> wrote:
>> Le vendredi 30 juillet 2010 12:25:48, Michael Smith a ?crit :
[?]
> New contributors would be very welcome.
Great !
>> If no one has time to review the patch, perhaps giving Niv Sardi a
commit
>> access is the right answer ? Not necessarily for an immediate stable
release,
>> but, hey, commiting them somewhere would help users testing, reporting,
etc..
>
> Given the number of issues with the patches as they stand, directly
> giving access right now would seem counterproductive.
Agreed, more on that later.
> In Niv is
> interested in becoming part of the icecast community (rather than just
> dumping a pile of patches on us), then in the medium term that would
> be great.
As Romain said, SmartJog is interested in maintaining the patches (they
use IceCast daily), are truely opensource friendly, and will have a guy
(Remi) on the job. On my side, I'd love to see this work merged, and to
continue getting things in IceCast.
Can we cut things up a bit ?
the first patch is a direct feature patch, (not even originaly from me),
maybe that can go in ?
[PATCH 01/31] Hack to support IPhone streaming
Then you can ignore this one, it's actually only needed much after.
[Thread connection]
[PATCH 02/31] introduce thread_cond_init and refrase thread_cond_create.
This one is clean:
[Get rid of the {WARN,INFO,DEBUG,..}n macros]
[PATCH 03/31] LOGGING add non arg-counted macros.
This part is the first cleanup:
[simplify header parsing, inspired by source.c code]
[PATCH 04/31] UTIL, add find_eos_delim and use it to simplify util_read_header
(pending con)
[PATCH 05/31] connection: make process_request_queue use util_find_eos_delim
[PATCH 06/31] connection.c: use util_find_eos_delim to simplify
_handle_shoutcast_compatible
Can we look into getting those ones in ? it's a feature and a bit of
cleanups
Cheers,
--
Niv Sardi