Gnome-media is actively being maintained. I'm accepting patches, since I'm busy doing other things.

I am not leet enough to contribute to the GNOME community :)

But hey, I am updating my local CVS checkout for years as you might imagine. I don't see gnome-media being that 'actively maintained'. It gets some love every now and then but that's more or less all. If it was that 'actively maintained' then it would already been supporting GST 0.9 which it doesn't (at least not the last time I tried compiling against it).

I'm working on porting gnome-media to 0.10 right now. I hope to have it done by the end of the week.

Created attachment 57027 [details] [review]
preliminary patch #1

Preliminary patch to port things to GStreamer-0.10. gnome-cd isn't ported yet and hence disabled, but I'm working on that. The rest should work fine. gnome-sound-recorder needs some more cleaning up, working on that as well.

Requires gst-plugins-base from CVS as of today.

There's a little gotcha with the gnome-audio-profiles schema that might be worth pointing out: The old version from 2.12 won't work any longer and will result in an 'Internal pad problem. Please file a bug' error message if the schema isn't updated to the 2.14 version. Not sure how that works. Maybe we should check for this special-case in gnome-sound-recorder and print a meaningful 'Please update your schema to XYZ' error message or something? Or is this not a problem at all (I have no ideas how schemas work).

Those are packaging problems, they should be relatively easy to figure out... So no need for that.

For one, I'd appreciate if it worked with both 0.8 and 0.10, like Totem. GNOME 2.14 will use 0.8, with 0.10 being optional.

Ok, first scan of the patch:
* I want 0.8 compatibility
* I don't like all the added "/////////////// bla bla" and "#warning **FIXME **" comments everywhere. Comments are normal /* bla */ style, also in gnome-media
* this patch does way more than a port to 0.8; it changes open/new UI behaviour in g-s-r, it rewrites cdparanoia in -cd, gstreamer-properties appears to be half-rewritten... What's going on there?
* render_bin_from_description() in g-s-r should be used from gst-gconf. If it doesn't exist anymore because gst-gconf was replaced by the gconf plugins, then I'd recommend adding such a utility function to gst-core, or use a utility bin like autoaudiosrc or so instead.
* unrelated fixes like in cdrom.c, the gsr_sample_sound, the channel number string things, comment fix in gsr-window.c, bitrate stuff etc. would better be in a separate patch
* you don't appear to use grecord/src/gst/*, but there's many changes. Why? Half of them are not port-specific, so see above

