Reviewing merge request #148: Automated playlist generator

The APG is like mixing the Smart Playlists from Amarok 1.4 with dynamic bias system from Amarok 2.0, and turning the whole thing up to 11. Watch a short demo video: http://www.youtube.com/watch?v=h6VEsvQNnjQ

Commits that would be merged:

Version 7
  • Version 1
  • Version 2
  • Version 3
  • Version 4
  • Version 5
  • Version 6
  • Version 7
  • 9b6ae45
  • Soren Harward
almost 2 years ago
  • 0f63529
  • Soren Harward
almost 2 years ago
  • 5e82068
  • Soren Harward
almost 2 years ago
  • f2b39ed
  • Soren Harward
almost 2 years ago
  • f0fee1a
  • Soren Harward
almost 2 years ago
  • d67bbb3
  • Soren Harward
almost 2 years ago
  • 16de2b6
  • Soren Harward
almost 2 years ago
  • 568fefa
  • Soren Harward
almost 2 years ago
  • a20f8b1
  • Soren Harward
almost 2 years ago
  • 626f77a
  • Soren Harward
almost 2 years ago
  • 5f9efb0
  • Soren Harward
almost 2 years ago
  • 8f6af52
  • Soren Harward
almost 2 years ago
  • 7c0c3a4
  • Soren Harward
almost 2 years ago
  • ab8220b
  • Soren Harward
almost 2 years ago
  • 8e8598f
  • Soren Harward
almost 2 years ago
Showing 301e907-9b6ae45

Comments

Sigh,

No Meta::Playlist, no PlaylistProvider a.k.a. no integration whatsoever.

Integration will give us: copying APG playlists to media-devices and synchronization (once it’s enabled, first need to fix MD framework).

I realize this is the same situation as the Biased playlists, but it had the “excuse” of not having a functional playlists framework to work with.

I'm sure I've insisted on this before:
Make this generate Meta:Playlists and add a (fairly simple) PlaylistProvider.

A little bug.. when making a Match All group with a globalmatch constraint that is matching on the ‘year’ tag (jebus what a mouthful), I can’t type numbers greater than 2 digits in length.

I just had a thought about the UI.

The APG system is very powerful, and as such this is reflected in the complexity of the UI. With that being said however, the UI as it is now is too complicated. IMHO, it must be simplified before being released to normal users.

Obviously, while simplifying the UI, we want to minimize feature loss.

My thought is this:
(disclaimer I am using preset here in reference to constraints, not the constraint trees)

Include default constraint presets, such as:
“About X Minutes”
“Mostly/Some/All of Genre X”

Perhaps there should be two modes to this dialog. The “Exact” mode where you can specify everything in detail, and the “InExact” (“Relative”?) mode.

The Exact mode would be the current UI (with w/e UI tweaks it needs), but the Imprecise or InExact mode would have a bunch of preset constraints that have been determined to be common. We could make a survey in a blog post and ask people what they would find useful in this system.

Like I said, the trick here is to keep the power of the system, while making it easier for the user. Because the truth is, most users aren’t going to create a large number of complex constraint trees, so a set of some sane defaults should work for them. But, there will be cases when creating complex constraint trees is desirable.

Let me wrap up with an analogy to another part of Amarok: The Token renamer in the Organize and TagsFromFileName dialogs. The default pretty mode has a nice DND interface and supports a subset of all the features. If the user wants to do something more complex they use the text editing mode.

Basic Screenshot
Advanced Screenshot

@Bart: not really sure what you’re talking about.

I don’t understand why you think APG playlists can’t be copied to a media device or synchronized like any other playlists. Once the generator puts track in the active playlist, you can do anything you want with those tracks, just like if you'd dragged them all in there by hand.

I also don’t understand why APG should be a PlaylistProvider, because it doesn’t actually store any playlists. It just stores rules for generating them.

You’re probably right that I may need to make some changes, but to do so I need you to give me at least one use case. Explain how you think the APG should behave, and then I’ll be able to change it, or explain why it is the way it is.

@Casey: I dislike the idea of two different modes both from a UI and a programming standpoint. I thought about a DND interface for the editor, but building a tree structure using DND is not any easier than building it using a menu. It’s hard to drop things in exactly the right place in a tree, whereas it’s trivial in a list (like a string). And programming a DND builder is extremely difficult.

However, I think it would be a very good idea to provide a handful of simple, common examples to get users started. I can definitely add some exported presets that get installed in Amarok’s doc directory. I can probably also program it so that the first time the user opens up the APG, the examples are already there, ready for experimentation.

Two questions:

Why are you not using the global ThreadWeaver?

