XML config files for Live Broadcasting preferences #1283

Palakis · 2017-06-15T14:25:53Z

These contributions add an XML settings system for Live Broadcasting settings which allows management of several named sets of settings (a.k.a "profiles").

This is phase 1 out of 2 of my GSoC project for Mixxx to add multi-broadcasting to Mixxx (being able to stream to several servers simultaneously). Each broadcasting output will have its own profile

This PR aims to implement this subsystem into Mixxx while keep the current Live Broadcasting preferences UI, which will be refactored in phase 2. Settings are saved in an XML document named Default Profile.bcp.xml located in the broadcast_profiles subfolder of Mixxx's settings folder.

Tasks:

Implement the BroadcastProfile class:
- Basic structure
- XML read/write code
- Validate proper functioning
- Write unit tests
Refactor the BroadcastSettings class
- Make sure only one instance of BroadcastSettings exists
- QList of BroadcastProfile instances
- List profiles from filesystem
Automatic upgrade from legacy Live Broadcasting preferences format (in mixxx.cfg) to new XML engine

I initially forgot to add broadcastprofile.cpp into the list of source files in build/depends.py, so I didn't noticed all the errors and mistakes I've made while writing this earlier. This commit aims to fix these errors and have the BroadcastProfile class to build.

No idea if this works. I just want to make sure it is saved somewhere out of my computer. I may squash this with other commits later.

Both ways didn't handled the document's root tag. Now, profile values are saved properly.

…ast-profiles

daschuer · 2017-06-15T18:33:14Z

src/broadcast/broadcastmanager.cpp

@@ -6,14 +6,15 @@
 #include "engine/sidechain/enginesidechain.h"
 #include "soundio/soundmanager.h"

