Re: Review Request 112802: Extend dbus unterface for reading and writing the rating of the current song

[Alex Merry]

> On Sept. 20, 2013, 11:33 a.m., Matěj Laitl wrote:
> > src/dbus/mpris2/org.kde.amarok.Mpris2Extensions.Player.xml, lines 21-23
> > <http://git.reviewboard.kde.org/r/112802/diff/2/?file=190470#file190470line21>
> >
> > Hmm, I must say I don't like having a rating property for a "player" 
> > object, because it doesn't logically belong there. It logically belongs to 
> > a "track".
> > 
> > For reading purposes, the only reasonable place is 
> > MediaPlayer2.TrackList.GetTracksMetadata, and we *already* fill in 
> > xesam:userRating, so please let's don't add unnecessary redundancy (yet 
> > another undocumented ad-hoc method).
> > 
> > For writing let's wait for the outcome of the discussion on the MPRIS 
> > list.
> 
> Alex Busenius wrote:
> The MPRIS list seems quite dead to me, but let's wait a bit...
> 
> I'll take care of the other issues as soon as we agree on how to write 
> rating.

Things are stalled for a bit while Mirsal (the other MPRIS maintainer) is 
super-busy.  Once things have calmed down for him, we should progress with this.


- Alex


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112802/#review40381
---


On Sept. 18, 2013, 7:34 p.m., Alex Busenius wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112802/
> ---
> 
> (Updated Sept. 18, 2013, 7:34 p.m.)
> 
> 
> Review request for Amarok and Alex Merry.
> 
> 
> Repository: amarok
> 
> 
> Description
> ---
> 
> The D-Bus interface is currently missing convenient method for reading and 
> setting user rating (the stars). The proposed patch adds an Mpris2 extension 
> to /org/mpris/MediaPlayer2 for that: 
> 
> property readwrite int org.kde.amarok.Mpris2Extensions.Player.Rating
> 
> 
> Diffs
> -
> 
>   src/dbus/mpris2/MediaPlayer2AmarokExtensions.h 964c7b8 
>   src/dbus/mpris2/MediaPlayer2AmarokExtensions.cpp ba8b54e 
>   src/dbus/mpris2/org.kde.amarok.Mpris2Extensions.Player.xml 3397df4 
> 
> Diff: http://git.reviewboard.kde.org/r/112802/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Busenius
> 
>

___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request 112802: Extend dbus unterface for reading and writing the rating of the current song

[Alex Merry]

> On Sept. 18, 2013, 9:46 p.m., Alex Merry wrote:
> > I'm torn between following Amarok's internals (which you do) and following 
> > MPRIS2, meaning it should be called UserRating and return/allow a double 
> > between 0.0 and 1.0.
> > 
> > There's also the question of how urgent this is, as there's discussion on 
> > the MPRIS list (which had stalled for a few months, but I started up again) 
> > about adding a general way of editing Metadata entries.
> 
> Alex Busenius wrote:
> I don't see anyy sign of UserRating in MPRIS2 specs: 
> http://specifications.freedesktop.org/mpris-spec/latest/... Can you point me 
> to the right place? If there is a more standard way to set rating, I would 
> prefer doing it that way.
> 
> A general way of editing metadata would be great, but would it not be 
> better to handle rating differently (e.g. standardize range 0.0 ... 1.0 and 
> let applications map their understanding of rating into that range)?

http://www.freedesktop.org/wiki/Specifications/mpris-spec/metadata/#index22h4 
(xesam:userRating), linked to from 
http://specifications.freedesktop.org/mpris-spec/latest/Track_List_Interface.html#Mapping:Metadata_Map

So my point here is that MPRIS has "user rating" and "auto rating", while 
Amarok has "rating" and "score".  MPRIS standardises both ratings to be in the 
range 0.0 to 1.0, while Amarok has the user rating be an integer number between 
0 and 10.  The question is, should an Amarok extension to the MPRIS interface 
follow the MPRIS conventions or the Amarok conventions?  I'm leaning slightly 
towards following the MPRIS conventions.


- Alex


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112802/#review40313
---


On Sept. 18, 2013, 7:34 p.m., Alex Busenius wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112802/
> ---
> 
> (Updated Sept. 18, 2013, 7:34 p.m.)
> 
> 
> Review request for Amarok and Alex Merry.
> 
> 
> Description
> ---
> 
> The D-Bus interface is currently missing convenient method for reading and 
> setting user rating (the stars). The proposed patch adds an Mpris2 extension 
> to /org/mpris/MediaPlayer2 for that: 
> 
> property readwrite int org.kde.amarok.Mpris2Extensions.Player.Rating
> 
> 
> Diffs
> -
> 
>   src/dbus/mpris2/MediaPlayer2AmarokExtensions.h 964c7b8 
>   src/dbus/mpris2/MediaPlayer2AmarokExtensions.cpp ba8b54e 
>   src/dbus/mpris2/org.kde.amarok.Mpris2Extensions.Player.xml 3397df4 
> 
> Diff: http://git.reviewboard.kde.org/r/112802/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Busenius
> 
>

