iiiypuk/cantata
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

- Fix memleak when copying items to/from devices.

Browse Source 
- When creating temp files, ensure these are in /tmp!
- If applying various artist workaround for a remote device, apply the workaround to a local temp file, and send this.
This commit is contained in:
craig.p.drummond
2012-12-03 20:02:02 +00:00
parent eba6060e1b
commit ca96ddc8c9
12 changed files with 180 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
ChangeLog
View File

@@ -116,6 +116,10 @@
MPD.
67. In sync dialog, when detecting items unique to library/device, revert
various artist work-around for each track if it is enabled on the device.
68. Fix memleak when copying items to/from devices.
69. When creating temp files, ensure these are in /tmp!
70. If applying various artist workaround for a remote device, apply the
workaround to a local temp file, and send this.
0.8.3.1
-------

27
devices/device.cpp
View File

@@ -26,7 +26,7 @@
#include "covers.h"
#include "utils.h"
#include <QtCore/QDir>
#include <QtCore/QTemporaryFile>
#ifndef Q_OS_WIN
#include "devicesmodel.h"
#include "umsdevice.h"
@@ -191,6 +191,31 @@ bool Device::fixVariousArtists(const QString &file, Song &song, bool applyFix)
return false;
}
QTemporaryFile * Device::copySongToTemp(Song &song)
{
QTemporaryFile *temp=new QTemporaryFile();
int index=song.file.lastIndexOf('.');
if (index>0) {
temp=new QTemporaryFile(QDir::tempPath()+"/cantata_XXXXXX"+song.file.mid(index));
} else {
temp=new QTemporaryFile(QDir::tempPath()+"/cantata_XXXXXX");
}
temp->setAutoRemove(false);
if (temp->open()) {
temp->close();
if (QFile::exists(temp->fileName())) {
QFile::remove(temp->fileName()); // Copy will *not* overwrite file!
}
if (!QFile::copy(song.file, temp->fileName())) {
temp->remove();
temp=0;
}
}
return temp;
}
void Device::applyUpdate()
{
if (!update) {

2
devices/device.h
View File

@@ -45,6 +45,7 @@ namespace Device
class QWidget;
class QImage;
class QTemporaryFile;
class DevicesModel;
class Device : public MusicLibraryItemRoot
@@ -54,6 +55,7 @@ class Device : public MusicLibraryItemRoot
public:
static Device * create(DevicesModel *m, const QString &udi);
static bool fixVariousArtists(const QString &file, Song &song, bool applyFix);
static QTemporaryFile * copySongToTemp(Song &song);
static void moveDir(const QString &from, const QString &to, const QString &base, const QString &coverFile);
static void cleanDir(const QString &dir, const QString &base, const QString &coverFile, int level=0);

2
devices/encoders.cpp
View File

@@ -309,7 +309,7 @@ bool Encoder::isDifferent(const QString &file)
return file!=changeExtension(file);
}
QStringList Encoder::params(int value, const QString &in, const QString &out)
QStringList Encoder::params(int value, const QString &in, const QString &out) const
{
QStringList p;
p << ffmpeg << QLatin1String("-i") << in << QLatin1String("-threads") << QLatin1String("0") << QLatin1String(usingAvconv ? "-c:a" : "-acodec") << codec;

2
devices/encoders.h
View File

@@ -61,7 +61,7 @@ namespace Encoders
}
QString changeExtension(const QString &file);
bool isDifferent(const QString &file);
QStringList params(int value, const QString &in, const QString &out);
QStringList params(int value, const QString &in, const QString &out) const;
QString name;
QString description;
QString tooltip;

42
devices/filejob.cpp
View File

@@ -29,6 +29,7 @@
#include <QtCore/QFile>
#include <QtCore/QThread>
#include <QtCore/QTimer>
#include <QtCore/QTemporaryFile>
#ifdef ENABLE_KDE_SUPPORT
#include <KDE/KGlobal>
@@ -71,6 +72,14 @@ void FileScheduler::stop()
}
}
FileJob::FileJob()
: stopRequested(false)
, progressPercent(0)
{
FileScheduler::self()->addJob(this);
connect(this, SIGNAL(result(int)), SLOT(deleteLater()));
}
void FileJob::start()
{
QTimer::singleShot(0, this, SLOT(run()));
@@ -84,10 +93,36 @@ void FileJob::setPercent(int pc)
}
}
CopyJob::~CopyJob()
{
if (temp) {
temp->remove();
delete temp;
}
}
static const int constChunkSize=32*1024;
void CopyJob::run()
{
QString origSrcFile(srcFile);
// First, if we are going to update tags (e.g. to fix various artists), then check if we want to do that locally, before
// copying to device. For UMS devices, we just modify on device, but for remote (e.g. sshfs) then it'd be better to do locally :-)
if (copyOpts&OptsFixLocal && (copyOpts&OptsApplyVaFix || copyOpts&OptsUnApplyVaFix)) {
song.file=srcFile;
temp=Device::copySongToTemp(song);
if (!temp || !Device::fixVariousArtists(temp->fileName(), song, copyOpts&OptsApplyVaFix)) {
emit result(StatusFailed);
return;
}
srcFile=temp->fileName();
}
if (stopRequested) {
emit result(StatusCancelled);
return;
}
QFile src(srcFile);
if (!src.open(QIODevice::ReadOnly)) {
@@ -145,9 +180,12 @@ void CopyJob::run()
}
} while ((readPos+bytesRead)<totalBytes);
if (copyCover) {
if (!stopRequested && !(copyOpts&OptsFixLocal) && (copyOpts&OptsApplyVaFix || copyOpts&OptsUnApplyVaFix)) {
Device::fixVariousArtists(destFile, song, copyOpts&OptsApplyVaFix);
}
if (!stopRequested && copyCover) {
song.file=destFile;
Covers::copyCover(song, Utils::getDir(srcFile), Utils::getDir(destFile), coverOpts.name, coverOpts.maxSize);
Covers::copyCover(song, Utils::getDir(origSrcFile), Utils::getDir(destFile), coverOpts.name, coverOpts.maxSize);
}
setPercent(100);
emit result(StatusOk);

25
devices/filejob.h
View File

@@ -29,6 +29,7 @@
#include "song.h"
#include "fsdevice.h"
class QTemporaryFile;
class QThread;
class FileJob;
@@ -56,11 +57,7 @@ public:
StatusCancelled
};
FileJob()
: stopRequested(false)
, progressPercent(0) {
FileScheduler::self()->addJob(this);
}
FileJob();
virtual ~FileJob() { }
void setPercent(int pc);
@@ -85,19 +82,33 @@ protected:
class CopyJob : public FileJob
{
public:
CopyJob(const QString &src, const QString &dest, const FsDevice::CoverOptions &c, const Song &s)
enum Options
{
OptsNone = 0x00,
OptsApplyVaFix = 0x01,
OptsUnApplyVaFix = 0x02,
OptsFixLocal = 0x04 // Apply any fixes to a local temp file before sending...
};
CopyJob(const QString &src, const QString &dest, const FsDevice::CoverOptions &c, int co, Song &s)
: srcFile(src)
, destFile(dest)
, coverOpts(c)
, song(s) {
, copyOpts(co)
, song(s)
, temp(0) {
}
virtual ~CopyJob();
private:
void run();
private:
QString srcFile;
QString destFile;
FsDevice::CoverOptions coverOpts;
int copyOpts;
Song song;
QTemporaryFile *temp;
};
class DeleteJob : public FileJob

18
devices/fsdevice.cpp
View File

@@ -245,13 +245,17 @@ void FsDevice::addSong(const Song &s, bool overwrite)
currentSong=s;
if (encoder.codec.isEmpty() || (opts.transcoderWhenDifferent && !encoder.isDifferent(s.file))) {
transcoding=false;
CopyJob *job=new CopyJob(s.file, destFile, coverOpts, currentSong);
CopyJob *job=new CopyJob(s.file, destFile, coverOpts, (needToFixVa ? CopyJob::OptsApplyVaFix : CopyJob::OptsNone)|
(Device::RemoteFs==devType() ? CopyJob::OptsFixLocal : CopyJob::OptsNone),
currentSong);
connect(job, SIGNAL(result(int)), SLOT(addSongResult(int)));
connect(job, SIGNAL(percent(int)), SLOT(percent(int)));
job->start();
} else {
transcoding=true;
TranscodingJob *job=new TranscodingJob(encoder.params(opts.transcoderValue, s.file, destFile));
TranscodingJob *job=new TranscodingJob(encoder, opts.transcoderValue, s.file, destFile, coverOpts,
(needToFixVa ? CopyJob::OptsApplyVaFix : CopyJob::OptsNone)|
(Device::RemoteFs==devType() ? CopyJob::OptsFixLocal : CopyJob::OptsNone));
connect(job, SIGNAL(result(int)), SLOT(addSongResult(int)));
connect(job, SIGNAL(percent(int)), SLOT(percent(int)));
job->start();
@@ -302,7 +306,7 @@ void FsDevice::copySongTo(const Song &s, const QString &baseDir, const QString &
}
currentSong=s;
CopyJob *job=new CopyJob(source, dest, CoverOptions(), currentSong);
CopyJob *job=new CopyJob(source, dest, CoverOptions(), needToFixVa ? CopyJob::OptsUnApplyVaFix : CopyJob::OptsNone, currentSong);
connect(job, SIGNAL(result(int)), SLOT(copySongToResult(int)));
connect(job, SIGNAL(percent(int)), SLOT(percent(int)));
job->start();
@@ -388,13 +392,9 @@ void FsDevice::addSongResult(int status)
if (FileJob::StatusOk!=status) {
emit actionStatus(transcoding ? TranscodeFailed : Failed);
} else {
if (transcoding && Device::constNoCover!=coverOpts.name) {
// This copy should really be in transcoding thread - but it should be quick enough in the GUI thread anyway...
Covers::copyCover(currentSong, Utils::getDir(currentSong.file), Utils::getDir(destFileName), coverOpts.name, coverOpts.maxSize);
}
currentSong.file=destFileName;
if (needToFixVa) {
Device::fixVariousArtists(audioFolder+destFileName, currentSong, true);
currentSong.fixVariousArtists();
}
addSongToList(currentSong);
emit actionStatus(Ok);
@@ -416,7 +416,7 @@ void FsDevice::copySongToResult(int status)
} else {
currentSong.file=currentMusicPath; // MPD's paths are not full!!!
if (needToFixVa) {
Device::fixVariousArtists(currentBaseDir+currentSong.file, currentSong, false);
currentSong.revertVariousArtists();
}
Utils::setFilePerms(currentBaseDir+currentSong.file);
MusicLibraryModel::self()->addSongToList(currentSong);

39
devices/mtpdevice.cpp
View File

@@ -530,21 +530,10 @@ void MtpConnection::putSong(const Song &s, bool fixVa)
if (fixVa) {
// Need to 'workaround' broken various artists handling, so write to a temporary file first...
int index=song.file.lastIndexOf('.');
if (index>0) {
temp=new QTemporaryFile("cantata_XXXX"+song.file.mid(index));
} else {
temp=new QTemporaryFile("cantata_XXXX");
}
temp->setAutoRemove(false);
if (temp->open()) {
fileName=temp->fileName();
temp->close();
if (QFile::exists(fileName)) {
QFile::remove(fileName); // Copy will *not* overwrite file!
}
if (QFile::copy(song.file, fileName) && Device::fixVariousArtists(fileName, song, true)) {
song.file=fileName;
temp=Device::copySongToTemp(song);
if (temp) {
song.file=temp->fileName();
if (Device::fixVariousArtists(temp->fileName(), song, true)) {
fixedVa=true;
}
}
@@ -649,7 +638,7 @@ void MtpConnection::putSong(const Song &s, bool fixVa)
QTemporaryFile *temp=0;
// Cantata covers are either JPG or PNG. Assume device supports JPG...
if (image.fileName.isEmpty() || (image.fileName.endsWith(".png ") && !supportedTypes.contains(LIBMTP_FILETYPE_PNG))) {
temp=new QTemporaryFile("cantata_XXXX.jpg");
temp=new QTemporaryFile(QDir::tempPath()+"/cantata_XXXXXX.jpg");
temp->setAutoRemove(true);
if (!image.img.save(temp, "JPG")) {
image.fileName=QString();
@@ -713,10 +702,10 @@ static void saveFile(const QString &name, char *data, uint64_t size)
}
}
void MtpConnection::getSong(const Song &song, const QString &dest)
void MtpConnection::getSong(const Song &song, const QString &dest, bool fixVa)
{
bool copiedSong=device && 0==LIBMTP_Get_File_To_File(device, song.id, dest.toUtf8(), &progressMonitor, this);
if (copiedSong) {
if (copiedSong && !abortRequested()) {
QString destDir=Utils::getDir(dest);
if (QDir(destDir).entryList(QStringList() << QLatin1String("*.jpg") << QLatin1String("*.png"), QDir::Files|QDir::Readable).isEmpty()) {
@@ -744,6 +733,10 @@ void MtpConnection::getSong(const Song &song, const QString &dest)
}
}
}
if (copiedSong && fixVa && !abortRequested()) {
Song s(song);
Device::fixVariousArtists(dest, s, false);
}
emit getSongStatus(copiedSong);
}
@@ -1069,7 +1062,7 @@ void MtpDevice::addSong(const Song &s, bool overwrite)
if (!opts.transcoderWhenDifferent || encoder.isDifferent(s.file)) {
deleteTemp();
tempFile=new QTemporaryFile("cantata_XXXX"+encoder.extension);
tempFile=new QTemporaryFile(QDir::tempPath()+"/cantata_XXXXXX"+encoder.extension);
tempFile->setAutoRemove(false);
if (!tempFile->open()) {
@@ -1083,7 +1076,7 @@ void MtpDevice::addSong(const Song &s, bool overwrite)
QFile::remove(destFile);
}
transcoding=true;
TranscodingJob *job=new TranscodingJob(encoder.params(opts.transcoderValue, s.file, destFile));
TranscodingJob *job=new TranscodingJob(encoder, opts.transcoderValue, s.file, destFile);
connect(job, SIGNAL(result(int)), SLOT(transcodeSongResult(int)));
connect(job, SIGNAL(percent(int)), SLOT(transcodePercent(int)));
job->start();
@@ -1133,7 +1126,7 @@ void MtpDevice::copySongTo(const Song &s, const QString &baseDir, const QString
}
currentSong=s;
emit getSong(s, currentBaseDir+currentMusicPath);
emit getSong(s, currentBaseDir+currentMusicPath, needToFixVa);
}
void MtpDevice::removeSong(const Song &s)
@@ -1184,7 +1177,7 @@ void MtpDevice::putSongStatus(bool ok, int id, const QString &file, bool fixedVa
currentSong.id=id;
currentSong.file=file;
if (needToFixVa && fixedVa) {
Device::fixVariousArtists(QString(), currentSong, true);
currentSong.fixVariousArtists();
} else if (!opts.fixVariousArtists) { // TODO: ALBUMARTIST: Remove when libMTP supports album artist!
currentSong.albumartist=currentSong.artist;
}
@@ -1236,7 +1229,7 @@ void MtpDevice::getSongStatus(bool ok)
} else {
currentSong.file=currentMusicPath; // MPD's paths are not full!!!
if (needToFixVa) {
Device::fixVariousArtists(currentBaseDir+currentSong.file, currentSong, false);
currentSong.revertVariousArtists();
}
Utils::setFilePerms(currentBaseDir+currentSong.file);
MusicLibraryModel::self()->addSongToList(currentSong);

4
devices/mtpdevice.h
View File

@@ -67,7 +67,7 @@ public Q_SLOTS:
void disconnectFromDevice(bool showStatus=true);
void updateLibrary();
void putSong(const Song &song, bool fixVa);
void getSong(const Song &song, const QString &dest);
void getSong(const Song &song, const QString &dest, bool fixVa);
void delSong(const Song &song);
void cleanDirs(const QSet<QString> &dirs);
void getCover(const Song &song);
@@ -146,7 +146,7 @@ Q_SIGNALS:
// These are for talking to connection thread...
void updateLibrary();
void putSong(const Song &song, bool fixVa);
void getSong(const Song &song, const QString &dest);
void getSong(const Song &song, const QString &dest, bool fixVa);
void delSong(const Song &song);
void cleanMusicDirs(const QSet<QString> &dirs);
void getCover(const Song &s);

58
devices/transcodingjob.cpp
View File

@@ -21,28 +21,60 @@
*/
#include "transcodingjob.h"
#include "device.h"
#include "covers.h"
#include <QtCore/QTemporaryFile>
TranscodingJob::TranscodingJob(const QStringList &params)
: parameters(params)
TranscodingJob::TranscodingJob(const Encoders::Encoder &enc, int val, const QString &src, const QString &dest, const FsDevice::CoverOptions &c, int opts, const Song &s)
: encoder(enc)
, value(val)
, srcFile(src)
, destFile(dest)
, coverOpts(c)
, copyOpts(opts)
, song(s)
, process(0)
, duration(-1)
, temp(0)
{
}
TranscodingJob::~TranscodingJob()
{
if (temp) {
temp->remove();
delete temp;
}
delete process;
}
void TranscodingJob::run()
{
process = new QProcess;
process->setReadChannelMode(QProcess::MergedChannels);
process->setReadChannel(QProcess::StandardOutput);
connect(process, SIGNAL(readyReadStandardOutput()), this, SLOT(processOutput()));
connect(process, SIGNAL(finished(int, QProcess::ExitStatus)), this, SLOT(finished(int, QProcess::ExitStatus)));
QString cmd=parameters.takeFirst();
process->start(cmd, parameters);
QString src(srcFile);
// First, if we are going to update tags (e.g. to fix various artists), then check if we want to do that locally, before
// copying to device. For UMS devices, we just modify on device, but for remote (e.g. sshfs) then it'd be better to do locally :-)
if (copyOpts&CopyJob::OptsFixLocal && (copyOpts&CopyJob::OptsApplyVaFix || copyOpts&CopyJob::OptsUnApplyVaFix)) {
song.file=srcFile;
temp=Device::copySongToTemp(song);
if (!temp || !Device::fixVariousArtists(temp->fileName(), song, copyOpts&CopyJob::OptsApplyVaFix)) {
emit result(StatusFailed);
return;
}
src=temp->fileName();
}
if (stopRequested) {
emit result(StatusCancelled);
} else {
QStringList parameters=encoder.params(value, src, destFile);
process = new QProcess;
process->setReadChannelMode(QProcess::MergedChannels);
process->setReadChannel(QProcess::StandardOutput);
connect(process, SIGNAL(readyReadStandardOutput()), this, SLOT(processOutput()));
connect(process, SIGNAL(finished(int, QProcess::ExitStatus)), this, SLOT(finished(int, QProcess::ExitStatus)));
QString cmd=parameters.takeFirst();
process->start(cmd, parameters);
}
}
void TranscodingJob::stop()
@@ -65,12 +97,20 @@ void TranscodingJob::finished(int exitCode, QProcess::ExitStatus exitStatus)
emit result(StatusCancelled);
return;
}
if (!stopRequested && 0==exitCode && !(copyOpts&CopyJob::OptsFixLocal) && (copyOpts&CopyJob::OptsApplyVaFix || copyOpts&CopyJob::OptsUnApplyVaFix)) {
Device::fixVariousArtists(destFile, song, copyOpts&CopyJob::OptsApplyVaFix);
}
if (!stopRequested && 0==exitCode && Device::constNoCover!=coverOpts.name) {
song.file=destFile;
Covers::copyCover(song, Utils::getDir(srcFile), Utils::getDir(destFile), coverOpts.name, coverOpts.maxSize);
}
emit result(0==exitCode ? StatusOk : StatusFailed);
}
void TranscodingJob::processOutput()
{
if (stopRequested) {
emit result(StatusCancelled);
return;
}
QString output = process->readAllStandardOutput().data();

15
devices/transcodingjob.h
View File

@@ -23,6 +23,7 @@
#define TRANSCODING_JOB_H
#include "filejob.h"
#include "encoders.h"
#include <QtCore/QStringList>
#include <QtCore/QProcess>
@@ -30,8 +31,9 @@ class TranscodingJob : public FileJob
{
Q_OBJECT
public:
explicit TranscodingJob(const QStringList &params);
~TranscodingJob();
explicit TranscodingJob(const Encoders::Encoder &enc, int val, const QString &src, const QString &dest,
const FsDevice::CoverOptions &c=FsDevice::CoverOptions(), int opts=0, const Song &s=Song());
virtual ~TranscodingJob();
void stop();
@@ -47,10 +49,17 @@ private:
inline qint64 computeProgress(const QString &output);
private:
QStringList parameters;
const Encoders::Encoder &encoder;
int value;
QString srcFile;
QString destFile;
FsDevice::CoverOptions coverOpts;
int copyOpts;
Song song;
QProcess *process;
qint64 duration; //in csec
QString data;
QTemporaryFile *temp;
};