[First of all, sorry for the delay in replying. I didn't get any mails from bugzilla for some reason]

To address your issues:

(1) 0.8 compatibility
Not for me to decide (personally, I don't see why gnome-media should keep 0.8 compatibility given that (a) 0.8 is unsupported and (b) there aren't any regressions in 0.10 compared to 0.8 that would warrant staying with 0.8 for the gnome-media stuff, or are there?)

(2) all those ////// #warning FIXME  aren't supposed to be there. They are incredibly ugly, yes. Those are what I meant by 'needs cleaning up'. The reason I posted this patch with those ugly comments is to show that things are coming together on the GStreamer-0.10 support side.

(3a) could you explain exactly where the patch changes open/new UI behaviour in gnome-sound-recorder? (the only thing that I can think of is that it doesn't parse filenames passed on the command-line, but that's a temporary thing because I haven't figured out how to do that yet and I intended to fix that as well).

(3b) gstreamer-properties hasn't been half-rewritten. Most of those are formatting changes as far as I can see (there were many different styles used in the same file, and I changed the style in functions where I made other changes). No UI behaviour was changed.

(4) I am all in favour of adding something like render_bin_from_description() as utility function to the core. It's a detail though IMHO.

(5) None of the things in gnome-cd should have been part of the patch. I thought I removed all gnome-cd related changes. Must have accidently uploaded the unedited version, sorry. As I said, the main point was to demonstrate that things are coming together on the 0.10-side.

(6) good catch about grecord/src/gst/*. Those should not have been touched at all (or removed), as they are not used at the moment (the entire subdir with the helper lib is not used). (I started porting them before realising they were probably not needed any longer).


 Cheers
  -Tim

As an addendum to 3a:
I believe I changed one or two dialogs over to gtk_dialog_run(), because without gtk_dialog_run() things like the save dialog would crash if called from the window destroy function (Try: start sound recoder, record something, press control-F4, select 'save' in confirmation dialog, accept given filename and press 'save' button in file chooser => segfault). I was going to file reports and patches for the 2-12 branch for this as well of course, but porting g-s-r and making it actually work for me took priority; hope you don't hold it against me that I fixed a few issues I ran into on the way.

As maintainer, (1) is important for me. If GNOME ships with 0.8, it's important for about a ton of GNOME users who don't care about our internal ongoings and just want a working product, also...

A cleaned-up and improved version of this patch including a gnome-cd port has been committed to the BRANCH-GSTREAMER-0-10 branch in CVS to make it easier for interested parties to try it out and test it (see [1]).

Any outstanding issues will be fixed there as well.

gnome-cd has some minor work left to do, but should be fully usable.

Cheers
 -Tim

[1] http://mail.gnome.org/archives/release-team/2006-January/thread.html#00038

I'm attaching a patch that:

- adds a transmogrify-to-gstreamer-0.10 command to be run before autogen
- adds the necessary files for this
- touches configure.in to update the version to 2.13.0

This is so a release could be made for today's dev release that satisfies all involved parties.

Created attachment 57498 [details] [review]
converts g-m into a 2.13.0 release (?)

Ronald, from your ChangeLog message in HEAD and your comments on IRC I take it you still have issues with the 0.10 code after my clean-up and the various additional fixes.

Would you mind elaborating a little bit on that? Are there any concrete gripes you're having, besides the 0.8-backwards-compatibility thing?

I was under the impression my cleaned-up patch that I committed to the 0.10 branch had addressed all the issues you brought up above. The fact that you only replied to point (1) in comment #10 led me to believe that the other points were resolved/explained to your satisfaction.

So, if there's anything in particular you'd like to see improved, by all means please let me know.

 Cheers
  -Tim

It mostly just means that it is still there, unless I saw a comment saying 
"fixed". Anyway, since you said it was fixed, I went through it again.

- There's still code in there that should be in gstreamer, like find-pad or render-bin-from-description. All that must be moved.
- all error descriptions were wrongly moved back from using "plugin" to using "element". An element is a chemistry term. In UIs, those things are called "plugins".
- in several programs, gst_init_*popt*() was replaced with a no-args gst_init(). Please re-add argument parsing.
- there's still "//////////FIXME"s everywhere. If you want a FIXME, simply use /* FIXME: xyz */
- gst-cdrom.[ch] contains all this old msf code, which is by Iain, please add his name as (c).
- what is this set_pipeline_to_null() hack that I see everywhere? Its comment confirms it's a hack, please fix that.
- input selection in g-s-r is broken. See ChangeLog from the previous time I fixed it. End conclusion: ALSA needs multiple inputs selected (Input, mic, mic something else) to record anything from a single input, and thus we decided to not touch the mixer controls and just let the user figure it out. All we do on a change of selection is mute the $previous and unmute the $$current.
- I noticed that you create new gconf clients for every tick in gstreamer-properties. This is somewhat inefficient, please don't do that. Just save the one you get on startup.
- it seems every error leads to g_warning rather than a dialog? (I guess previously, we didn't care at all, but still very suboptimal...)
- as said before, playback in g-s-r doesn't work.

Comments on previous:
2 - I still see some... Please use grep :).
3a - things like file dialog behaviour and behaviour of labels and when what text was printed in them was changed also, that is UI behaviour. Anyway, too late now.
3b - that should be in a separate patch. Technical ports and indenting fixes should not go along.
4 - let's do that then.

I'm still pissed off about the 0.8 thing, since it means I have to go back to running unstable, incomplete CVS versions of things. Corporate PR wins does it again... Bleh.

People, it's been a week since the last comment; the patch has been applied, but that's merely under pressure from release-team. I'd like comments to the above and I'd like someone to work on them. I've done you guys a favour by early-applying the patch, but that doesn't mean that you're finished now. I want all the above points to be taken care of.

Beta1 is due this weekend, I'd like to have something nicer by then. I'm still disgusted by parts of the patch.

Corresponding patch for gnome-applets Mixer applet.

http://bugzilla.gnome.org/show_bug.cgi?id=328786

Created attachment 58318 [details] [review]
patch fixing ugly comments and copyright in gst-cdrom, and improving gconf client handling in gstreamer-properties

Patch:
 - fixes ugly comments in gst-cdrom.c (all other ugly comments I found seem to
    be part of files not touched by any the above patches)
 - adds Ian's copyright to gst-cdrom.c (not so much for the MSF stuff but for the
    #ifdef 0'ed block in _update().
 - makes gstreamer-properties save and re-use the default gconf client

Created attachment 58526 [details] [review]
patch fixing options parsing by migrating it to GOption


This patch fixes options parsing by migrating it to GOption.

It ups the requirements for libgnome/libgnomeui to 2.13.x, as only those version support GOption. The particular versions in configure.in have been chosen because there was some kind of incompatibility/bug with certain earlier combinations of libgnome-2.13/libgnomeui-2.13 that would lead to segfaults at startup when used with GOption (can't remember details though, sorry).

There is no easy way to fully restore the previous options parsing functionality without migrating to GOption.

Created attachment 58701 [details] [review]
patch to make g-s-r use gst_parse_bin_from_description() from GStreamer CVS

Use newly-added gst_parse_bin_from_description() from gst core CVS. Bumps GStreamer core requirements to current CVS (0.10.2.1).

I cannot test the goption one, since I don't have the relevant libgnomeui version. I'll work on that later...

Committed. Looking at the other problems now.

Created attachment 59088 [details] [review]
Fix possible 'Internal Data Flow' error when stopping recording; make input selection insensitive while recording


This patch:
  - makes input selection combo box insensitive while recording
  - fixes a problem when stopping to record ('Internal Data Flow'
    error dialog might pop up when pressing 'Stop')

As for your playback problems, I can't reproduce them nor know anyone who can. Could you please submit a GST_DEBUG=gsr:5 log for that problem (and preferably open a separate bug report for the issue so it's easier to keep track of things)?

set_pipeline_state_to_null() is not a hack, it's necessary to be informed of bus messages on element shut-down (the bus needs to drop messages when going to NULL because there are circular references involved and it's the only way to break those). But even if you consider it a 'hack', it's still necessary for now.

done by now, closing