___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request 112802: Extend dbus unterface for reading and writing the rating of the current song

[Alex Merry]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112802/#review40313
---


I'm torn between following Amarok's internals (which you do) and following 
MPRIS2, meaning it should be called UserRating and return/allow a double 
between 0.0 and 1.0.

There's also the question of how urgent this is, as there's discussion on the 
MPRIS list (which had stalled for a few months, but I started up again) about 
adding a general way of editing Metadata entries.

- Alex Merry


On Sept. 18, 2013, 7:34 p.m., Alex Busenius wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112802/
> ---
> 
> (Updated Sept. 18, 2013, 7:34 p.m.)
> 
> 
> Review request for Amarok and Alex Merry.
> 
> 
> Description
> ---
> 
> The D-Bus interface is currently missing convenient method for reading and 
> setting user rating (the stars). The proposed patch adds an Mpris2 extension 
> to /org/mpris/MediaPlayer2 for that: 
> 
> property readwrite int org.kde.amarok.Mpris2Extensions.Player.Rating
> 
> 
> Diffs
> -
> 
>   src/dbus/mpris2/MediaPlayer2AmarokExtensions.h 964c7b8 
>   src/dbus/mpris2/MediaPlayer2AmarokExtensions.cpp ba8b54e 
>   src/dbus/mpris2/org.kde.amarok.Mpris2Extensions.Player.xml 3397df4 
> 
> Diff: http://git.reviewboard.kde.org/r/112802/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Busenius
> 
>

___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request 111510: Consistently use FindFFMpeg.cmake from kdelibs

[Alex Merry]

> On July 15, 2013, 5:12 p.m., Mark Kretschmann wrote:
> > Builds ok here with KDE 4.10.90. I'm wondering though if FindFFMpeg.cmake 
> > is also included in KDE 4.8, which we still support.

Just checked: yes, it is, and the docs at the top of the file suggest none of 
the variables have changed.


- Alex


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111510/#review35988
---


