Remove gstreamer dependency

[MerlijnWajer]

It would be nice to rip out the gstreamer dependency. This means:

1. Writing code for peak calculation using sox (we have this ready)
2. Encoding using the flac tool (we have this ready)
3. AR calculation using the accuraterip_checksum tool (we have this, kinda)

For 3 - I will only for V1 first. Adding V2 support is relatively easy, but that should be done separately.

[JoeLametta]

Step 3 is also linked to issue #18.

[chrysn]

gst is also around in other locations than 1-3, which might have easier fixes (for example AudioLengthTask of the verify/retag pathway, for which i'm currently preparing a sox patch)

[MerlijnWajer]

OK, so just as an update. I will write the ARv2 result code soon; and right after that the flac encoding. Both should be very simple, and it will definitely be ready before Christmas.

[MerlijnWajer]

We will also need the peak patch merged in one form or another.

[RecursiveForest]

Do you mean sox peak detection? That was merged a while ago.

[JoeLametta]

OK, so just as an update. I will write the ARv2 result code soon; and right after that the flac encoding. Both should be very simple, and it will definitely be ready before Christmas.

Thanks for the update: keep up the good work!

[ArchangeGabriel]

Not sure what’s the overall status here, but just in case this could help find eventual places with remaining gst code, I’ve been long using a very trimmed version of gstreamer just for morituri (and now whipper), which has only the following components (found by trial and error adding them one by one until it worked):

--enable-level
--enable-wavenc
--enable-wavparse
--enable-flac
--enable-taglib

I suppose level was for peak detection and could no go away already. flac is for encoding, and I guess @MerlijnWajer incoming work will take care of this. What about the others?

[MerlijnWajer]

I think the only real functional dependency on gstreamer right now is the encoding. Which is trivially replaced by just issuing a call to the 'flac' binary.

Oh, that, and tagging, presumably. This could be done with mutagen with little problems.

[IvanDSM]

Oh, that, and tagging, presumably. This could be done with mutagen with little problems.

Tagging can be done in the encoding call to the flac executable.

[MerlijnWajer]

Yes, it can be done that way, but it might mess a bit with the current workflow. And adding more advanced things like a picture using PIL is definitely going to be much harder that way, I'd say.

[MerlijnWajer]

To be fair the main reason that I am suggesting mutagen is that I already have code that I can copy pretty much verbatim. Adding it to the flac program means we'll have to expand quite a lot on the to-be-created morituri/program/flac.py -- something I am not too happy with if mutagen exists and has (AFAIK) little external dependencies, so it doesn't hurt to just depend on it.

[IvanDSM]

Yes, it can be done that way, but it might mess a bit with the current workflow. And adding more advanced things like a picture using PIL is definitely going to be much harder that way, I'd say.

Do you think there would be the same difficulties if we used the metaflac tool after encoding? Using it for the tagging instead of mutagen would make whipper a little lighter on dependencies since it's a part of the flac package.

[MerlijnWajer]

Mutagen is pretty light on dependencies itself, as well. Using metaflac is an option, but much more difficult, and really feels like reinventing the wheel, especially since the mutagen wheel is so darn round. ;-)

While metaflac can likely do most of what mutagen does, why not just use mutagen? That way it may also be easier to support other formats, if we ever plan on doing that again. I just don't see why we would want to reinvent the wheel - writing custom PIL-to-tempfile and then passing it to metaflac's --import-picture-from, etc. Mutagen makes this easy, and it's native python, with few dependencies (I could not find any in the ebuild, actually, apart from setuptools). metaflac isn't, even though it may be slightly "lightweight" on dependencies.

[IvanDSM]

I see! I overlooked the issue of other formats! Agree with using mutagen then! :)

[ArchangeGabriel]

@MerlijnWajer What’s the status here? Christmas is coming very soon, and there wouldn’t be a better present than being allowed to drop https://aur.archlinux.org/packages/gstreamer0.10-good-morituri/ and https://aur.archlinux.org/packages/gstreamer0.10-good-plugins-morituri/ from the AUR. ;)

[MerlijnWajer]

Christmas was a bit full, so the present is arriving slightly late, but it won't be much longer now. I have to fly tomorrow, but I have my gear set up at my destination. :)

[mokkurkalve]

Actually I was sort of hoping that you could fix wavpack encoding to work again also... ;-) There's a "profile = wavpack" option in morituri but I haven't been able to make it work. There's a bug it seems... I've reported this [1] but upstream have not been responsive...
[1]
https://bugs.archlinux.org/task/42997
https://thomas.apestaart.org/morituri/trac/ticket/157

[MerlijnWajer]

All the encoding profiles will be removed, in favour or just flac or plain wave (as cdparanoia outputs it). You can then use avconv to convert to your favourite format.

[ArchangeGabriel]

Woo… https://thomas.apestaart.org/morituri/trac is an akward state. Not sure since when, but that would make morituri definitively orphaned from my POV.

#!/usr/bin/python
# -*- coding: utf-8 -*-
#
# Copyright (C) 2003-2009 Edgewall Software
# Copyright (C) 2003-2004 Jonas Borgström <[email protected]>
# All rights reserved.
#
# This software is licensed as described in the file COPYING, which
# you should have received as part of this distribution. The terms
# are also available at http://trac.edgewall.org/wiki/TracLicense.
#
# This software consists of voluntary contributions made by many
# individuals. For the exact contribution history, see the revision
# history and logs, available at http://trac.edgewall.org/log/.
#
# Author: Jonas Borgström <[email protected]>

