You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@openmeetings.apache.org by "seba.wagner@gmail.com" <se...@gmail.com> on 2021/03/04 19:43:27 UTC
Refactor Kurento and Stream handling to make configurable
As tried out in https://github.com/apache/openmeetings/pull/135
=> There are a lot of methods in those interfaces. Most of them have
little to do with handling streams or Kurento.
I would suggest we refactor first a few things, and then introduce the
interfaces after:
1. Move static methods out of those classes into Util classes
2. Move methods like StreamProcessor.hasRightsToShare,
streamProcessor.getBySid and other methods that do only rely on class
Client.java and ClientManager. Those methods should simply go into
ClientManager.
3. Refactor so that "facade" methods like StreamProcessor.isRecording -
which simply could call a (new) method in kHandler.isRecording - so that
they call the right place right away
So the idea would be that:
1. StreamHandler holds the list of KStreams and makes operations to
get/add/remove those
2. KurentoHandler holds the list of KRoom and makes operations to
get/add/remove those (+ holds the reference to the KurentoClient)
After that we can discuss the next steps. I think there will be still more
to do before introducing Interfaces.
But I think having those classes being clear on their purpose and moving
methods unrelated to streams/Kurento out of them will make it a lot easier
- Easier to add new behaviour, Interfaces but also to make it easier to
concentrate when adding new behaviour to not look at a lot of
methods/functionality unrelated to handling streams/Kurento.
I would make a new PR based on master for above.
Thanks
Seb
Sebastian Wagner
Director Arrakeen Solutions, OM-Hosting.com
http://arrakeen-solutions.co.nz/
https://om-hosting.com - Cloud & Server Hosting for HTML5
Video-Conferencing OpenMeetings
<https://www.youracclaim.com/badges/da4e8828-743d-4968-af6f-49033f10d60a/public_url>
<https://www.youracclaim.com/badges/b7e709c6-aa87-4b02-9faf-099038475e36/public_url>
Re: Refactor Kurento and Stream handling to make configurable
Posted by "seba.wagner@gmail.com" <se...@gmail.com>.
Have a look at:
https://github.com/apache/openmeetings/pull/136
It just does some minimal changes:
- Move hasActivity/hasRight to existing methods in Client.java
- Static methods into Util class
- Remove wrapper/methods and pass in dependency in Constructor that are
required for class
Thanks
Seb
Sebastian Wagner
Director Arrakeen Solutions, OM-Hosting.com
http://arrakeen-solutions.co.nz/
https://om-hosting.com - Cloud & Server Hosting for HTML5
Video-Conferencing OpenMeetings
<https://www.youracclaim.com/badges/da4e8828-743d-4968-af6f-49033f10d60a/public_url>
<https://www.youracclaim.com/badges/b7e709c6-aa87-4b02-9faf-099038475e36/public_url>
On Fri, 5 Mar 2021 at 16:02, Maxim Solodovnik <so...@gmail.com> wrote:
> Hello Sebastian,
>
> unfortunately my free time is very limited :(
>
> On Fri, 5 Mar 2021 at 02:43, seba.wagner@gmail.com <se...@gmail.com>
> wrote:
>
>> As tried out in https://github.com/apache/openmeetings/pull/135
>> => There are a lot of methods in those interfaces. Most of them have
>> little to do with handling streams or Kurento.
>>
>
> yes,
> by introducing such interface(s) you make yourself to copy/paste almost
> all contents of classes you would like to generalize
>
>
>>
>> I would suggest we refactor first a few things, and then introduce the
>> interfaces after:
>>
>> 1. Move static methods out of those classes into Util classes
>> 2. Move methods like StreamProcessor.hasRightsToShare,
>> streamProcessor.getBySid and other methods that do only rely on class
>> Client.java and ClientManager. Those methods should simply go into
>> ClientManager.
>> 3. Refactor so that "facade" methods like StreamProcessor.isRecording
>> - which simply could call a (new) method in kHandler.isRecording - so that
>> they call the right place right away
>>
>> So the idea would be that:
>> 1. StreamHandler holds the list of KStreams and makes operations to
>> get/add/remove those
>> 2. KurentoHandler holds the list of KRoom and makes operations to
>> get/add/remove those (+ holds the reference to the KurentoClient)
>>
>
> All above is not required for phase0/phase1
> all you need for these phases is to add reference to KurentoClient to
> KStream (and use it for this particular MediaPipeline)
>
> I would like to suggest you to start implement your alternative clustering
> this way you will see what are the "generalisation points" and only then
> made any changed to current implementations
>
> And the very first thing I would do is to invent the way to plug-in new
> implementation into current code
>
> a quick search pointed me to @Primary Spring annotation which might help
> (and this also requires zero changes to OM code ...)
>
>
>> After that we can discuss the next steps. I think there will be still
>> more to do before introducing Interfaces.
>>
>> But I think having those classes being clear on their purpose and moving
>> methods unrelated to streams/Kurento out of them will make it a lot easier
>> - Easier to add new behaviour, Interfaces but also to make it easier to
>> concentrate when adding new behaviour to not look at a lot of
>> methods/functionality unrelated to handling streams/Kurento.
>>
>> I would make a new PR based on master for above.
>>
>> Thanks
>> Seb
>>
>> Sebastian Wagner
>> Director Arrakeen Solutions, OM-Hosting.com
>> http://arrakeen-solutions.co.nz/
>> https://om-hosting.com - Cloud & Server Hosting for HTML5
>> Video-Conferencing OpenMeetings
>>
>> <https://www.youracclaim.com/badges/da4e8828-743d-4968-af6f-49033f10d60a/public_url>
>> <https://www.youracclaim.com/badges/b7e709c6-aa87-4b02-9faf-099038475e36/public_url>
>>
>
>
> --
> Best regards,
> Maxim
>
Re: Refactor Kurento and Stream handling to make configurable
Posted by Maxim Solodovnik <so...@gmail.com>.
Hello Sebastian,
currently it is the time i need to do my day-time job
I can only spend my free time for open source
so please expect delays in answers
I believe the very first thing to investigate/implement (with small to none
changes in OM code) is good enough way to replace "vanilla" beans with
customized once
@Primary can help here if
your custom bean will have @Primary annotation AND
- bean ID will be the same for "vanilla" bean and your custom bean
- and/or your custom bean will "extend" vanilla bean overriding some
methods
this have to be investigated
IMO the less changes will be required - the better
my goal is to limit the amount of necessary actions to "copy jar file then
restart OM" ...
On Fri, 5 Mar 2021 at 12:30, seba.wagner@gmail.com <se...@gmail.com>
wrote:
> @Primary
> But those annotations are on source code level. It would mean to switch
> which bean to use you need to recompile the source code.
>
> So that doesn't really allow configuring a different bean to be used at
> runtime based on configuration.
>
> Thanks
> Sebastian
>
> Sebastian Wagner
> Director Arrakeen Solutions, OM-Hosting.com
> http://arrakeen-solutions.co.nz/
> https://om-hosting.com - Cloud & Server Hosting for HTML5
> Video-Conferencing OpenMeetings
>
> <https://www.youracclaim.com/badges/da4e8828-743d-4968-af6f-49033f10d60a/public_url>
> <https://www.youracclaim.com/badges/b7e709c6-aa87-4b02-9faf-099038475e36/public_url>
>
>
> On Fri, 5 Mar 2021 at 16:02, Maxim Solodovnik <so...@gmail.com>
> wrote:
>
>> Hello Sebastian,
>>
>> unfortunately my free time is very limited :(
>>
>> On Fri, 5 Mar 2021 at 02:43, seba.wagner@gmail.com <se...@gmail.com>
>> wrote:
>>
>>> As tried out in https://github.com/apache/openmeetings/pull/135
>>> => There are a lot of methods in those interfaces. Most of them have
>>> little to do with handling streams or Kurento.
>>>
>>
>> yes,
>> by introducing such interface(s) you make yourself to copy/paste almost
>> all contents of classes you would like to generalize
>>
>>
>>>
>>> I would suggest we refactor first a few things, and then introduce the
>>> interfaces after:
>>>
>>> 1. Move static methods out of those classes into Util classes
>>> 2. Move methods like StreamProcessor.hasRightsToShare,
>>> streamProcessor.getBySid and other methods that do only rely on class
>>> Client.java and ClientManager. Those methods should simply go into
>>> ClientManager.
>>> 3. Refactor so that "facade" methods like
>>> StreamProcessor.isRecording - which simply could call a (new) method in
>>> kHandler.isRecording - so that they call the right place right away
>>>
>>> So the idea would be that:
>>> 1. StreamHandler holds the list of KStreams and makes operations to
>>> get/add/remove those
>>> 2. KurentoHandler holds the list of KRoom and makes operations to
>>> get/add/remove those (+ holds the reference to the KurentoClient)
>>>
>>
>> All above is not required for phase0/phase1
>> all you need for these phases is to add reference to KurentoClient to
>> KStream (and use it for this particular MediaPipeline)
>>
>> I would like to suggest you to start implement your alternative clustering
>> this way you will see what are the "generalisation points" and only then
>> made any changed to current implementations
>>
>> And the very first thing I would do is to invent the way to plug-in new
>> implementation into current code
>>
>> a quick search pointed me to @Primary Spring annotation which might help
>> (and this also requires zero changes to OM code ...)
>>
>>
>>> After that we can discuss the next steps. I think there will be still
>>> more to do before introducing Interfaces.
>>>
>>> But I think having those classes being clear on their purpose and moving
>>> methods unrelated to streams/Kurento out of them will make it a lot easier
>>> - Easier to add new behaviour, Interfaces but also to make it easier to
>>> concentrate when adding new behaviour to not look at a lot of
>>> methods/functionality unrelated to handling streams/Kurento.
>>>
>>> I would make a new PR based on master for above.
>>>
>>> Thanks
>>> Seb
>>>
>>> Sebastian Wagner
>>> Director Arrakeen Solutions, OM-Hosting.com
>>> http://arrakeen-solutions.co.nz/
>>> https://om-hosting.com - Cloud & Server Hosting for HTML5
>>> Video-Conferencing OpenMeetings
>>>
>>> <https://www.youracclaim.com/badges/da4e8828-743d-4968-af6f-49033f10d60a/public_url>
>>> <https://www.youracclaim.com/badges/b7e709c6-aa87-4b02-9faf-099038475e36/public_url>
>>>
>>
>>
>> --
>> Best regards,
>> Maxim
>>
>
--
Best regards,
Maxim
Re: Refactor Kurento and Stream handling to make configurable
Posted by "seba.wagner@gmail.com" <se...@gmail.com>.
@Primary
But those annotations are on source code level. It would mean to switch
which bean to use you need to recompile the source code.
So that doesn't really allow configuring a different bean to be used at
runtime based on configuration.
Thanks
Sebastian
Sebastian Wagner
Director Arrakeen Solutions, OM-Hosting.com
http://arrakeen-solutions.co.nz/
https://om-hosting.com - Cloud & Server Hosting for HTML5
Video-Conferencing OpenMeetings
<https://www.youracclaim.com/badges/da4e8828-743d-4968-af6f-49033f10d60a/public_url>
<https://www.youracclaim.com/badges/b7e709c6-aa87-4b02-9faf-099038475e36/public_url>
On Fri, 5 Mar 2021 at 16:02, Maxim Solodovnik <so...@gmail.com> wrote:
> Hello Sebastian,
>
> unfortunately my free time is very limited :(
>
> On Fri, 5 Mar 2021 at 02:43, seba.wagner@gmail.com <se...@gmail.com>
> wrote:
>
>> As tried out in https://github.com/apache/openmeetings/pull/135
>> => There are a lot of methods in those interfaces. Most of them have
>> little to do with handling streams or Kurento.
>>
>
> yes,
> by introducing such interface(s) you make yourself to copy/paste almost
> all contents of classes you would like to generalize
>
>
>>
>> I would suggest we refactor first a few things, and then introduce the
>> interfaces after:
>>
>> 1. Move static methods out of those classes into Util classes
>> 2. Move methods like StreamProcessor.hasRightsToShare,
>> streamProcessor.getBySid and other methods that do only rely on class
>> Client.java and ClientManager. Those methods should simply go into
>> ClientManager.
>> 3. Refactor so that "facade" methods like StreamProcessor.isRecording
>> - which simply could call a (new) method in kHandler.isRecording - so that
>> they call the right place right away
>>
>> So the idea would be that:
>> 1. StreamHandler holds the list of KStreams and makes operations to
>> get/add/remove those
>> 2. KurentoHandler holds the list of KRoom and makes operations to
>> get/add/remove those (+ holds the reference to the KurentoClient)
>>
>
> All above is not required for phase0/phase1
> all you need for these phases is to add reference to KurentoClient to
> KStream (and use it for this particular MediaPipeline)
>
> I would like to suggest you to start implement your alternative clustering
> this way you will see what are the "generalisation points" and only then
> made any changed to current implementations
>
> And the very first thing I would do is to invent the way to plug-in new
> implementation into current code
>
> a quick search pointed me to @Primary Spring annotation which might help
> (and this also requires zero changes to OM code ...)
>
>
>> After that we can discuss the next steps. I think there will be still
>> more to do before introducing Interfaces.
>>
>> But I think having those classes being clear on their purpose and moving
>> methods unrelated to streams/Kurento out of them will make it a lot easier
>> - Easier to add new behaviour, Interfaces but also to make it easier to
>> concentrate when adding new behaviour to not look at a lot of
>> methods/functionality unrelated to handling streams/Kurento.
>>
>> I would make a new PR based on master for above.
>>
>> Thanks
>> Seb
>>
>> Sebastian Wagner
>> Director Arrakeen Solutions, OM-Hosting.com
>> http://arrakeen-solutions.co.nz/
>> https://om-hosting.com - Cloud & Server Hosting for HTML5
>> Video-Conferencing OpenMeetings
>>
>> <https://www.youracclaim.com/badges/da4e8828-743d-4968-af6f-49033f10d60a/public_url>
>> <https://www.youracclaim.com/badges/b7e709c6-aa87-4b02-9faf-099038475e36/public_url>
>>
>
>
> --
> Best regards,
> Maxim
>
Re: Refactor Kurento and Stream handling to make configurable
Posted by Maxim Solodovnik <so...@gmail.com>.
Hello Sebastian,
unfortunately my free time is very limited :(
On Fri, 5 Mar 2021 at 02:43, seba.wagner@gmail.com <se...@gmail.com>
wrote:
> As tried out in https://github.com/apache/openmeetings/pull/135
> => There are a lot of methods in those interfaces. Most of them have
> little to do with handling streams or Kurento.
>
yes,
by introducing such interface(s) you make yourself to copy/paste almost all
contents of classes you would like to generalize
>
> I would suggest we refactor first a few things, and then introduce the
> interfaces after:
>
> 1. Move static methods out of those classes into Util classes
> 2. Move methods like StreamProcessor.hasRightsToShare,
> streamProcessor.getBySid and other methods that do only rely on class
> Client.java and ClientManager. Those methods should simply go into
> ClientManager.
> 3. Refactor so that "facade" methods like StreamProcessor.isRecording
> - which simply could call a (new) method in kHandler.isRecording - so that
> they call the right place right away
>
> So the idea would be that:
> 1. StreamHandler holds the list of KStreams and makes operations to
> get/add/remove those
> 2. KurentoHandler holds the list of KRoom and makes operations to
> get/add/remove those (+ holds the reference to the KurentoClient)
>
All above is not required for phase0/phase1
all you need for these phases is to add reference to KurentoClient to
KStream (and use it for this particular MediaPipeline)
I would like to suggest you to start implement your alternative clustering
this way you will see what are the "generalisation points" and only then
made any changed to current implementations
And the very first thing I would do is to invent the way to plug-in new
implementation into current code
a quick search pointed me to @Primary Spring annotation which might help
(and this also requires zero changes to OM code ...)
> After that we can discuss the next steps. I think there will be still more
> to do before introducing Interfaces.
>
> But I think having those classes being clear on their purpose and moving
> methods unrelated to streams/Kurento out of them will make it a lot easier
> - Easier to add new behaviour, Interfaces but also to make it easier to
> concentrate when adding new behaviour to not look at a lot of
> methods/functionality unrelated to handling streams/Kurento.
>
> I would make a new PR based on master for above.
>
> Thanks
> Seb
>
> Sebastian Wagner
> Director Arrakeen Solutions, OM-Hosting.com
> http://arrakeen-solutions.co.nz/
> https://om-hosting.com - Cloud & Server Hosting for HTML5
> Video-Conferencing OpenMeetings
>
> <https://www.youracclaim.com/badges/da4e8828-743d-4968-af6f-49033f10d60a/public_url>
> <https://www.youracclaim.com/badges/b7e709c6-aa87-4b02-9faf-099038475e36/public_url>
>
--
Best regards,
Maxim