On July 15, 2013, 1:18 p.m., Alex Merry wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111510/
> ---
> 
> (Updated July 15, 2013, 1:18 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> ---
> 
> Consistently use FindFFMpeg.cmake from kdelibs
> 
> FindFFMpeg from kdelibs works differently to our internal one (and, in
> fact, more consistently with standard Find* files).  Since we depends on
> kdelibs anyway, we now just use the kdelibs one and don't bother
> shipping our own.
> 
> 
> Diffs
> -
> 
>   ChangeLog 5fe5d2e64c771d722f3f90bf6c98d5013e56553c 
>   cmake/modules/FindFFmpeg.cmake 0aa2cdc33a88953fe8b94532cb45c43ad35e0da8 
>   src/CMakeLists.txt 589fddb9ea2177102cabd3a193a9e6848ea9c12d 
> 
> Diff: http://git.reviewboard.kde.org/r/111510/diff/
> 
> 
> Testing
> ---
> 
> Build works with FFMpeg and libOFA installed, and kdelibs 4.10.5.  No other 
> configurations tested.  (Without the patch, the build fails with this 
> configuration).
> 
> 
> Thanks,
> 
> Alex Merry
> 
>

___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request 111510: Consistently use FindFFMpeg.cmake from kdelibs

[Alex Merry]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111510/
---

(Updated July 15, 2013, 1:18 p.m.)


Review request for Amarok.


Changes
---

Now with Changelog entry.


Description
---

Consistently use FindFFMpeg.cmake from kdelibs

FindFFMpeg from kdelibs works differently to our internal one (and, in
fact, more consistently with standard Find* files).  Since we depends on
kdelibs anyway, we now just use the kdelibs one and don't bother
shipping our own.


Diffs (updated)
-

  ChangeLog 5fe5d2e64c771d722f3f90bf6c98d5013e56553c 
  cmake/modules/FindFFmpeg.cmake 0aa2cdc33a88953fe8b94532cb45c43ad35e0da8 
  src/CMakeLists.txt 589fddb9ea2177102cabd3a193a9e6848ea9c12d 

Diff: http://git.reviewboard.kde.org/r/111510/diff/


Testing
---

Build works with FFMpeg and libOFA installed, and kdelibs 4.10.5.  No other 
configurations tested.  (Without the patch, the build fails with this 
configuration).


Thanks,

Alex Merry

___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request 111512: MPRIS2: avoid updating Metadata when between tracks

[Alex Merry]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111512/
---

(Updated July 15, 2013, 1:18 p.m.)


Review request for Amarok.


Changes
---

Now with Changelog entry


Description
---

MPRIS2: avoid updating Metadata when between tracks

When changing tracks, we would emit PropertiesChanged for Metadata
twice, once with mpris:trackid set to /org/kde/amarok/PendingTrack and
once with the actual new trackid (because the first time the playlist
code had not yet updated the active track).  If the track was changed
manually (not just progressing to the next one) we would often also emit
a PropertiesChanged with an empty Metadata before repopulating it.

This solves the first issue by making the signal connection for
trackChanged from EngineController queued, meaning that by the time the
MPRIS2 code gets the signal, the playlist has updated the activeTrack
and we can easily figure out the correct trackid.

It solves the second issue by ignoring the trackLengthChanged signal
when it has a meaningless value (<0), which seems to happen at some
point during track changes that are not predictable.

BUG: 321602


Diffs (updated)
-

  ChangeLog 5fe5d2e64c771d722f3f90bf6c98d5013e56553c 
  src/dbus/mpris2/MediaPlayer2Player.cpp 
a633756bf558a89ba2a3db2307c0ebbc373a759b 

Diff: http://git.reviewboard.kde.org/r/111512/diff/


Testing
---

Tested using a tool that listens to the PropertiesChanged signal of the MPRIS2 
interface and lists when the mpris:trackid changes.

Without the patch, I get output like
Track change: "/org/kde/amarok/Track/5739423209746661216" -> 
"/org/kde/amarok/PendingTrack"
Track change: "/org/kde/amarok/PendingTrack" -> 
"/org/kde/amarok/Track/8264712350997591513"
when the track progresses because the previous track finished, and
Track change: "/org/kde/amarok/Track/5739423209746661216" -> ""
Track change: "" -> "/org/kde/amarok/Track/8264712350997591513"
when I manually change the track (eg: by clicking "next" or by double-clicking 
another track).

With the patch, I only get things like
Track change: "/org/kde/amarok/Track/5739423209746661216" -> 
"/org/kde/amarok/Track/8264712350997591513"


Thanks,

Alex Merry

___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Review Request 111512: MPRIS2: avoid updating Metadata when between tracks

[Alex Merry]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111512/
---

Review request for Amarok.


Description
---

MPRIS2: avoid updating Metadata when between tracks

When changing tracks, we would emit PropertiesChanged for Metadata
twice, once with mpris:trackid set to /org/kde/amarok/PendingTrack and
once with the actual new trackid (because the first time the playlist
code had not yet updated the active track).  If the track was changed
manually (not just progressing to the next one) we would often also emit
a PropertiesChanged with an empty Metadata before repopulating it.

This solves the first issue by making the signal connection for
trackChanged from EngineController queued, meaning that by the time the
MPRIS2 code gets the signal, the playlist has updated the activeTrack
and we can easily figure out the correct trackid.

It solves the second issue by ignoring the trackLengthChanged signal
when it has a meaningless value (<0), which seems to happen at some
point during track changes that are not predictable.

BUG: 321602


Diffs
-

  src/dbus/mpris2/MediaPlayer2Player.cpp 
a633756bf558a89ba2a3db2307c0ebbc373a759b 

Diff: http://git.reviewboard.kde.org/r/111512/diff/


Testing
---

Tested using a tool that listens to the PropertiesChanged signal of the MPRIS2 
interface and lists when the mpris:trackid changes.

Without the patch, I get output like
Track change: "/org/kde/amarok/Track/5739423209746661216" -> 
"/org/kde/amarok/PendingTrack"
Track change: "/org/kde/amarok/PendingTrack" -> 
"/org/kde/amarok/Track/8264712350997591513"
when the track progresses because the previous track finished, and
Track change: "/org/kde/amarok/Track/5739423209746661216" -> ""
Track change: "" -> "/org/kde/amarok/Track/8264712350997591513"
when I manually change the track (eg: by clicking "next" or by double-clicking 
another track).

With the patch, I only get things like
Track change: "/org/kde/amarok/Track/5739423209746661216" -> 
"/org/kde/amarok/Track/8264712350997591513"


Thanks,

Alex Merry

___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request: MediaDeviceCache: remove polling, solid events should suffice

[Alex Merry]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105221/#review14622
---

Ship it!


Testing done: mounted an external HDD via Plasma's Device Notifier.  This made 
an item corresponding to it appear in the "Local Music" pane of Amarok.  
Unmounting it by the same method made it go away again.

- Alex Merry


On June 11, 2012, 3:14 p.m., Matěj Laitl wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105221/
> ---
> 
> (Updated June 11, 2012, 3:14 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> ---
> 
> MediaDeviceCache: remove polling, solid events should suffice
> 
> This fixes a bug where Amarok (very probably needlessly) polls solid
> for all devices every single second (!!!) just to detect whether some
> unmounted paths become mounted or vice versa. This should not be needed
> at all, solid should notify us about everything.
> 
> However, I am not sure, so this is definitely not a material for 2.6
> final but rather 2.7 if no problems show up.
> 
> BUG: 289462
> FIXED-IN: 2.7
> REVIEW: 105221
> 
> 
> This addresses bug 289462.
> https://bugs.kde.org/show_bug.cgi?id=289462
> 
> 
> Diffs
> -
> 
>   src/MediaDeviceCache.h a48d453213e684d10b0a38b5b8ac01ae39680b52 
>   src/MediaDeviceCache.cpp 15583b8d4eb14f842242deaab18bc2d7033b5991 
> 
> Diff: http://git.reviewboard.kde.org/r/105221/diff/
> 
> 
> Testing
> ---
> 
> little
> 
> 
> Thanks,
> 
> Matěj Laitl
> 
>

___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Fwd: Our research tool has generated a FAQ for Amarok

[Alex Merry]

Anyone else get these?  Not sure where they mined my address from, as 
it's not the one I put in source files.


Alex


 Original Message 
Subject:Our research tool has generated a FAQ for Amarok
Date:   Thu, 22 Sep 2011 03:37:32 +0200
From:   ste...@faqcluster.com
To: Alex Merry 



Dear Mr. Merry,

Our team at the University of Darmstadt (Germany) has created a tool which 
generates FAQs from mailing lists and forums using data mining techniques. We 
have generated a FAQ for your project and would like to have your opinion on it.

Within approx. 10 minutes, you can select and reformulate good question/answer 
pairs found by our system. Eventually, you will be able to download your FAQ in 
HTML or XML. We will also use your selected questions to further evaluate and 
improve our system.

You can review the generated FAQ for Amarok at 
http://faqcluster.com/review/1337996360.

We thank you very much in advance for your FAQ review!

Yours sincerely,

Stefan and Martin
University of Darmstadt, Germany

___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Review Request: Auto-save the playlist so it is not lost if Amarok crashes

[Alex Merry]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102444/
---

Review request for Amarok.


Summary
---

One of the things that bugs me about Amarok is that if it crashes (as it tends 
to do on logout, for me), the playlist is lost, and next time you start it up 
it gives you the playlist from the last time it exited properly.

This fixes that by auto-saving the playlist state.  It schedules a save for 5 
seconds in the future when a change is made to the playlist - any changes made 
withing those 5 seconds will not trigger a further save, but will be included 
in the scheduled save.  This prevents the disk being hammered when making a 
flurry of changes.

Possibly this timeout should be longer - 20 seconds?


Diffs
-

  ChangeLog ca2d6561ac607ad3dd4886a33a357f3add0b2eaf 
  src/playlist/PlaylistModel.h 21e5f2ca8a4fdb2ccf77c8ea2b3cb47bf2a1c5e8 
  src/playlist/PlaylistModel.cpp 842095a11f649fd9a91f5bb53869ac607a552749 

Diff: http://git.reviewboard.kde.org/r/102444/diff


Testing
---

Changed the playlist, waited 5 seconds, killed amarok (killall -9 amarok).  
Starting up Amarok again displayed the modified playlist.


Thanks,

Alex

___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request: Use KImageCache if possible, rather than KPixmapCache

[Alex Merry]

> On May 14, 2011, 11:37 a.m., Ralf Engels wrote:
> > I would like not to use seperate backends for the cache, but for now I 
> > guess there is no way around.
> > 
> > I would say ship it, but could you have a look at the code. I found several 
> > places where a QPixmapCache is used and using three kinds of caches seems 
> > to be a little bit over the top for me.

QPixmapCache does something different - it caches the actual pixmaps (on the X 
server), rather than just the image data.  I renamed my compatibility class to 
ImageCache to make the distintion more obvious, and put some apidocs comments 
in to clarify.  CoverCache should definitely be using QPixmapCache and not 
KPixmapCache/KImageCache, for example.


- Alex


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101347/#review3313
---


On May 12, 2011, 11:52 p.m., Alex Merry wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101347/
> ---
> 
> (Updated May 12, 2011, 11:52 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Summary
> ---
> 
> KPixmapCache is horribly buggy, and that is unlikely to ever change.  Its 
> replacement, KImageCache, is much better, but is only available with kdelibs 
> 4.5 and later.  So we use KImageCache if we are building against kdelibs 4.5 
> or later.
> 
> To simplify this, I've created a header-only all-inline class PixmapCache 
> which is a practically-zero-cost wrapper around either KPixmapCache or 
> KImageCache.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 8b4134f 
>   ChangeLog b278ef3 
>   config-amarok.h.cmake bcf9c8b 
>   src/App.cpp 1f01536 
>   src/MainWindow.cpp 5c14e89 
>   src/PixmapCache.h PRE-CREATION 
>   src/SvgHandler.h 7425cde 
>   src/SvgHandler.cpp 765eda0 
>   src/moodbar/MoodbarManager.h e3f32b5 
>   src/moodbar/MoodbarManager.cpp acde9e2 
>   src/widgets/BookmarkPopup.cpp 1f62537 
> 
> Diff: http://git.reviewboard.kde.org/r/101347/diff
> 
> 
> Testing
> ---
> 
> Works with latest kdelibs.  Haven't checked build against kdelibs 4.4.x.
> 
> This fixes at least one crash that I could reproduce reliably.
> 
> 
> Thanks,
> 
> Alex
> 
>

___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Review Request: Use KImageCache if possible, rather than KPixmapCache

[Alex Merry]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101347/
---

Review request for Amarok.


Summary
---

KPixmapCache is horribly buggy, and that is unlikely to ever change.  Its 
replacement, KImageCache, is much better, but is only available with kdelibs 
4.5 and later.  So we use KImageCache if we are building against kdelibs 4.5 or 
later.

To simplify this, I've created a header-only all-inline class PixmapCache which 
is a practically-zero-cost wrapper around either KPixmapCache or KImageCache.


Diffs
-

  CMakeLists.txt 8b4134f 
  ChangeLog b278ef3 
  config-amarok.h.cmake bcf9c8b 
  src/App.cpp 1f01536 
  src/MainWindow.cpp 5c14e89 
  src/PixmapCache.h PRE-CREATION 
  src/SvgHandler.h 7425cde 
  src/SvgHandler.cpp 765eda0 
  src/moodbar/MoodbarManager.h e3f32b5 
  src/moodbar/MoodbarManager.cpp acde9e2 
  src/widgets/BookmarkPopup.cpp 1f62537 

Diff: http://git.reviewboard.kde.org/r/101347/diff


Testing
---

Works with latest kdelibs.  Haven't checked build against kdelibs 4.4.x.

This fixes at least one crash that I could reproduce reliably.


Thanks,

Alex

___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Fix for MySQL embedded permissions error on upgrading to MySQL 5.5

[Alex Merry]

Isn't this what Ian's eean-mysql5.5.9fix branch fixes?

Alex



On 20/03/11 17:45, Nikhil Marathe wrote:
> Hi,
>
> If any user/developer updates to MySQL 5.5 (for example Archlinux now
> has 5.5.9), there is a bug in the MySQL build
> which stops MYSQL_HOME from being treated as the directory to search
> for my.cnf. This leads to permissions errors
> and a crash.
>
> BUG report: http://bugs.mysql.com/bug.php?id=59280
>
> The fix is to set the environment variable
> DEFAULT_HOME_ENV=$KDEHOME/share/apps/amarok
>
> $ export DEFAULT_HOME_ENV=$KDEHOME/share/apps/amarok
>
> To developers:
> This patch should fix it. In fact should we commit this to master so
> that we take care of any installations in the 5.5 range
> from the next release onwards?
>
> diff --git 
> a/src/core-impl/collections/db/sql/mysqlecollection/MySqlEmbeddedStorage.cpp
> b/src/core-impl/collections/db/sql/mysqlecollection/MySqlEmbeddedStorage.cpp
> index 258675a..f8b64b8 100644
> --- 
> a/src/core-impl/collections/db/sql/mysqlecollection/MySqlEmbeddedStorage.cpp
> +++ 
> b/src/core-impl/collections/db/sql/mysqlecollection/MySqlEmbeddedStorage.cpp
> @@ -82,6 +82,7 @@ MySqlEmbeddedStorage::MySqlEmbeddedStorage( const
> QString&storageLocation )
>   }
>
>   setenv( "MYSQL_HOME", storagePath.toAscii().data(), 1 );
> +setenv( "DEFAULT_HOME_ENV", storagePath.toAscii().data(), 1 );
>   char *args[] = { "amarok" };
>   if( mysql_library_init( 1 , args, 0 ) != 0 )
>   {
>
> I hope this saves somebody else a few hours.
>
> Best,
> Nikhil
> ___
> Amarok-devel mailing list
> Amarok-devel@kde.org
> https://mail.kde.org/mailman/listinfo/amarok-devel

___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Re: mysql 5.5 may require mysql_upgrade

[Alex Merry]

Your branch works for me (with mysql 5.5.9), but I didn't have to (and 
never have) run mysql_upgrade.

Alex



On 06/03/11 02:47, Ian Monroe wrote:
> First off mysql 5.5 apparently ignores the MYSQL_HOME, which is
> disappointing. The branch eean-mysql5.5.9fix addresses this.
>
> I had to run mysql_upgrade before it would work though.
> mysql_library_init just returned non-zero until I ran it, so I assume
> that was the problem.
>
> mysql_upgrade is generally not packaged with mysql embedded so this
> might mean a new dependency. Would be nice if we could somehow do it
> using mysql embedded. Any ideas?
>
> Ian
> ___
> Amarok-devel mailing list
> Amarok-devel@kde.org
> https://mail.kde.org/mailman/listinfo/amarok-devel

___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request: Coverbling applet (playground) build fix

[Alex Merry]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/100133/#review302
---

Ship it!


Looks good (modulo my code style comment below).


playground/src/context/applets/coverbling/CoverBlingApplet.cpp


Code style: there should be a space after m_autojump.

Actually, you could put
if ( !m_autojump )
  return
as the first line of the method, before querying the current track, but 
that doesn't really matter.


- Alex


On 2010-11-05 15:59:22, Manu Wagner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100133/
> ---
> 
> (Updated 2010-11-05 15:59:22)
> 
> 
> Review request for Amarok.
> 
> 
> Summary
> ---
> 
> This fixes CoverBling applet build (broken since yesterday's delivery).
> 
> 
> Diffs
> -
> 
>   playground/src/context/applets/coverbling/CoverBlingApplet.h 5ffc8b8 
>   playground/src/context/applets/coverbling/CoverBlingApplet.cpp d5ac4df 
> 
> Diff: http://git.reviewboard.kde.org/r/100133/diff
> 
> 
> Testing
> ---
> 
> It works fine at my end.
> Compiles and runs OK
> 
> 
> Thanks,
> 
> Manu
> 
>

___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request: Coverbling applet (playground) build fix

[Alex Merry]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/100133/#review292
---



playground/src/context/applets/coverbling/CoverBlingApplet.cpp


What happens when the value of m_autojump is changed?


- Alex


On 2010-11-04 18:39:02, Manu Wagner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100133/
> ---
> 
> (Updated 2010-11-04 18:39:02)
> 
> 
> Review request for Amarok.
> 
> 
> Summary
> ---
> 
> This fixes CoverBling applet build (broken since yesterday's delivery).
> 
> 
> Diffs
> -
> 
>   playground/src/context/applets/coverbling/CoverBlingApplet.cpp d5ac4df 
>   playground/src/context/applets/coverbling/CoverBlingApplet.h 5ffc8b8 
> 
> Diff: http://git.reviewboard.kde.org/r/100133/diff
> 
> 
> Testing
> ---
> 
> It works fine at my end.
> Compiles and runs OK
> 
> 
> Thanks,
> 
> Manu
> 
>

___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request: Remove EngineObserver and replacing it with signals/slots for better thread safety

[Alex Merry]

> On 2010-10-31 20:14:08, Alex Merry wrote:
> > src/EngineController.h, line 350
> > <http://git.reviewboard.kde.org/r/100120/diff/1/?file=2895#file2895line350>
> >
> > But not when it resumes, right?  This should be clarified in the docs.
> 
> Ralf Engels wrote:
> Wow. very pedantic. 
> For resuming we have the trackPlaying signal.
> But I will update the comments anyway.

Yes, I am when it comes to apidocs :-)

My attitude is that apidocs should be detailed enough that you shouldn't have 
to read the code to use the API.  This is one of those things where I look at 
it and go "well, it probably isn't triggered by a resume, but I'm not certain - 
I'll check the code".  Better to make that explicit.


> On 2010-10-31 20:14:08, Alex Merry wrote:
> > src/EngineController.h, line 425
> > <http://git.reviewboard.kde.org/r/100120/diff/1/?file=2895#file2895line425>
> >
> > The apidocs should be clearer about "userSeek" - when it is false, it 
> > means that the position change is due to normal playback progression.
> > 
> > Perhaps this should be split into two signals, one for each value of 
> > userSeek - would this be more efficient, with a reduced number of listeners 
> > when the track is just progressing normally?
> > 
> > Perhaps seeked( qint64 position ) and trackPositionChanged( qint64 
> > position ), with the former being connected to the latter?
> 
> Ralf Engels wrote:
> Not many objects are connecting to this signal and I have not seen that 
> as a big problem.
> None of the connected objects cared about userSeek so it might as well be 
> removed.

None?  I'm pretty sure the MPRIS interface makes use of the userSeek attribute.


- Alex


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/100120/#review239
---


On 2010-10-31 15:53:10, Ralf Engels wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100120/
> ---
> 
> (Updated 2010-10-31 15:53:10)
> 
> 
> Review request for Amarok.
> 
> 
> Summary
> ---
> 
> Remove EngineObserver and the observer pattern.
> Instead we have several new signals and functions in EngineController.
> Also the EngineController is now a MetaObserver for the current track and 
> exporting the relevant signals.
> 
> This patch improves the tread safety and reduces the complexity of the code 
> by a lot.
> Over 600 lines of codes could be removed while at the same time fixing the 
> image update problems and several small problems where classes listened for 
> the album meta changed but forgot that the album could change when a user 
> changed the track.
> 
> 
> Diffs
> -
> 
>   playground/src/context/applets/coverbling/CoverBlingApplet.h a3ff113 
>   playground/src/context/applets/coverbling/CoverBlingApplet.cpp a2a50e7 
>   playground/src/context/applets/video/Video.h 1d52f0b 
>   playground/src/context/applets/video/Video.cpp 8471497 
>   src/ActionClasses.h aa67ec7 
>   src/ActionClasses.cpp 61e8af8 
>   src/App.cpp e6006b9 
>   src/EngineController.h aae4503 
>   src/EngineController.cpp 65b3f2e 
>   src/KNotificationBackend.h 02cc9d8 
>   src/KNotificationBackend.cpp b98391c 
>   src/MainWindow.h 4593d47 
>   src/MainWindow.cpp 9102558 
>   src/TrayIcon.h d1ffd43 
>   src/TrayIcon.cpp 47f56bf 
>   src/amarokurls/AmarokUrlHandler.cpp 1d8cd46 
>   src/context/ContextView.h a3f36fb 
>   src/context/ContextView.cpp ccfe4f3 
>   src/context/applets/photos/PhotosApplet.h 4cac9ea 
>   src/context/applets/photos/PhotosApplet.cpp 0b47da9 
>   src/context/applets/similarartists/SimilarArtistsApplet.h 3a2d7ea 
>   src/context/applets/similarartists/SimilarArtistsApplet.cpp 0cf2c76 
>   src/context/applets/videoclip/VideoclipApplet.h a0704c6 
>   src/context/applets/videoclip/VideoclipApplet.cpp 789173e 
>   src/context/engines/current/CurrentEngine.h 0369065 
>   src/context/engines/current/CurrentEngine.cpp 2c32f75 
>   src/context/engines/labels/LabelsEngine.h 626a030 
>   src/context/engines/labels/LabelsEngine.cpp 65278b4 
>   src/context/engines/lyrics/LyricsEngine.h 771bf6f 
>   src/context/engines/lyrics/LyricsEngine.cpp 527cd0d 
>   src/context/engines/similarartists/SimilarArtistsEngine.h fea9bf5 
>   src/context/engines/similarartists/SimilarArtistsEngine.cpp 97bc109 
>   src/context/engines/upcomingevents/Upcomin

Re: Review Request: Remove EngineObserver and replacing it with signals/slots for better thread safety

[Alex Merry]

> On 2010-10-31 20:14:08, Alex Merry wrote:
> > src/EngineController.h, line 117
> > <http://git.reviewboard.kde.org/r/100120/diff/1/?file=2895#file2895line117>
> >
> > Delete stuff rather than commenting it :-)
> 
> Ralf Engels wrote:
> It was unused (after my refactoring) and redundant as you can get the 
> state from the media object.

But that doesn't change my comment - you should delete the code if it's not 
needed.  It can always be reclaimed from the git history.


- Alex


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/100120/#review239
---


On 2010-10-31 15:53:10, Ralf Engels wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100120/
> ---
> 
> (Updated 2010-10-31 15:53:10)
> 
> 
> Review request for Amarok.
> 
> 
> Summary
> ---
> 
> Remove EngineObserver and the observer pattern.
> Instead we have several new signals and functions in EngineController.
> Also the EngineController is now a MetaObserver for the current track and 
> exporting the relevant signals.
> 
> This patch improves the tread safety and reduces the complexity of the code 
> by a lot.
> Over 600 lines of codes could be removed while at the same time fixing the 
> image update problems and several small problems where classes listened for 
> the album meta changed but forgot that the album could change when a user 
> changed the track.
> 
> 
> Diffs
> -
> 
>   playground/src/context/applets/coverbling/CoverBlingApplet.h a3ff113 
>   playground/src/context/applets/coverbling/CoverBlingApplet.cpp a2a50e7 
>   playground/src/context/applets/video/Video.h 1d52f0b 
>   playground/src/context/applets/video/Video.cpp 8471497 
>   src/ActionClasses.h aa67ec7 
>   src/ActionClasses.cpp 61e8af8 
>   src/App.cpp e6006b9 
>   src/EngineController.h aae4503 
>   src/EngineController.cpp 65b3f2e 
>   src/KNotificationBackend.h 02cc9d8 
>   src/KNotificationBackend.cpp b98391c 
>   src/MainWindow.h 4593d47 
>   src/MainWindow.cpp 9102558 
>   src/TrayIcon.h d1ffd43 
>   src/TrayIcon.cpp 47f56bf 
>   src/amarokurls/AmarokUrlHandler.cpp 1d8cd46 
>   src/context/ContextView.h a3f36fb 
>   src/context/ContextView.cpp ccfe4f3 
>   src/context/applets/photos/PhotosApplet.h 4cac9ea 
>   src/context/applets/photos/PhotosApplet.cpp 0b47da9 
>   src/context/applets/similarartists/SimilarArtistsApplet.h 3a2d7ea 
>   src/context/applets/similarartists/SimilarArtistsApplet.cpp 0cf2c76 
>   src/context/applets/videoclip/VideoclipApplet.h a0704c6 
>   src/context/applets/videoclip/VideoclipApplet.cpp 789173e 
>   src/context/engines/current/CurrentEngine.h 0369065 
>   src/context/engines/current/CurrentEngine.cpp 2c32f75 
>   src/context/engines/labels/LabelsEngine.h 626a030 
>   src/context/engines/labels/LabelsEngine.cpp 65278b4 
>   src/context/engines/lyrics/LyricsEngine.h 771bf6f 
>   src/context/engines/lyrics/LyricsEngine.cpp 527cd0d 
>   src/context/engines/similarartists/SimilarArtistsEngine.h fea9bf5 
>   src/context/engines/similarartists/SimilarArtistsEngine.cpp 97bc109 
>   src/context/engines/upcomingevents/UpcomingEventsEngine.h c57b004 
>   src/context/engines/upcomingevents/UpcomingEventsEngine.cpp 66131fd 
>   src/context/engines/wikipedia/WikipediaEngine.h 5d53082 
>   src/context/engines/wikipedia/WikipediaEngine.cpp 4c65799 
>   src/core-impl/meta/stream/Stream_p.h cd217bf 
>   src/core-impl/meta/timecode/TimecodeObserver.h b727196 
>   src/core-impl/meta/timecode/TimecodeObserver.cpp dea809d 
>   src/core/CMakeLists.txt 5863ca1 
>   src/core/engine/EngineObserver.h 5a93062 
>   src/core/engine/EngineObserver.cpp 7d5728b 
>   src/dbus/mpris1/PlayerHandler.h 8f90f27 
>   src/dbus/mpris1/PlayerHandler.cpp 0afeb59 
>   src/dbus/mpris2/Mpris2DBusHandler.h a767c79 
>   src/dbus/mpris2/Mpris2DBusHandler.cpp 59afbf3 
>   src/dialogs/ScriptManager.h 6104dcf 
>   src/dialogs/ScriptManager.cpp e98f45f 
>   src/dynamic/BiasSolver.cpp bf8e3c8 
>   src/dynamic/biases/EchoNest.h 9a6ecc7 
>   src/dynamic/biases/EchoNest.cpp 16f26f4 
>   src/mac/GrowlInterface.h 3bb35d2 
>   src/mac/GrowlInterface.cpp 862d019 
>   src/playlist/PlaylistActions.h 035ff77 
>   src/playlist/PlaylistActions.cpp 2358b29 
>   src/playlist/PlaylistController.cpp d38c9d6 
>   src/playlist/view/listview/PrettyItemDelegate.cpp ab19ccc 
>   src/scriptengine/AmarokEngineScript.h 55e275c 
>   src/scriptengine/Amar