try:
    import os
    import pkg_resources
    if 'TRAC_ENV' not in os.environ and \
       'TRAC_ENV_PARENT_DIR' not in os.environ:
        os.environ['TRAC_ENV'] = '/home/trac/morituri'
    if 'PYTHON_EGG_CACHE' not in os.environ:
        if 'TRAC_ENV' in os.environ:
            egg_cache = os.path.join(os.environ['TRAC_ENV'], '.egg-cache')
        elif 'TRAC_ENV_PARENT_DIR' in os.environ:
            egg_cache = os.path.join(os.environ['TRAC_ENV_PARENT_DIR'], 
                                     '.egg-cache')
        pkg_resources.set_extraction_path(egg_cache)
    from trac.web import cgi_frontend
    cgi_frontend.run()
except SystemExit:
    raise
except Exception, e:
    import sys
    import traceback

    print>>sys.stderr, e
    traceback.print_exc(file=sys.stderr)

    print 'Status: 500 Internal Server Error'
    print 'Content-Type: text/plain'
    print
    print 'Oops...'
    print
    print 'Trac detected an internal error:', e
    print
    traceback.print_exc(file=sys.stdout)

[MerlijnWajer]

Use http://web.archive.org/web/20160427155828/http://thomas.apestaart.org/morituri/trac

[ArchangeGabriel]

Ticket 157 isn’t archived…

[JoeLametta]

Woo… https://thomas.apestaart.org/morituri/trac is an akward state. Not sure since when, but that would make morituri definitively orphaned from my POV.

It works fine if you fetch the webpages using plain HTTP.
Anyway it would be a wise decision to archive all the tickets (in case that website goes offline).

Ticket 157 isn’t archived…

[mokkurkalve]

Yes that works....
http://thomas.apestaart.org/morituri/trac/ticket/157

[ArchangeGabriel]

ArchLinux is dropping gstreamer0.10 very soon. As a consequence, morituri has already been dropped from the repo. whipper can’t make it as long as gstreamer is a dependency.

[MerlijnWajer]

I am going to take care of this tonight & tomorrow evening.

[ArchangeGabriel]

You better would indeed, gstreamer0.10 is now dropped (not even moved to AUR). So current whipper is not installable anymore on ArchLinux as of now.

[MerlijnWajer]

OK, I have a working flac encoder. Let's see if I can get this PR'd today and then let's look at tagging with mutagen.

[MerlijnWajer]

So there are a few more things to note. Gstreamer is being used for:

• encoding
• tagging
• crc32 (sigh)

For the above two parts, I now have code. I will submit a PR for the Flac encoder as default in an hour or so. The tagging code is also ready, but I want to clear it up a bit before I send a PR.

For some reason morituri also does the tagging in the 'Read and verify' track task. I don't see how that makes sense, so I am ripping that out.

The CRC32 code is not yet written - although I personally don't understand the real need for it. I guess we want it for the .log files? For just comparing tracks, any other checksum should also be perfectly fine. Can't we just get rid of it altogether, as long as we have an integrity check? ( @JoeLametta @RecursiveForest )

We'll have to then remove all the 'encoding profile' code. We can also throw out the old AccurateRip checksum code. We can then also throw out the tagging. And when we have an alternative for CRC32 code, we can throw that out too. We can then throw out the gstreamer task code. And we can then get rid of the 'gstreamerVersion' and 'gstPythonVersion' code. Any volunteers to rip out that code...?

[MerlijnWajer]

Flac commit can be found here - https://github.com/MerlijnWajer/whipper/tree/flac-encoder

It worked for me to rip several tracks. Note that it doesn't do tagging yet, so don't merge this. (EncodeTask also does the tagging, for some reason that only makes sense in gstreamer universe)

[MerlijnWajer]

The same branch ('flac-encoder') now also does the tagging. I have created a pull request here - #121

Please test.

[MerlijnWajer]

Added CRC code to the same PR.

[MerlijnWajer]

Just for fun, I tried to remove all gstreamer+profile related code, on top of PR #121; and it fully ripped all tracks, generated a .log, everything!

[MerlijnWajer]

https://github.com/MerlijnWajer/whipper/tree/without-gstreamer

[ArchangeGabriel]

So, now that #121 has been merged, what’s left? Image subcommands, logger changes, anything else?

[MerlijnWajer]

I don't plan to implement commands that it seems most people don't use. 'Retag' is not anything a 'perfect ripper' should have to do. (Just use Picard?) 'Verify'ing an image also doesn't seem useful at this point.

I would like to rip out whatever code we don't need now (at least whatever uses gstreamer), and then gradually add back in functionality that we would need.

Logger changes are not required for gstreamer, afaik.

As you can see - the without-gstreamer branch I linked already works without gstreamer. It does likely break some image related functionality. So we'll need to give that some more thought.

[ArchangeGabriel]

Hum indeed, retag is more of a tagger tool job, and I can’t see the point in verify. We might also reimplement it later if it turns out to be useful for something.

Do some people have use case for those commands?

@MerlijnWajer Maybe open a PR removing gstreamer and image subcommand, if people have something to say about it they’ll be able to do it on the PR too. ;)

[MerlijnWajer]

Once I'm back from FOSDEM (after the weekend) I'll send a proper 'remove gstreamer patch', the current 'without-gstreamer' branch is a test where I mindlessly ripped out anything that even looked like gstreamer code, so I want to re-do that more sensibly.

[MerlijnWajer]

#130

[JoeLametta]

Completed with #130.

Thanks to @MerlijnWajer!