The database queries, initiated by list models, must be executed
asynchronously (on a thread different from the UI thread).
When a list model is destroyed (from the UI thread), some database
queries might still be executing. It would be incorrect for the list
model to wait for them (it would block the UI thread), so they must be
executed on an external thread pool.
Since the queries themselves require the medialib to be executed, they
must not outlive it (or the asynchronous code might crash). Therefore,
the right scope for executing asynchronous queries is the MediaLib
instance.
Signed-off-by: Pierre Lamot <pierre@videolabs.io>
When the database queries are executed asynchronously, we must still be
able to delete a list model from the main thread without blocking. As a
consequence, the data loader must be able to outlive the list model
instance; this implies that loading data must not involve calling
methods on the list model itself (fetch() and countTotalElements()).
Nevertheless, the actual data to load depends on the list model (the
content is not the same between a list of videos and a list of albums).
To be able to detach the lifetime of the data loader from the one of the
list model, make the list models provide loader instances on demand,
instead of implementing the loading methods directly.
Signed-off-by: Pierre Lamot <pierre@videolabs.io>
Currently, the loader is owned by MLModelCache, but in the future it
might outlive it: database queries will be executed from a separate
thread, and MLModelCache could be deleted while an asynchronous request
is running.
To prepare for this change, do not require to receive the loader via a
std::unique_ptr (let the cache wrap it).
Signed-off-by: Pierre Lamot <pierre@videolabs.io>
MLSlidingWindowModel declares two pure virtual methods to be implemented
by subclasses:
- countTotalElements()
- fetch()
The query parameters to use were implicitly defined by the class fields.
Instead, pass them explicitly. This paves the way to implement a
separate cache and execute queries from a separate thread.
Signed-off-by: Pierre Lamot <pierre@videolabs.io>
The fetch() methods returns the data it loads, it does not need access
(except possibly to mutable fields for caching).
This makes it consistent with countTotalElements().
Signed-off-by: Pierre Lamot <pierre@videolabs.io>
done using MainInterface.MainGridView and MainInterface.MainTableView as base for these views or explicitely specifying footer as MiniPlayerBottomMargin where former is not applicable
Signed-off-by: Pierre Lamot <pierre@videolabs.io>
The m_cover field is exposed (via getCover()) as a QML property, so it
must always be changed from the UI thread.
Signed-off-by: Alexandre Janniaux <ajanni@videolabs.io>
If cancel() was called before the first lock was acquired in run(), then
cancel() would return immediately and run() would continue as if it was
not canceled (which could cause segfaults).
To avoid the problem, always set the "canceled" flag on cancel, so that
any future execution of run() will return immediately.
Note: we could alternatively track the "finished" state, and wait until
the task is finished on cancel(). However, if run() may not start
immediately (if the thread pool is full), this strategy would cause to
block on cancel() until run() is actually executed, which is
unnecessary.
Signed-off-by: Alexandre Janniaux <ajanni@videolabs.io>
Now that the UI state is always accessed from the UI thread, locks and
atomics are not necessary anymore.
Signed-off-by: Alexandre Janniaux <ajanni@videolabs.io>
The medialib events notify changes that must be reflected in the UI, but
they can be executed from any thread, so it is incorrect to update the
UI from there (for example calling dataChanged() using an index not
necessary valid within the UI thread).
The callbacks provide an event structure pointing to data which are only
valid during the callback, so create a struct to copy the values and
dispatch the events to the UI thread.
Signed-off-by: Alexandre Janniaux <ajanni@videolabs.io>
A call to rowCount() only needs the number of items, there is no need to
fetch the actual data.
Signed-off-by: Alexandre Janniaux <ajanni@videolabs.io>
which was causing a whole bunch of "'< 0' is always false" warnings.
review of the original fix ([1]) indicated that in fact the parameter was
wrong rather than the checks within the function being out of step with
the final design of the function interface, since apparently callers
are in fact passing in a signed index, where a negative represents an
invalid index which the function is expected to handle appropriately.
[1]: https://mailman.videolan.org/pipermail/vlc-devel/2020-September/137711.html
Co-authored-by: Pierre Lamot <pierre@videolabs.io>
Signed-off-by: Pierre Lamot <pierre@videolabs.io>
Romain Vimont
Prince Gupta
Pierre Lamot
Fatih Uzunoglu
Lyndon Brown