It looks like the whole code is not using anything but primary Collection. Is it able to generate playlists with tracks from other collections? If not, why not? I do not see any code

*I do not see any code that requires it to work with one collection

I think it would be really really interesting to make the APG collection-independent. With a MetaQueryMaker it’s transparent for the client code to know what collections it is querying, and it opens up a lot of possibilities—-think tracks from ampache, mp3tunes, magnatune, etc.

it would help discover new music from e.g. jamendo as well

@Leo good idea, in theory. But I think the only QM that could support these queries atm is Jamendo and maybe Magnatune.

@Soren My idea was that the “Complicated” mode would be the current one, i.e., creating trees.

IMHO, the simple one shouldn’t be creating trees, at least not from the user’s perspective.

@Casey: that the service querymaker support is incomplete is irrelevant. There is no reason that APG does not support media devices. The service querymakers just have to be fixed.

@Soren

Use cases for PlaylistProviders:
– Copying playlists to mediadevices, along with the tracks
– Saving playlist as a playlist-file directly (no passing The::Playlist)
– Synchronizing playlists automatically to mediadevices, online playlist services, etc
– Use of generic Meta::Playlist model/view classes using a breadcrum browser (currently in a local git branch to be merged this weekend)

Not having proper PlaylistProviders is the issue I'm having with the mediadevice framework and the reason why we still don’t have playlists on MD’s except for the UMS podcasts. If those were there from the beginning 2.3 would have blown everything else out of the water since I would have had time to implement playlist synchronization.

So if you want to enable automatic playlist synchonization based on APG implement Meta::Playlist and a UserPlaylistProvider.

Bart

@Max : w/r/t the separate ThreadWeavers, it’s a vestigial element from an early design. I’ll try to get rid of it, but the fact that QueryMaker doesn’t expose its ThreadWeaver::Job may prove to be a problem, because I can’t just add the Solver Job as a dependency of the QueryMaker Job. Let me see what I can do.

@Leo, @Casey: It’s theoretically possible to use any type of collection in the APG, as long as I can get Meta::Track objects from that collection. If I understand you correctly, I should use MetaQueryMaker instead of QueryMaker on each collection individually, correct?

@Soren:
That is correct, you can’t use ThreadWeaver’s dependency handling. There is a reason for that though, as not all QueryMaker implementations actually use a different thread, e.g. Ampache.

I think there’s an easy way around that though:

Just add let the ConstraintSolver add itself to the global Weaver in ConstraintSolver::receiveQueryDone.

The code that waits for ConstraintSolver still waits for the ConstratinsSolver::done() signal

Using a MetaQueryMaker is one option, combined with all queryable collections from CollectionManager. Another option would be using ProxyCollection. But that would need a bit of thought, as the only ProxyCollection instance created so far is not globally accessible.

Yes, if you just need Meta::Tracks, you should be able to extend the system to be collection-generic quite easily.

The issue with the current BiasSolver code is that it uses the sqltrackid, which is obviously non-portable. i have a patch lying around to remove that, but never got around to polishing it enough to be workable.

cheers

As of version 6, I've implemented all suggestions (including those from the mailing list) except two:

Make the APG a PlaylistProvider

I don’t yet know enough about PlaylistProviders to know how to do this, or even if it’s appropriate in the first place.

Have a “simple” UI alternative

I added a feature to load a set of example presets the first time the user opens the APG. This, along with the extensive “What’s This?” support throughout the PresetEditor, is about as much direction as I can provide to the user. I haven’t completely dismissed the idea of a simplified interface, but I have yet to figure out an interface (or have one suggested to me) that seems like it would really be worth the effort to code, and not add confusion for having two interfaces for one system.

So unless anyone has any more major objections (and if you do, raise them now), I'm going to plan to merge this in for 2.3.1.

thanks for the update—-i’ll take a look in a bit.

Version 7 has been rebased, so if you have this branch checked out, you’re going to need to either force the update, or delete your local copy of the branch and check it out anew. Sorry guys, but the major overhaul of the core/ directory necessitated this change.

ok. sorry, been swamped, and looking at other MRs too.

my thoughts: this should be a 2.4.0 change, not a 2.3.1 change. that way, maybe people can tweak the UI as well for a bit after it hits trunk.

your thoughts?

on second thoughts…. 2.4 is way too far in the future. 2.3.1 won’t even get tagged till may 14th. so actually, i'd say to merge it this weekend. i’ll try to get some discussion going on the list to see if anyone is really against it. maybe if devs get comfortable with the UI we can come up with a better one, or ways to improve it.

→ State changed from Open to Merged

Markey gave me the merge greenlight yesterday. Just committed it.

Add a new comment:

Login or create an account to post a comment

How to apply this merge request to your repository