command: make playlist writable #14990
Conversation
|
Can av_dict be used instead of adding khash? |
|
av_dict is O(n) and uses string comparison. |
| #endif | ||
| #ifndef kfree | ||
| #define kfree(P) free(P) | ||
| #endif |
There was a problem hiding this comment.
Bundling khash seems hefty. Either way this should be defined to use talloc_ as everything else in mpv.
There was a problem hiding this comment.
Also not sure if we should include whole khash implementation just for a single property use.
There was a problem hiding this comment.
The obvious advantage of playlist_entry_id over index is that it's unique so if parallel modifications happen it will not mess up the playlist. Users could be thankful for the extra safety and khash basically requires no maintenance on the developer's end.
However (AFAIK) no other mpv API supports this kind of ID addressing so even though it sounds great, maybe it's useless to be too smart here.
The only thing I'm not sure about whether a simple set_property("playlist", get_property("playlist")) should work. If ID lookup gets removed users have to manually hack an index for each entry (that may or may not be an issue since they already transform the playlist by some code).
An attempt to improve #14939. It enhances the playlist property by making it writable: it allows reordering, removing and loading new playlist entries in one go. Thanks na-na-hi for the idea.
Compared to my previous approach it adds much more code but I hope the added functionality justifies it (though most of it is just copy-pasted hash map implementation).
I couldn't find too much prior work on tests so I came up with a custom script. Not sure whether this was the correct way to do it.