[vlc-devel] [Feature request] Automatically persist and restore per-video custom bookmarks without a playlist
Kevin Du
kevin7du at gmail.com
Sun Sep 20 16:28:01 UTC 2026
Hello VLC developers,
Following up on my earlier bookmark feature request, I am submitting
an implementation proposal and regression tests for the Qt interface
in VLC 3.0.23. The patch and developer notes are attached.
The user-facing goal is straightforward: creating a bookmark should
save it automatically in VLC's application storage; opening that same
video directly should restore its bookmarks; editing, deleting, or
clearing bookmarks should persist immediately. Users should not have
to create a bookmark and then separately save a playlist, or remember
to open that playlist later. Those extra steps are redundant and
unintuitive for a bookmark intended for later use.
The patch:
- Saves names and exact timestamps in a separate, atomic per-video
JSON record under VLC's configuration directory.
- Saves directly after bookmark changes in the Qt dialog, rather than
waiting for shutdown.
- Restores bookmarks when the same local video opens, without forcing
a popup or requiring a playlist.
- Persists deletions and clear-all, including an empty record that
prevents stale playlist bookmarks from returning.
- Separates different videos by their canonical full file URI and
keeps explicit bookmarks independent of recent-history eviction.
- Reports save failures and validates stored data before replacing
live bookmarks.
This is independent of the earlier playback-resume patch and applies
to the unmodified official 3.0.23 source. An AI coding assistant
helped prepare it.
Validation completed: source/lifecycle review, syntax parsing of the
newly added C++ code, git diff --check, and applying the patch to a
separate original-source copy followed by exact comparison of all
eight changed files.
Important limitation: the patch has NOT been compiled or tested in a
running VLC instance. The attached patch includes a standalone QtTest
regression suite, but that suite has NOT been executed because a
C++/Qt development toolchain was unavailable. Please treat this as an
RFC implementation requiring human review, build validation, and
integration testing—not as a verified or merge-ready fix.
The developer notes document test cases and scope, including
local-file support, Qt-dialog-only mutation persistence, path changes,
simultaneous instances, and the policy that saved local bookmarks take
precedence over playlist copies. A master-branch implementation would
need a port to the newer architecture.
Could you review this approach or direct me to the appropriate
existing issue/merge-request workflow? I understand VLC accepts code
through VideoLAN GitLab; this email supplies the code and tests for
discussion and is not itself a merge request.
Thank you,
Kevin
On Sun, 20 Sep 2026 11:08:40 -0500, Kevin Du <kevin7du at gmail.com> wrote:
> Hello VLC developers,
>
> I use VLC 3.0.23 Vetinari and would like to request automatic, persistent custom bookmarks associated with each video.
>
> Current experience:
> I create a bookmark while watching a video, close VLC, and later reopen the same video directly. The Custom Bookmarks window is empty. Keeping the bookmark requires an additional Save Playlist to File step, and retrieving it requires opening that saved playlist instead of simply opening the video.
>
> This is redundant and unintuitive. Creating a bookmark already expresses the intention to save a place for later. Requiring two separate saving actions—Create Bookmark, then Save Playlist—makes the first action feel incomplete, without an obvious indication that the bookmark will otherwise be lost. It also makes users manage a separate playlist file for something they expect to belong to the video.
>
> Requested behavior:
> 1. When a user creates a bookmark, automatically save its timestamp and name in VLC-managed application storage, such as a local application data file or database, associated with that video. Do not require modifying the video file.
> 2. Persist bookmark additions, renames, and deletions as they happen, so they survive closing VLC and restarting the laptop.
> 3. When the same video is opened directly again, automatically load its saved bookmarks into the Custom Bookmarks window/menu. The user should not have to find or open a playlist first. The request is to populate the bookmark interface automatically; forcing a popup every time is not necessary.
> 4. Let the user delete an individual bookmark or clear all bookmarks for the current video. Save that deletion automatically so removed bookmarks do not return next time. No playlist export or resave should be required.
> 5. Keep bookmarks associated with the correct video, avoiding collisions between different videos with the same filename.
> 6. Keep playlist export available for users who want it, but make it optional for ordinary bookmark persistence.
>
> Example acceptance scenario:
> - Open a video directly and create bookmarks at 12:30 and 27:10.
> - Close VLC and restart the laptop.
> - Open that same video directly: both bookmarks are available again.
> - Delete the 12:30 bookmark, close VLC, and reopen the video: only 27:10 remains.
> - Clear the remaining bookmark and reopen again: the bookmark list stays empty.
> None of these actions should require saving or opening a playlist file.
>
> These are explicitly created bookmarks and should remain available until the user deletes them or deliberately clears stored bookmark data; they should not disappear merely because a video falls out of the recent-media list.
>
> This is separate from my earlier request about automatically remembering the last playback position. Here the goal is to preserve the multiple places a user deliberately marks within a video.
>
> This is a feature and usability proposal, not a tested code patch. If a newer VLC version already implements this behavior, please point me to it; otherwise, please consider it for a future release or direct me to an existing issue.
>
> Thank you,
> Kevin
-------------- next part --------------
diff --git a/modules/gui/qt/Makefile.am b/modules/gui/qt/Makefile.am
index 2d59ded..c5632aa 100644
--- a/modules/gui/qt/Makefile.am
+++ b/modules/gui/qt/Makefile.am
@@ -10,7 +10,8 @@
# - Add it to DEPS_res
SUFFIXES += .ui .h .hpp .moc.cpp
-EXTRA_DIST += gui/qt/qt6.pro gui/qt/private-gui.pro
+EXTRA_DIST += gui/qt/qt6.pro gui/qt/private-gui.pro \
+ gui/qt/tests/bookmark_store_test.cpp gui/qt/tests/bookmark_store.pro
libqt_plugin_la_CPPFLAGS = $(AM_CPPFLAGS) \
-DQT_USE_QSTRINGBUILDER \
@@ -148,6 +149,7 @@ libqt_plugin_la_SOURCES = \
gui/qt/util/qvlcframe.hpp \
gui/qt/util/qvlcapp.hpp \
gui/qt/util/singleton.hpp \
+ gui/qt/util/bookmark_store.hpp \
gui/qt/util/imagehelper.cpp gui/qt/util/imagehelper.hpp \
gui/qt/styles/seekstyle.cpp gui/qt/styles/seekstyle.hpp
if HAVE_WIN32
diff --git a/modules/gui/qt/dialogs/bookmarks.cpp b/modules/gui/qt/dialogs/bookmarks.cpp
index 0840097..bff6809 100644
--- a/modules/gui/qt/dialogs/bookmarks.cpp
+++ b/modules/gui/qt/dialogs/bookmarks.cpp
@@ -32,6 +32,8 @@
#include <QPushButton>
#include <QDialogButtonBox>
#include <QModelIndexList>
+#include <QMessageBox>
+#include <QSignalBlocker>
BookmarksDialog::BookmarksDialog( intf_thread_t *_p_intf ):QVLCFrame( _p_intf )
{
@@ -122,7 +124,7 @@ void BookmarksDialog::update()
{
if ( b_ignore_updates ) return;
input_thread_t *p_input = THEMIM->getInput();
- if( !p_input ) return;
+ const QSignalBlocker blocker( bookmarksList );
seekpoint_t **pp_bookmarks;
int i_bookmarks = 0;
@@ -132,7 +134,7 @@ void BookmarksDialog::update()
bookmarksList->model()->removeRows( 0, bookmarksList->topLevelItemCount() );
}
- if( input_Control( p_input, INPUT_GET_BOOKMARKS, &pp_bookmarks,
+ if( !p_input || input_Control( p_input, INPUT_GET_BOOKMARKS, &pp_bookmarks,
&i_bookmarks ) != VLC_SUCCESS )
return;
@@ -171,7 +173,8 @@ void BookmarksDialog::add()
QByteArray raw = name.toUtf8();
bookmark.psz_name = raw.data();
- input_Control( p_input, INPUT_ADD_BOOKMARK, &bookmark );
+ if( input_Control( p_input, INPUT_ADD_BOOKMARK, &bookmark ) == VLC_SUCCESS )
+ save( p_input );
}
}
@@ -194,6 +197,7 @@ void BookmarksDialog::del()
}
input_Control( p_input, INPUT_DEL_BOOKMARK, (*it).row() );
b_ignore_updates = false;
+ save( p_input );
update();
}
}
@@ -203,7 +207,8 @@ void BookmarksDialog::clear()
input_thread_t *p_input = THEMIM->getInput();
if( !p_input ) return;
- input_Control( p_input, INPUT_CLEAR_BOOKMARKS );
+ if( input_Control( p_input, INPUT_CLEAR_BOOKMARKS ) == VLC_SUCCESS )
+ save( p_input );
}
void BookmarksDialog::edit( QTreeWidgetItem *item, int column )
@@ -263,7 +268,8 @@ void BookmarksDialog::edit( QTreeWidgetItem *item, int column )
}
// Send the modification
- input_Control( p_input, INPUT_CHANGE_BOOKMARK, p_seekpoint, i_edit );
+ if( input_Control( p_input, INPUT_CHANGE_BOOKMARK, p_seekpoint, i_edit ) == VLC_SUCCESS )
+ save( p_input );
clear:
// Clear the bookmark list
@@ -272,6 +278,16 @@ clear:
free( pp_bookmarks );
}
+void BookmarksDialog::save( input_thread_t *input )
+{
+ QString error;
+ if( !THEMIM->getIM()->SaveBookmarks( input, error ) )
+ QMessageBox::warning( this, qtr( "Bookmarks could not be saved" ),
+ qtr( "The bookmark changes are available in this session, but could "
+ "not be saved for the next time you open this video." )
+ + "\n\n" + error );
+}
+
void BookmarksDialog::extract()
{
// TODO
diff --git a/modules/gui/qt/dialogs/bookmarks.hpp b/modules/gui/qt/dialogs/bookmarks.hpp
index 17c7bde..0a58200 100644
--- a/modules/gui/qt/dialogs/bookmarks.hpp
+++ b/modules/gui/qt/dialogs/bookmarks.hpp
@@ -45,6 +45,7 @@ private:
QPushButton *clearButton;
QPushButton *delButton;
bool b_ignore_updates;
+ void save( input_thread_t * );
private slots:
void update();
diff --git a/modules/gui/qt/input_manager.cpp b/modules/gui/qt/input_manager.cpp
index 4a3f689..7f2e790 100644
--- a/modules/gui/qt/input_manager.cpp
+++ b/modules/gui/qt/input_manager.cpp
@@ -29,6 +29,7 @@
#include "input_manager.hpp"
#include "recents.hpp"
+#include "util/bookmark_store.hpp"
#include <vlc_actions.h> /* ACTION_ID */
#include <vlc_url.h> /* vlc_uri_decode */
@@ -112,6 +113,8 @@ void InputManager::setInput( input_thread_t *_p_input )
UpdateVout();
p_item = input_GetItem( p_input );
+ RestoreBookmarks();
+ emit bookmarksChanged();
emit rateChanged( var_GetFloat( p_input, "rate" ) );
/* Get Saved Time */
@@ -190,6 +193,7 @@ void InputManager::delInput()
vlc_object_release( p_input );
p_input = NULL;
+ emit bookmarksChanged();
emit positionUpdated( -1.0, 0 ,0 );
emit rateChanged( var_InheritFloat( p_intf, "rate" ) );
@@ -435,6 +439,113 @@ void InputManager::UpdatePosition()
emit positionUpdated( f_pos, i_time, i_length / CLOCK_FREQ );
}
+static QString BookmarkDirectory()
+{
+ const QString config = QVLCUserDir( VLC_CONFIG_DIR );
+ return config.isEmpty() ? QString() : QDir( config ).filePath( "bookmarks" );
+}
+
+bool InputManager::SaveBookmarks( input_thread_t *input, QString &error )
+{
+ error.clear();
+ if( !input )
+ {
+ error = qtr( "No video is open." );
+ return false;
+ }
+ char *uri = input_item_GetURI( input_GetItem( input ) );
+ const QString key = uri ? VLCBookmarks::Store::mediaKey( uri ) : QString();
+ free( uri );
+ /* Keep the existing transient behavior for streams and non-file media. */
+ if( key.isEmpty() )
+ return true;
+
+ seekpoint_t **points = NULL;
+ int count = -1;
+ const int result = input_Control( input, INPUT_GET_BOOKMARKS, &points, &count );
+ /* VLC 3.x returns VLC_EGENERIC with count == 0 for an empty list. */
+ if( result != VLC_SUCCESS && count != 0 )
+ {
+ error = qtr( "Could not read the current bookmarks." );
+ return false;
+ }
+ QVector<VLCBookmarks::Bookmark> bookmarks;
+ for( int i = 0; i < count; ++i )
+ {
+ if( points[i] )
+ bookmarks.push_back( VLCBookmarks::Bookmark{
+ qfu( points[i]->psz_name ), points[i]->i_time_offset } );
+ else
+ error = qtr( "Could not copy a bookmark." );
+ vlc_seekpoint_Delete( points[i] );
+ }
+ free( points );
+ if( !error.isEmpty() )
+ return false;
+
+ /* Save an empty list too, so stale playlist bookmarks cannot resurrect
+ * bookmarks explicitly cleared by the user. Never save during teardown:
+ * the input thread clears its bookmarks as part of normal shutdown. */
+ return VLCBookmarks::Store( BookmarkDirectory() ).save( key, bookmarks, error );
+}
+
+void InputManager::RestoreBookmarks()
+{
+ char *uri = input_item_GetURI( p_item );
+ const QString key = uri ? VLCBookmarks::Store::mediaKey( uri ) : QString();
+ free( uri );
+ if( key.isEmpty() )
+ return;
+
+ QVector<VLCBookmarks::Bookmark> bookmarks;
+ QString error;
+ const VLCBookmarks::Store::Result result =
+ VLCBookmarks::Store( BookmarkDirectory() ).load( key, bookmarks, error );
+ if( result == VLCBookmarks::Store::Missing )
+ return; /* Preserve explicitly supplied playlist bookmarks. */
+ if( result == VLCBookmarks::Store::Error )
+ {
+ msg_Warn( p_intf, "Could not restore bookmarks: %s", qtu( error ) );
+ return;
+ }
+
+ /* Validate/allocate the entire replacement before touching live bookmarks. */
+ QVector<seekpoint_t *> points;
+ bool allocated = true;
+ for( const VLCBookmarks::Bookmark &bookmark : bookmarks )
+ {
+ seekpoint_t *point = vlc_seekpoint_New();
+ if( point )
+ {
+ point->psz_name = strdup( qtu( bookmark.name ) );
+ point->i_time_offset = bookmark.time;
+ }
+ if( !point || !point->psz_name )
+ {
+ vlc_seekpoint_Delete( point );
+ allocated = false;
+ break;
+ }
+ points.push_back( point );
+ }
+ if( allocated )
+ {
+ input_Control( p_input, INPUT_CLEAR_BOOKMARKS );
+ for( seekpoint_t *point : points )
+ {
+ if( input_Control( p_input, INPUT_ADD_BOOKMARK, point ) != VLC_SUCCESS )
+ {
+ msg_Warn( p_intf, "Could not restore all bookmarks" );
+ break;
+ }
+ }
+ }
+ else
+ msg_Warn( p_intf, "Could not allocate restored bookmarks" );
+ for( seekpoint_t *point : points )
+ vlc_seekpoint_Delete( point );
+}
+
void InputManager::UpdateNavigation()
{
/* Update navigation status */
diff --git a/modules/gui/qt/input_manager.hpp b/modules/gui/qt/input_manager.hpp
index 6170e71..ce3d929 100644
--- a/modules/gui/qt/input_manager.hpp
+++ b/modules/gui/qt/input_manager.hpp
@@ -131,6 +131,7 @@ public:
virtual ~InputManager();
void delInput();
+ bool SaveBookmarks( input_thread_t *, QString &error );
bool hasInput() const { return p_input != NULL; }
int playingStatus() const;
@@ -168,6 +169,7 @@ private:
void UpdateStatus();
void UpdateNavigation();
void UpdatePosition();
+ void RestoreBookmarks();
void UpdateTeletext();
void UpdateArt();
void UpdateInfo();
diff --git a/modules/gui/qt/tests/bookmark_store.pro b/modules/gui/qt/tests/bookmark_store.pro
new file mode 100644
index 0000000..b8d3881
--- /dev/null
+++ b/modules/gui/qt/tests/bookmark_store.pro
@@ -0,0 +1,7 @@
+QT = core testlib
+CONFIG += console testcase c++11
+CONFIG -= app_bundle
+TEMPLATE = app
+TARGET = bookmark_store_test
+SOURCES += bookmark_store_test.cpp
+HEADERS += ../util/bookmark_store.hpp
diff --git a/modules/gui/qt/tests/bookmark_store_test.cpp b/modules/gui/qt/tests/bookmark_store_test.cpp
new file mode 100644
index 0000000..6627f28
--- /dev/null
+++ b/modules/gui/qt/tests/bookmark_store_test.cpp
@@ -0,0 +1,177 @@
+/* Persistent bookmark storage regression tests. GPL-2.0-or-later. */
+#include <QtTest/QtTest>
+#include <QTemporaryDir>
+#include <limits>
+#include "../util/bookmark_store.hpp"
+
+using namespace VLCBookmarks;
+
+class BookmarkStoreTest : public QObject
+{
+ Q_OBJECT
+
+ static QString key( const QString &path )
+ {
+ return Store::mediaKey( QUrl::fromLocalFile( path ).toEncoded() );
+ }
+
+private slots:
+ void createReopenEditDeleteClear()
+ {
+ QTemporaryDir dir;
+ QVERIFY( dir.isValid() );
+ const QString media = key( dir.filePath( "movie.mp4" ) );
+ const QString storage = dir.filePath( "bookmarks" );
+ QString error;
+ QVector<Bookmark> saved{ { "First", 750000000 }, { "Second", 1630000000 } };
+ QVERIFY2( Store( storage ).save( media, saved, error ), qPrintable( error ) );
+
+ QVector<Bookmark> loaded;
+ QCOMPARE( Store( storage ).load( media, loaded, error ), Store::Loaded );
+ QCOMPARE( loaded.size(), 2 );
+ QCOMPARE( loaded[0].time, qint64(750000000) );
+ QCOMPARE( loaded[1].name, QString("Second") );
+
+ loaded[0].name = "Renamed";
+ loaded[0].time = 760123456;
+ QVERIFY( Store( storage ).save( media, loaded, error ) );
+ loaded.clear();
+ QCOMPARE( Store( storage ).load( media, loaded, error ), Store::Loaded );
+ QCOMPARE( loaded[0].name, QString("Renamed") );
+ QCOMPARE( loaded[0].time, qint64(760123456) );
+ loaded.remove( 0 );
+ QVERIFY( Store( storage ).save( media, loaded, error ) );
+ QCOMPARE( Store( storage ).load( media, loaded, error ), Store::Loaded );
+ QCOMPARE( loaded.size(), 1 );
+ QCOMPARE( loaded[0].name, QString("Second") );
+
+ loaded.clear();
+ QVERIFY( Store( storage ).save( media, loaded, error ) );
+ loaded.push_back( Bookmark{ "stale playlist bookmark", 123 } );
+ QCOMPARE( Store( storage ).load( media, loaded, error ), Store::Loaded );
+ QVERIFY( loaded.isEmpty() );
+ }
+
+ void sameFilenameDifferentDirectories()
+ {
+ QTemporaryDir dir;
+ const QString a = key( dir.filePath( "a/movie.mp4" ) );
+ const QString b = key( dir.filePath( "b/movie.mp4" ) );
+ QVERIFY( a != b );
+ Store store( dir.filePath( "bookmarks" ) );
+ QString error;
+ QVERIFY( store.save( a, QVector<Bookmark>{ { "A", 11 } }, error ) );
+ QVERIFY( store.save( b, QVector<Bookmark>{ { "B", 22 } }, error ) );
+ QVector<Bookmark> loaded;
+ QCOMPARE( store.load( a, loaded, error ), Store::Loaded );
+ QCOMPARE( loaded[0].name, QString("A") );
+ QCOMPARE( store.load( b, loaded, error ), Store::Loaded );
+ QCOMPARE( loaded[0].name, QString("B") );
+ }
+
+ void unicodeAndExactTicks()
+ {
+ QTemporaryDir dir;
+ Store store( dir.path() );
+ QString error;
+ const QString media = key( dir.filePath( QString::fromUtf8("film % # \xe7\x8c\xab.mp4") ) );
+ const QString name = QString::fromUtf8("scene {one}, = \"two\"\n\xe7\x8c\xab");
+ const qint64 ticks = std::numeric_limits<qint64>::max();
+ QVERIFY( store.save( media, QVector<Bookmark>{ { name, ticks }, { "", 0 } }, error ) );
+ QVector<Bookmark> loaded;
+ QCOMPARE( store.load( media, loaded, error ), Store::Loaded );
+ QCOMPARE( loaded[0].time, ticks );
+ QCOMPARE( loaded[0].name, name );
+ QCOMPARE( loaded[1].time, qint64(0) );
+ QVERIFY( loaded[1].name.isEmpty() );
+ }
+
+ void missingIsDifferentFromCleared()
+ {
+ QTemporaryDir dir;
+ const QString storage = dir.filePath( "not-created" );
+ const QString media = key( dir.filePath( "movie.mp4" ) );
+ QVector<Bookmark> loaded{ { "playlist", 1 } };
+ QString error;
+ QCOMPARE( Store( storage ).load( media, loaded, error ), Store::Missing );
+ QCOMPARE( loaded.size(), 1 );
+ QVERIFY( !QFileInfo::exists( storage ) );
+ QVERIFY( Store( storage ).save( media, QVector<Bookmark>(), error ) );
+ QCOMPARE( Store( storage ).load( media, loaded, error ), Store::Loaded );
+ QVERIFY( loaded.isEmpty() );
+ }
+
+ void invalidFilesDoNotReplaceLiveBookmarks_data()
+ {
+ QTest::addColumn<QByteArray>( "data" );
+ QTest::newRow("truncated") << QByteArray("{\"version\":1,");
+ QTest::newRow("future-version") << QByteArray("{\"version\":2,\"uri\":\"MEDIA\",\"bookmarks\":[]}");
+ QTest::newRow("wrong-media") << QByteArray("{\"version\":1,\"uri\":\"other\",\"bookmarks\":[]}");
+ QTest::newRow("negative-time") << QByteArray("{\"version\":1,\"uri\":\"MEDIA\",\"bookmarks\":[{\"name\":\"bad\",\"time\":\"-1\"}]}");
+ QTest::newRow("overflow-time") << QByteArray("{\"version\":1,\"uri\":\"MEDIA\",\"bookmarks\":[{\"name\":\"bad\",\"time\":\"9223372036854775808\"}]}");
+ QTest::newRow("numeric-time") << QByteArray("{\"version\":1,\"uri\":\"MEDIA\",\"bookmarks\":[{\"name\":\"bad\",\"time\":3}]}");
+ QTest::newRow("oversized") << QByteArray( 4 * 1024 * 1024 + 1, 'x' );
+ }
+
+ void invalidFilesDoNotReplaceLiveBookmarks()
+ {
+ QFETCH( QByteArray, data );
+ QTemporaryDir dir;
+ Store store( dir.path() );
+ const QString media = key( dir.filePath( "movie.mp4" ) );
+ QString error;
+ QVERIFY( store.save( media, QVector<Bookmark>{ { "old", 1 } }, error ) );
+ const QStringList files = QDir( dir.path() ).entryList( QStringList("*.json"), QDir::Files );
+ QCOMPARE( files.size(), 1 );
+ QJsonArray encoded;
+ encoded.append( media );
+ QByteArray escaped = QJsonDocument( encoded ).toJson( QJsonDocument::Compact );
+ escaped = escaped.mid( 1, escaped.size() - 2 );
+ data.replace( "\"MEDIA\"", escaped );
+ QFile file( dir.filePath( files[0] ) );
+ QVERIFY( file.open( QIODevice::WriteOnly | QIODevice::Truncate ) );
+ QCOMPARE( file.write( data ), qint64(data.size()) );
+ file.close();
+ QVector<Bookmark> loaded{ { "live", 2 } };
+ QCOMPARE( store.load( media, loaded, error ), Store::Error );
+ QVERIFY( !error.isEmpty() );
+ QCOMPARE( loaded.size(), 1 );
+ QCOMPARE( loaded[0].name, QString("live") );
+ }
+
+ void failedSaveKeepsPreviousData()
+ {
+ QTemporaryDir dir;
+ Store store( dir.path() );
+ const QString media = key( dir.filePath( "movie.mp4" ) );
+ QString error;
+ QVERIFY( store.save( media, QVector<Bookmark>{ { "old", 1 } }, error ) );
+ QVERIFY( !store.save( media, QVector<Bookmark>{ { "invalid", -1 } }, error ) );
+ QVERIFY( !store.save( media, QVector<Bookmark>{ { QString( 4*1024*1024, 'x' ), 1 } }, error ) );
+ QVector<Bookmark> loaded;
+ QCOMPARE( store.load( media, loaded, error ), Store::Loaded );
+ QCOMPARE( loaded[0].name, QString("old") );
+ }
+
+ void storageFailureIsReported()
+ {
+ QTemporaryDir dir;
+ const QString path = dir.filePath( "not-a-directory" );
+ QFile blocker( path );
+ QVERIFY( blocker.open( QIODevice::WriteOnly ) );
+ blocker.close();
+ QString error;
+ QVERIFY( !Store( path ).save( key( dir.filePath("movie.mp4") ), QVector<Bookmark>(), error ) );
+ QVERIFY( !error.isEmpty() );
+ }
+
+ void nonFileMediaIsExcluded()
+ {
+ QVERIFY( Store::mediaKey( "https://example.com/movie.mp4" ).isEmpty() );
+ QVERIFY( Store::mediaKey( "dvd:///D:/" ).isEmpty() );
+ QVERIFY( Store::mediaKey( QByteArray() ).isEmpty() );
+ }
+};
+
+QTEST_GUILESS_MAIN( BookmarkStoreTest )
+#include "bookmark_store_test.moc"
diff --git a/modules/gui/qt/util/bookmark_store.hpp b/modules/gui/qt/util/bookmark_store.hpp
new file mode 100644
index 0000000..e4df3fd
--- /dev/null
+++ b/modules/gui/qt/util/bookmark_store.hpp
@@ -0,0 +1,191 @@
+/*****************************************************************************
+ * bookmark_store.hpp : Persistent, per-file custom bookmarks
+ *****************************************************************************
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ * This program is distributed WITHOUT ANY WARRANTY; without even the implied
+ * warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ *****************************************************************************/
+
+#ifndef VLC_QT_BOOKMARK_STORE_HPP
+#define VLC_QT_BOOKMARK_STORE_HPP
+
+#include <QCryptographicHash>
+#include <QDir>
+#include <QFile>
+#include <QFileInfo>
+#include <QJsonArray>
+#include <QJsonDocument>
+#include <QJsonObject>
+#include <QSaveFile>
+#include <QString>
+#include <QUrl>
+#include <QVector>
+
+namespace VLCBookmarks
+{
+struct Bookmark
+{
+ QString name;
+ qint64 time;
+};
+
+/* Independent of recent-media history. No write takes place on load or exit.
+ * One atomic file per media avoids losing other videos' edits across instances.
+ * Concurrent edits to the same media have last-successful-write semantics. */
+class Store
+{
+public:
+ enum Result { Missing, Loaded, Error };
+
+ explicit Store( const QString &directory ) : directory( directory ) {}
+
+ static QString mediaKey( const QByteArray &uri )
+ {
+ const QUrl url = QUrl::fromEncoded( uri, QUrl::StrictMode );
+ if( !url.isValid() || !url.isLocalFile() )
+ return QString();
+
+ const QFileInfo info( url.toLocalFile() );
+ QString path = info.canonicalFilePath();
+ if( path.isEmpty() )
+ path = info.absoluteFilePath();
+ return QString::fromUtf8( QUrl::fromLocalFile(
+ QDir::cleanPath( path ) ).toEncoded( QUrl::FullyEncoded ) );
+ }
+
+ Result load( const QString &key, QVector<Bookmark> &bookmarks,
+ QString &error ) const
+ {
+ error.clear();
+ if( key.isEmpty() || directory.isEmpty() )
+ {
+ error = QStringLiteral("No local media key or bookmark directory");
+ return Error;
+ }
+ QFile file( fileName( key ) );
+ if( !file.exists() )
+ return Missing;
+ if( !file.open( QIODevice::ReadOnly ) )
+ {
+ error = file.errorString();
+ return Error;
+ }
+ if( file.size() > maxBytes() )
+ {
+ error = QStringLiteral("Bookmark file is too large");
+ return Error;
+ }
+ QJsonParseError parseError;
+ const QJsonDocument doc = QJsonDocument::fromJson( file.readAll(), &parseError );
+ if( file.error() != QFileDevice::NoError )
+ {
+ error = file.errorString();
+ return Error;
+ }
+ const QJsonObject root = doc.object();
+ if( parseError.error != QJsonParseError::NoError || !doc.isObject()
+ || root.value( "version" ).toDouble( -1 ) != 1
+ || !root.value( "uri" ).isString()
+ || root.value( "uri" ).toString() != key
+ || !root.value( "bookmarks" ).isArray() )
+ {
+ error = QStringLiteral("Invalid or unsupported bookmark file");
+ return Error;
+ }
+
+ QVector<Bookmark> loaded;
+ const QJsonArray array = root.value( "bookmarks" ).toArray();
+ for( const QJsonValue &value : array )
+ {
+ const QJsonObject item = value.toObject();
+ bool ok = false;
+ const qint64 time = item.value( "time" ).toString().toLongLong( &ok );
+ if( !value.isObject() || !item.value( "name" ).isString()
+ || !item.value( "time" ).isString() || !ok || time < 0 )
+ {
+ error = QStringLiteral("Invalid bookmark entry");
+ return Error;
+ }
+ loaded.push_back( Bookmark{ item.value( "name" ).toString(), time } );
+ }
+ bookmarks = loaded;
+ return Loaded;
+ }
+
+ bool save( const QString &key, const QVector<Bookmark> &bookmarks,
+ QString &error ) const
+ {
+ error.clear();
+ if( key.isEmpty() || directory.isEmpty() )
+ {
+ error = QStringLiteral("No local media key or bookmark directory");
+ return false;
+ }
+ QJsonArray array;
+ for( const Bookmark &bookmark : bookmarks )
+ {
+ if( bookmark.time < 0 )
+ {
+ error = QStringLiteral("Negative bookmark time");
+ return false;
+ }
+ QJsonObject item;
+ item.insert( "name", bookmark.name );
+ /* Decimal strings preserve the complete 64-bit tick value. */
+ item.insert( "time", QString::number( bookmark.time ) );
+ array.append( item );
+ }
+ QJsonObject root;
+ root.insert( "version", 1 );
+ root.insert( "uri", key );
+ root.insert( "bookmarks", array );
+ const QByteArray data = QJsonDocument( root ).toJson( QJsonDocument::Compact );
+ if( data.size() > maxBytes() )
+ {
+ error = QStringLiteral("Bookmark file is too large");
+ return false;
+ }
+ if( !QDir().mkpath( directory ) )
+ {
+ error = QStringLiteral("Cannot create bookmark directory");
+ return false;
+ }
+ QSaveFile file( fileName( key ) );
+ /* Never fall back to truncating the previous good copy. */
+ file.setDirectWriteFallback( false );
+ if( !file.open( QIODevice::WriteOnly ) )
+ {
+ error = file.errorString();
+ return false;
+ }
+ if( file.write( data ) != data.size() )
+ {
+ error = file.errorString();
+ file.cancelWriting();
+ return false;
+ }
+ if( !file.commit() )
+ {
+ error = file.errorString();
+ return false;
+ }
+ return true;
+ }
+
+private:
+ static qint64 maxBytes() { return 4 * 1024 * 1024; }
+
+ QString fileName( const QString &key ) const
+ {
+ const QByteArray hash = QCryptographicHash::hash(
+ key.toUtf8(), QCryptographicHash::Sha256 ).toHex();
+ return QDir( directory ).filePath( QString::fromLatin1( hash ) + ".json" );
+ }
+
+ QString directory;
+};
+}
+#endif
-------------- next part --------------
# Persistent custom bookmarks: VLC 3.0.23 Qt patch
Status: proposed implementation for review, not a compiled or verified VLC release. No changes were made to the user's installed VLC.
## Problem and resulting behavior
Creating a custom bookmark expresses the user's intent to preserve a place in a video. Requiring a separate Save Playlist action, and requiring that playlist to be opened later, makes creating the bookmark feel incomplete and unintuitive. Deleting a bookmark should also persist without exporting a playlist.
The patch saves custom bookmarks immediately after successful Create, Edit, and Clear actions and after a batch Delete action in the desktop Qt bookmark dialog. Opening the same local video restores the saved list automatically, whether or not the bookmark window is open. It populates the existing window/menu rather than forcing a popup. Playlist export remains available.
This patch is independent of the earlier playback-resume proposal. Apply it to the official VLC 3.0.23 tag; it does not require that earlier patch.
## Implementation
- Application storage: a `bookmarks` directory underneath VLC's configuration directory, with one JSON file per canonical local media URI, named using SHA-256 of that URI. This includes different directories in the identity, preventing videos with the same basename from sharing bookmarks.
- Data: schema version, media URI, ordered bookmark names, and exact 64-bit time offsets stored as decimal strings. Names containing Unicode and punctuation do not depend on VLC's comma-separated playlist-option parser.
- Writes: `QSaveFile` replaces the previous file atomically; direct-write fallback is disabled. A save failure is shown in the bookmark dialog rather than silently claiming the change is durable.
- Reads: malformed, unsupported, mismatched, or oversized data is rejected before replacing live bookmarks. Failed loads leave the current input's bookmarks intact and log a warning.
- Deletions: an empty saved array records that all bookmarks were cleared. It takes precedence over stale playlist-supplied bookmarks, so reopening an old playlist cannot resurrect explicitly deleted bookmarks. If no saved record exists, supplied playlist bookmarks are left alone.
- Lifecycle: restoring happens in the input manager; saving happens on explicit Qt UI mutations, not on shutdown. The original input thread destroys its bookmark list during teardown, so a shutdown snapshot could accidentally overwrite the stored list with an empty one.
- History: explicitly saved bookmarks are independent of the recent-media list and its size/privacy setting.
- UI refresh: refresh signals are emitted on input changes, and tree-item signals are blocked during population to prevent restoration/refresh from being mistaken for an edit.
- Distribution: the storage header is added to the plugin source list; the standalone QtTest suite and its qmake project are included in `EXTRA_DIST`.
## Checks actually completed
1. Retrieved the relevant original source files from the official VideoLAN mirror at tag `3.0.23`.
2. Reviewed the input bookmark control operations, initialization and teardown, Qt dialog mutation paths, and data ownership.
3. Ran C++ syntax parsing on the new storage header, new test source, and newly introduced integration functions. Qt declaration macros were normalized for syntax parsing. This does not perform type checking, linking, or runtime execution.
4. Passed `git diff --check`.
5. Applied the complete patch to a separate copy of the original files and compared every reconstructed file with the implementation: all eight matched exactly.
## Tests supplied, not executed
The patch includes `modules/gui/qt/tests/bookmark_store_test.cpp` and `bookmark_store.pro`. The tests exercise:
- Create, reopen, rename/time edit, delete one, and clear all.
- Independent records for identical filenames in different directories.
- Unicode, punctuation, empty names, zero timestamps, and exact 64-bit timestamps.
- The distinction between missing records and explicitly cleared records.
- Truncated JSON, unknown versions, URI mismatches, invalid/overflowing/numeric timestamps, and oversized files.
- Preservation of the previous valid data when invalid or oversized changes are rejected.
- Reporting a storage path failure.
- Exclusion of non-file media.
With a Qt development environment, build the standalone tests in a separate directory using qmake with `modules/gui/qt/tests/bookmark_store.pro`, then the platform's make tool, and run `bookmark_store_test`. Qt 5.15 is a suitable test target. The standalone suite does not require the full VLC build.
No working C++/Qt build toolchain was available in this task. The supplied QtTest suite has NOT been compiled or run. The full plugin and the integrated user workflow have NOT been tested in a running VLC instance. Maintainers must perform those checks before considering a merge or release.
## Required integration validation and policy review
- Create bookmarks at 12:30 and 27:10, exit/restart VLC or the laptop, open the video directly, and verify both are restored and seek correctly.
- Rename/edit a timestamp; delete one; clear all; repeat reopening after each action without saving a playlist.
- Keep the bookmark dialog open while switching videos and stopping playback; verify the displayed list follows the current video.
- Verify existing playlist bookmarks are retained when no local record exists, and saved local records—including empty records—take precedence when present. Maintainers should explicitly approve this precedence policy.
- Test read-only/unavailable storage, interrupted writes, Unicode paths, removable storage, UNC paths, and permissions on supported platforms.
- Test rapid Create/Delete followed by immediate input switching or closing, and input teardown races. Persistence is synchronous at the Qt mutation sites, but integrated scheduling still requires testing.
- Confirm the privacy/UI policy for long-lived explicit bookmarks independent of recent-history clearing. Clearing bookmarks removes their entries but retains an empty record containing the media URI to suppress stale playlist copies.
## Scope and remaining limitations
- Targets the Qt interface in VLC 3.0.23. VLC master has different architecture and needs a port; this is not a master-branch merge request.
- Persists local `file:` media, including filesystem-mounted media. Network-stream/disc URIs retain their existing transient behavior.
- Covers changes made in the Qt Custom Bookmarks dialog. Edits made by Lua, remote-control interfaces, or other front ends are not automatically persisted by this patch.
- Media identity is path-based. Renaming/moving a video does not transfer its record, and replacing the contents at an existing path reuses that path's bookmarks.
- Separate videos have separate storage files. Simultaneous edits to the same video from multiple VLC instances use last-successful-write semantics; live cross-instance merging is not implemented.
- Storage is bounded to 4 MiB per video. Corrupt records are not automatically overwritten on load.
- No claim is made of guaranteed recovery from hardware failure, unavailable disks, or every abrupt process/power interruption.
## Source references
- https://github.com/videolan/vlc/blob/3.0.23/modules/gui/qt/dialogs/bookmarks.cpp
- https://github.com/videolan/vlc/blob/3.0.23/modules/gui/qt/input_manager.cpp
- https://github.com/videolan/vlc/blob/3.0.23/src/input/control.c
- https://github.com/videolan/vlc/blob/3.0.23/src/input/input.c
The patch and tests were prepared with an AI coding assistant. This submission requests human maintainer review and testing; it is not represented as a verified fix.
More information about the vlc-devel
mailing list