Use qLogCat to put verbose GStreamer callback messages into a new
GstEnginePipelineCallbacks category. Filter that category instead of
the entire class by default.
The fix-up for URLs for files that that begin with // no longer works since the
QUrl class determines that these modifications are invalid, resulting in an
empty string when converted. Instead of attempting to modify the QUrl, add a
utility function that makes the correction on the encoded byte array at time of
usage.
When a directory is removed from the library during a scan, the scan continues
until complete. This change cancels the scan immediately, unblocking the watcher
thread, then signals the watcher to remove the directory.
A second issue occurs when a previously scanned device is removed during a
scan. All remaining files will be marked as deleted. This change mitigates
this issue, but a timing hole still remains here.
This is can be done without protecting the directory reference since the method
that removes directories from the watch list is only called on the same thread
as the scan, and never during the life of the ScanTransaction object.
When scanning a device such as a mobile phone, it's likely that a directory
containing both image and audio files will be found. Besides the appearance of a
hung scan, it's unlikely that a relevant image will be found.
In the case where filters for relevant filenames don't yield results, set a
sanity limit on the number of images. If the list size is beyond than that
threshold, return an empty path.
On Linux systems, failure to watch a path may be caused by the limit set in
/proc/sys/fs/inotify/max_user_watches. This can be demonstrated by creating
a directory with a large number of empty subdirectories and adding that test
directory as a library.
Check that a file is readable before adding a watch. If adding the watch fails,
report the error to the user only once. Only add the path to subdir_mapping_
if watch succeeds.
QFileSystemWatcher::addPath returns a boolean to indicate success. Modify
QtFSListener::AddPath to reflect that. For now, the MacFSListener version will
always return true.
Using libmtp or libgphoto2 to access a device that gvfs has mounted causes the
connection to fail. libmtp is calling libusb_claim_interface which, according to
libusb documentation, will return LIBUSB_ERROR_BUSY if claimed by a different
program. For mtp and gphoto2 devices discovered by the GioLister, use the file
scheme and access the device through the gvfs mount.
The uri returned from g_file_get_uri is percent encoded. This causes the regex
in GioLister::MakeDeviceUrls to fail and causes the URL to be invalid. In this
case, it falls back to the file scheme. Newer versions of gvfs obtain the serial
id from udev instead of using the bus and device IDs.
Note that this bug covers a different issue where mtp is failing to connect. The
result is actually desired behavior. The follow-up change will address this.
A LibraryBackend may be deleted while an associated LibraryModel object is using
it. An example is an async query running while a connected device is removed.
To prevent this, use a share pointer for the LibraryBackend.
This fixes one case where LibraryBackend is used after deletion. However, the
raw pointer is still passed around in several other places. These should be
evaluated on a case-by-case basis to insure that circular depencencies aren't
introduced.
Create a thread pool for each LibraryModel object and block destruction until
all threads that are operating on this object are complete.
Note that this is not a complete solution. The async query also uses the library
backend which may still be deleted before the thread exits. This will be
addressed in a future change.
A commit in qt 5.7 changes a qWarning to a qFatal if a QThread is still running
when it's deleted. When we get the LoadFinished signal in MtpDevice, stop
the loader thread's event loop to avoid this situation.
See qtbase commit c8277b6e532
Add an "Advanced Settings" option to the gpodder sign in. If selected, a fully
qualified URL must be specified as the gpodder base. Upon successful login, the
URL is saved along with username and password. If advanced settings are not
selected, an empty URL is stored and the default will be used.
When QProxyStyle is given a base style, it take ownership of that object.
PlaylistView creates a proxy style based on its own style, but that is a shared
resource. When the PlayListView is destroyed, this object is destroyed.
Instead of passing style() to QStyle, pass nullptr. This will use the native
style.
It's not necessary for the PodcastSettingsPage class to have knowledge of
GPodderSync's login implementation. Handling the network reply in a single
location sightly simplifies the code. It also makes the handling order
more deterministic.
A sessionid cookie is stored when logging in to gpodder. After logging out, a
subsequent login with the same user name but incorrect password will succeed,
ignoring the authorization header. The incorrect password will be stored for
future use.
To fix this, reset the cookie jar for GPodderSync's network access manager at
logout.
A closure created by NewClosure that handles Qt signals is destroyed if the
signal object is destroyed, the slot object is destroyed, or the signal is
invoked. In the case where the sender is passed as a shared pointer, the
reference prevents the sender from being destroyed before the closure.
So for closures built to handle responses returned from ApiRequest in
GPodderSync, the closure object and the response object will only be destroyed
after the signal is invoked. In some cases, separate closures are built for
error signals as well. For these, only one closure will be destroyed. The other
closures and the response object will be leaked.
A simple fix for the success cases is to remove the unnecessary error case
closures and directly connect the signals to slots. This is low hanging fruit
and still leaves leaks in the error cases. Those cases will require a more
complete solution to properly manage the life cycle of the response object.
In most cases, timeouts can be applied to a reply after a request has been made.
But some APIs, such as libmygpo-qt, don't always provide access to the reply or
provide abort methods. For these cases, add an optional timeout to
NetworkAccessManager. If set, create a NetworkTimeouts instance in createRequest
and add the reply. Use the reply as the parent so that it is destroyed when the
reply is destroyed.
Apparently this was introduced in 2011 as a workaround to solve a problem in
libimobiledevice. However already in 2013 the problem was solved
in libimobiledevice:
b811fbb05b
Queue2 tends to hang up on pause, unable to start playing
again. Pipeline actually stays PLAYING with ASYNC state
change, so it becomes impossible to unpause the player
without stop or forward/backward seeks.
This reverts commit 2b280de663.
Instead of immediately saving, which leads to poor performance,
and possible hardware damage (see #6057), limit saves to once
per second (similar to how KDE does it). It also guarantees
that only one save is required per second, by sharing a QSettings
object, and establishes a signaling framework to put other
setting save events into (but only uses this for the two major
offenders: playlist tab switching and window resizing).
This is in contrast to 6a312e7, which simply deferred the save
until program exit, and caused problems for some people (see #6217
and #6209).
Signed-off-by: Antonio Russo <antonio.e.russo@gmail.com>
Previously, the number of processes spawned was always
QThread::idealThreadCount() (returning the number of logical CPU
cores). On new systems with many cores, however, this can result
in 12, 16, 24, or ... processes being spawned, which is a bit
excessive.
This establishes a new config variable,
'max_numprocs_tagclients' within the Settings group, in order
to limit the maximum number of tag client processes that get
spawned. It also adds a means of setting this via the Behavior
page in Settings. It can be set to any integer in the interval
[1, QThread::idealThreadCount()]; it defaults to the maximal value
so as to emulate the old behavior.
"Clementine" (as returned by QCoreApplication::applicationName())
does not match the system .desktop file name (but it may match user
.desktop files, as was the case for me); Clementine won't be picked up
as an application in KDE Plasma notification settings unless it case
matches.
Qt 5.11 added a function allowing the first column in a list to be set
to be movable. Contingent on its availability, make the first column
of the playlist viewer draggable.
In 2011, there was a bug that caused NVIDIA drivers to hang
Clementine on shutdown. In 2012, only some drivers had the
fix for this issue. Now, in 2019, we do not need to work
around this bug. By reverting commit
c2723008a2
we work around known bad drivers, but do not penalize all
NVIDIA users for this ancient bug.
Async song loading can fail without user feedback. This change adds return codes
to these async load functions. It will now produce an error dialog in simple
scenarios (test case is user selecting a file that is not readable). Other cases,
such as directories and playlists, aren't yet covered.
QTimeLine duration must be greater than 0. If set to 0, a default of 1000ms will
be used. To avoid this, enforce a minimum of 1ms for pause and cross fade values
if those fades are enabled.
There are a number of cases where gst_pipeline_get_bus,
gst_element_get_static_pad, and g_object_get are called without releasing
references. In addition to memory usage, some of these elements hold file
descriptors. In normal operation, two file descriptors are leaked for each
played track. The default fd ulimit for many linux distros is 1024. This
is likely the cause of the crash reported in issue 6309.
This change fixes the obvious and consistent leaks, but it's probably not a
complete solution. There are many error and corner conditions that need to be
examined.
In the case that an error occurs in ReplaceDecodeBin before the bin is added to
the pipeline, unreference the object to allow cleanup. This change also separates
CreateDecodeBinFromUrl from ReplaceDecodeBin, following the pattern of
CreateDecodeBinFromString.
If ReplaceDecodeBin fails from TransitionToNext, uridecodebin_ will not be
replaced with a new element. Since TransitionToNext does not check the return
value, it unknowingly deletes uridecodebin_.
As reported in issue 6302, playing a stream that causes gstreamer to error at
start can cause a crash. The problem occurs when the MoodbarPipeline receives a
pad-added signal after it has handled an error callback. In the error callback,
the builder_ is freed. In the pad-added handler (NewPadCallback), this object
is accessed.
This change adds a running_ flag that is set when the pipeline is started and
cleared on an error, end of stream, or object destruction. We check this flag at
the beginning of NewPadCallback. For sanity sake, we also check the builder_
pointer before dereferencing. Note that checking the state of the pipeline
wasn't an option since the pipeline is in the process of changing states during
the pad-added callback and gst_element_get_state wants to block during a state
change.
This solution is not complete as there are still some syncronization issues.
With this specific situation, the error and new pad callbacks appear to always
occur on the same thread, but that's probably not true for all error conditions.
The object is also destroyed by a different thread, so it may be possible that a
callback can occur at the wrong time during or after the deletion of the object.
See https://github.com/clementine-player/Clementine/issues/6302
There is a small chance that a device lister is able to discover and add a
previously known device before it is added by the database loader thread.
In this case, copy the data that is user-settable to the existing DeviceInfo
object and destroy the object created from the database query.
This adds and utilizes a new FindEquivalentDevice method that compares the
device unique IDs. This could probably be made more robust as the unique
IDs for some listers may change. However, this is a problem with the database
storage implementation in general.
When DeviceManager initializes, it creates a thread to load device information
from the database. Part of this process includes use of QPixMap for icons which
produced a warning message:
22:32:53.763 WARN unknown QPixmap: It is not safe to use pixmaps outside the GUI thread
In addition, the device is added to the view using beginInsertRows and
endInsertRows. This could contend with a device added by a lister signaling
PhysicalDeviceAdded.
To solve these problems, this change moves the icon loading and insertion to the
main thread. LoadAllDevices reads the data from the database and creates the
DeviceInfo object, then sends a signal to the main thread. In the signal
handler, the icon is loaded and the device is added to the master list and view.
When unmounting a device, the ConnectedDevice object is destroyed. The
FileSystemDevice destructor waits on its worker thread. If a scan is in
progress, this will block until completion.
There is an existing Stop method in the LibraryWatcher class that is intended to
stop long running operations. To fix, or at least significantly shorten this
hang, we'll call this before waiting for the thread to exit. Also add a
stop_requested check in the cover art scan.
In addition, add a call to Stop in the Library destructor, which has a similar
usage.
Jim Broadus
Jonas Kvinge
Clementine Buildbot
John Maguire
Jim Broadus
Pavel Liavonau
Thor Merlin Lervik
Cesar Enrique Garcia Dabo
Luis Caceres
Mattias Andersson
Dmitry Valter
Antonio Russo
luz.paz
Andrew Reading
lordlethis
Paul Bonnen
James D. Smith
Filip Gawin
smithjd15