-BroadcastManager::BroadcastManager(UserSettingsPointer pConfig,
+BroadcastManager::BroadcastManager(SettingsManagerPointer pSettingsManager,


since you do not store the pointer, you should pass it as plain pointer.

@daschuer Same when passing it to DlgPreferences?

Yes. Shard pointer for shared ownership. With a plain pointer parameter, the function promises not to store the pointer.

@daschuer Okay. If thinking this way, I think I should use shared pointers for BroadcastProfile instances.

daschuer · 2017-06-15T18:40:30Z

src/encoder/encoderbroadcastsettings.cpp

-{
+EncoderBroadcastSettings::EncoderBroadcastSettings(
+        BroadcastSettingsPointer settings) :
+m_settings(settings) {


Style nit:

EncoderBroadcastSettings::EncoderBroadcastSettings( BroadcastSettingsPointer settings) : m_settings(settings) {

This isn't needed in the current state of the code.

These additions automatically fetches existing Live Broadcasting preferences from mixxx.cfg, puts them in the Default Profile and removes the now useless keys from mixxx.cfg

daschuer · 2017-06-16T12:30:49Z

Nice, please give a hint when this is worth to test manually.

Add more tests in the process, because the regex changed.

sblaisot · 2017-06-16T18:43:37Z

probably you need to take a look at upgrade.cpp/.h.
This is where migrating from old preference style to new ones should reside.

daschuer · 2017-06-20T23:20:16Z

src/preferences/broadcastprofile.cpp

+        return nullptr;
+
+    QString profileName = xmlFile.baseName();
+    BroadcastProfile* profile = new BroadcastProfile(profileName);


here you should use std::make_unique<> and return the BroadcastProfilePtr

daschuer · 2017-06-21T06:06:48Z

I refactored BroadcastSettings::m_profiles as a std::map<QString,

BroadcastProfilePtr>. The map key and the profile name are synchronized with a signal/slot pair triggered when a call to BroadcastProfile::setProfileName is made. What do you think of this solution? Perfect!

Palakis · 2017-06-21T09:48:20Z

I forgot to connect BroadcastProfile::profileNameChanged to BroadcastSettings::onProfileNameChanged. Since std::unique_ptr is not compatible with Qt's object system, I replaced it with QSharedPointer, as it is easy to pass around.

…t call BroadcastProfile notifies name changes to BroadcastSettings with Qt's signals & slots system. However Qt is not compatible with the std::unique_ptr smart pointer. A refactor of BroadcastProfilePtr is required to make it compatible. std::unique_ptr has been replaced with QSharedPointer, as this type is easy to pass around in the code while keeping a reference to the original pointer and deleting it when not used anymore.

daschuer · 2017-06-21T20:17:59Z

Loading resources from "/home/daniel/workspace/advanced_autodj_2/res/"
src/test/broadcastprofile_test.cpp:47: Failure
Value of: BroadcastProfile::validName( invalidProfile.getProfileName())
Actual: false
Expected: true
[ FAILED ] BroadcastProfileTest.ForbiddenChars (1 ms)

Else it works nice. Thank you.

Palakis · 2017-06-21T21:04:49Z

@daschuer Did you tried with the latest changes? This assert call was removed in one of them.

daschuer · 2017-06-21T21:43:19Z

I have rebuid and tested it again, and it still fails. Is it a linux only issue?

Palakis · 2017-06-22T06:37:34Z

My bad, I was supposed to remove validProfile and invalidProfile in the previous commit, since those are deprecated because the profile name isn't stripped of forbidden chars anymore.

daschuer · 2017-06-22T20:00:00Z

This looks goo to me so far, is there anything left to do in this branch?

Palakis · 2017-06-22T20:33:10Z

@daschuer Not much left to do: making sure it is ready for handling several broadcasting outputs/profiles.
One way I can come up with is to remove setCurrentProfile and getCurrentProfile (which won't be of use for multi-broadcasting), turn BroadcastSettings into a Qt read-only model and implement an overload of QAbstractItemModel::data that directly returns a BroadcastProfilePtr instead of a QVariant. This way I can get profiles by index and replace calls to getCurrentProfile with data.

Palakis · 2017-06-24T17:46:57Z

I believe there's nothing left to do on this. As usual, I'm open for comments on these contributions!
More additions are needed in BroadcastSettings for multi-broadcasting (e.g. : a method to rename a profile), but I believe those will come during phase 2 of GSoC. I'll also work on unit tests for BroadcastSettings.

daschuer · 2017-06-25T00:04:42Z

Fine, am happy with this as well so you can merge it to your GSoC branch and advance to the next PR.
Thank you for the work so far. :-)

Be-ing · 2017-07-25T11:33:20Z

Should this PR be closed?

Palakis · 2017-07-25T12:01:03Z

Yes, absolutely. Work continues in #1300.

Palakis added 12 commits June 2, 2017 14:45

preferences: add WIP class for broadcasting profiles

b75dc85

xml profile schema: refactor tag names to SnakeCase

eb7a29d

(WIP) BroadcastSettings and BroadcastProfile refactor

6449956

No idea if this works. I just want to make sure it is saved somewhere out of my computer. I may squash this with other commits later.

BroadcastSettings: fix behaviour of filename gen. method

afff79f

BroadcastProfile: fix XML load/save code

744cb93

Both ways didn't handled the document's root tag. Now, profile values are saved properly.

BroadcastSettings: enhance default profile creation

c1296be

BroadcastSettings: add qDebug statements

52f8d7e

BroadcastProfile: remove extra qualifications in header file

84dd954

Merge branch 'master' of https://github.com/mixxxdj/mixxx into broadc…

607bab5

…ast-profiles

BroadcastProfile: fix XML file load code

b2d089d

BroadcastSettings: make sure only one instance exists

29e2205

Palakis mentioned this pull request Jun 15, 2017

[WIP] Broadcasting profiles #1278

Closed

9 tasks

Palakis changed the title ~~[WIP] XML engine to save Live Broadcasting settings~~ [WIP] XML config files for Live Broadcasting preferences Jun 15, 2017

daschuer reviewed Jun 15, 2017

View reviewed changes

Palakis added 6 commits June 15, 2017 22:00

Code style fix in EncoderBroadcastSettings

0ee0095

SettingsManager: remove shared pointer type

6196f40

This isn't needed in the current state of the code.

Add auto migration from legacy broadcasting settings

48fd2f2

These additions automatically fetches existing Live Broadcasting preferences from mixxx.cfg, puts them in the Default Profile and removes the now useless keys from mixxx.cfg

Broadcasting settings: don't remove previous settings when migrating

9114789

Move BroadcastProfile::loadLegacySettings to BroadcastSettings

cb2ed7b

Unindent anonymous namespace of BroadcastProfile class

679b61d

Palakis added 5 commits June 16, 2017 15:18

BroadcastProfile: remove non-existent method declaration

0c8392c

Add unit tests for BroadcastProfile class

39cc28a

Fix and rename method BroadcastProfile::checkNameCompliance

ab14026

BroadcastProfile: fix DefaultValues unit test

c6e0af9

BroadcastProfile: fix typo in forbidden characters regex

38c9b1d

Add more tests in the process, because the regex changed.

BroadcastProfile: fix forbidden chars regex syntax

1f2452d

Palakis added 3 commits June 20, 2017 15:51

Minor fixes in DlgPrefBroadcast

6d4e369

BroadcastProfile: replace use of QSharedPointer with std::unique_ptr

7ddba28

BroadcastSettings: hashmap for fast lookup of profiles by name

799dc4e

daschuer reviewed Jun 20, 2017

View reviewed changes

BroadcastProfile: loadFromFile returns a smart pointer

337b260

Palakis added 2 commits June 21, 2017 10:39

Remove uses of C++14 std::make_unique

0676416

Revert removal of std::make_unique and use it properly

b9806f6

Palakis added 2 commits June 21, 2017 11:58

Refactor BroadcastProfile ForbiddenChars unit test

4c55ae6

BroadcastProfile tests: remove deprecated asserts

43468cd

BroadcastProfile: add clarifying comment

50e542b

Palakis added 3 commits June 23, 2017 14:00

BroadcastSettings: implement model and remove get/setCurrentProfile

f4d10b7

BroadcastProfile unit testing: use regular tests

113108d

BroadcastProfile tests: remove useless fixture

57e9047

Palakis changed the title ~~[WIP] XML config files for Live Broadcasting preferences~~ XML config files for Live Broadcasting preferences Jun 23, 2017

Palakis mentioned this pull request Jul 3, 2017

Multi-broadcasting #1300

Merged

19 tasks

Palakis closed this Jul 25, 2017

Palakis deleted the xml-broadcasting-settings branch July 25, 2017 16:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

XML config files for Live Broadcasting preferences #1283

XML config files for Live Broadcasting preferences #1283

Palakis commented Jun 15, 2017 •

edited

Loading

daschuer Jun 15, 2017 •

edited

Loading

Palakis Jun 15, 2017

daschuer Jun 15, 2017

Palakis Jun 15, 2017 •

edited

Loading

daschuer Jun 15, 2017

daschuer commented Jun 16, 2017

sblaisot commented Jun 16, 2017 •

edited

Loading

daschuer Jun 20, 2017

Palakis Jun 20, 2017

daschuer commented Jun 21, 2017 via email

Palakis commented Jun 21, 2017

daschuer commented Jun 21, 2017 •

edited

Loading

Palakis commented Jun 21, 2017

daschuer commented Jun 21, 2017

Palakis commented Jun 22, 2017

daschuer commented Jun 22, 2017

Palakis commented Jun 22, 2017

Palakis commented Jun 24, 2017

daschuer commented Jun 25, 2017

Be-ing commented Jul 25, 2017

Palakis commented Jul 25, 2017

XML config files for Live Broadcasting preferences #1283

XML config files for Live Broadcasting preferences #1283

Conversation

Palakis commented Jun 15, 2017 • edited Loading

daschuer Jun 15, 2017 • edited Loading

Choose a reason for hiding this comment

Palakis Jun 15, 2017

Choose a reason for hiding this comment

daschuer Jun 15, 2017

Choose a reason for hiding this comment

Palakis Jun 15, 2017 • edited Loading

Choose a reason for hiding this comment

daschuer Jun 15, 2017

Choose a reason for hiding this comment

daschuer commented Jun 16, 2017

sblaisot commented Jun 16, 2017 • edited Loading

daschuer Jun 20, 2017

Choose a reason for hiding this comment

Palakis Jun 20, 2017

Choose a reason for hiding this comment

daschuer commented Jun 21, 2017 via email

Palakis commented Jun 21, 2017

daschuer commented Jun 21, 2017 • edited Loading

Palakis commented Jun 21, 2017

daschuer commented Jun 21, 2017

Palakis commented Jun 22, 2017

daschuer commented Jun 22, 2017

Palakis commented Jun 22, 2017

Palakis commented Jun 24, 2017

daschuer commented Jun 25, 2017

Be-ing commented Jul 25, 2017

Palakis commented Jul 25, 2017

Palakis commented Jun 15, 2017 •

edited

Loading

daschuer Jun 15, 2017 •

edited

Loading

Palakis Jun 15, 2017 •

edited

Loading

sblaisot commented Jun 16, 2017 •

edited

Loading

daschuer commented Jun 21, 2017 •

edited

Loading