Adds RedisMpegVideoClass for a redis camera video-streamer #1100
Conversation
There was a problem hiding this comment.
In general it looks good:).
However, I was wondering, since the code is very similar to the one for the TangoLimaMPEGVideoClass, if we could merge the two classes and just difference between them by a set flag? That way, we could prevent confusion that might come with multiple different camera classes. What do you think about the feasibility of this?
| "-irc", | ||
| "mxcubeweb", | ||
| # "-d", |
There was a problem hiding this comment.
I think the redis-channel should not be hardcoded but be configurable through the xml files, as it depends on the actual channel name
There was a problem hiding this comment.
Good point ! I missed that one.
| <username>Camera redis</username> | ||
| <host>localhost</host> | ||
| <uri>redis://localhost:6379</uri> | ||
| <cam_type>redis</cam_type> |
There was a problem hiding this comment.
I think there is no cam_type parameter in the class, hence we might not need to add it to an example
| _s = size | ||
|
|
There was a problem hiding this comment.
I guess this can be removed as you are setting it below
| from mxcubecore.BaseHardwareObjects import HardwareObject | ||
|
|
||
|
|
||
| class RedisMpegVideo(HardwareObject): |
There was a problem hiding this comment.
A solution that would result in less duplication and easier maintenance would be to merge this code with TangoLimaMpegVideo so that both functionalities can be achieved via configuration. It looks like the principal difference is the use of "-irc", "mxcubeweb" which could be added via configuration.
The resulting class could arguably be renamed, perhaps to MpegVideo , what do you think @walesch-yan
There was a problem hiding this comment.
Seeing the comment above it looks like we had the same thought :), sorry for the repetition
There was a problem hiding this comment.
No problem at all:)
After some more investigation, one other important difference is that this class would directly inherit from hardwareobjects, where TangoLimaMPEGVideo inherits from TangoLimaVideo, which seems to contain some more properties, similar to the ones in the RedisMPEGVideoClass, but also some code for directly pulling images from a camera, instead of using the video-streamer, so we might have to check if this is still used
There was a problem hiding this comment.
I'm pretty sure that the inheritance from TangoLimaVideo can be removed now, there might be some methods that needs to be moved but I think TangoLimaVideo can be retired soon. ;)
There was a problem hiding this comment.
Thanks for the inputs. I like the idea of merging the two classes into one MpegVideo class. I am looking into it.
Adds a new class - RedisMpegVideo which acts as a HardwareObject that starts the vodeostreamer for a redis camera connectrd to a redis pub/sub channel.