google / sagetv

SageTV is a cross-platform networked DVR and media management system
http://forums.sagetv.com/
Apache License 2.0
267 stars 174 forks source link

Refactor SageTV into multiple projects #300

Open stuckless opened 7 years ago

stuckless commented 7 years ago

SageTV has long history of development, and as it happens, that tends to lead to monlithic code structures where everything is in a single project. This makes it harder to manage in the long run.

Initially there are some obvious boundaries

There maybe circular dependencies there, that would need to be solved in the process.

In addition to these projects, I'd like to see the "native" parts split out as well. Some time ago, I started this process, because I wanted to introduce unit tests that could run withtout the native parts. In the end I just did a "hack" and introduced the Native class that would load a library and optionally, not fail, if it wasn't there. (this is big task in how to logically structure things, not a big task in how it's done)

But, I think that the native parts should be accessed via more concrete interfaces. Currently Sage.java uses a lot of "native" method implemetations, and I think much of that should be delegated to native implementation classes. eg, there might be a WindowsNative class that defines getRegistryNames() and in Sage.java we can initialize the native implementations on startup, something slie getRegistryNames in Sage.java calls "nativeImpl.getRegistryNames()". This helps keep the native parts separate, but also allows for us to swap out the native parts with non-native parts should we need to (ie, for unit testing, etc).

I also would like to see OS specific implementation code in a separate project, or at least in a separate package space; sage.linux, sage.windows, sage.mac, sage.android.

In the process of moving to multiple projects, I'd like to see the UI/AWT/SWING code in a separate projects as well. There are numerous places in Sage where AWT libraries are used, and because of this, it makes it hard to port a server to something like Android. SageTV doesn't "depend" directly on awt to run (with the exception of some dependenices on Dimension, etc, but these can be abstracted out, and awt implementations can be injected when needed).

Lastly, sagetv has a lot fo "EMBEDDED" flags. We should determine if we still need/want to support an EMBEDDED mode.

CraziFuzzy commented 7 years ago

I think @Narflex mentioned to me at one point in the past that he doesn't see any need to keep the embedded tags, and they could easily be removed.

I definitely agree with the desire to remove native bits from the 'core' project - and I honestly would love to see the server be it's own package that would not have ANY native bits, and any native things remaining would reside in a separate process entirely. There are already network based methods for just about every interaction the server has with the outside world (tuners, clients, management, etc), I'd love to have a server that would only interact externally via those methods.

enternoescape commented 7 years ago

This sounds terrific. I was just thinking about how important it is to make things truly modular and how we could best accomplish this in SageTV. This would also make testing a lot easier and address some of the cases where correct testing isn't even possible right now.

I believe @Narflex has said a few times that removal of the EMBEDDED flagged code is not a problem and I have been doing it when I update code lately.

I would also like to see some of the larger shared components at least be moved into their own packages to make the project better organized and to better control how the internal references are accessed and updated. The Wizard is a good candidate for this and cleaning/simplifying that whole situation up could merit its own issue.

Narflex commented 7 years ago

This sounds good. :) And yes, EMBEDDED stuff can all get removed...I don't see anything that would ever use it in the future...especially if you try to break things apart like you're talking about here.

On Sun, Jun 4, 2017 at 8:42 PM, Joseph Shuttlesworth < notifications@github.com> wrote:

This sounds terrific. I was just thinking about how important it is to make things truly modular and how we could best accomplish this in SageTV. This would also make testing a lot easier and address some of the cases where correct testing isn't even possible right now.

I believe @Narflex https://github.com/narflex has said a few times that removal of the EMBEDDED flagged code is not a problem and I have been doing it when I update code lately.

I would also like to see some of the larger shared components at least be moved into their own packages to make the project better organized and to better control how the internal references are accessed and updated. The Wizard is a good candidate for this and cleaning/simplifying that whole situation up could merit its own issue.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/google/sagetv/issues/300#issuecomment-306096447, or mute the thread https://github.com/notifications/unsubscribe-auth/ANEIDJoOnFeoRydddsmnq8kc6Deud9nCks5sA3kfgaJpZM4NvXRn .

-- Jeffrey Kardatzke jkardatzke@google.com Google, Inc.

CraziFuzzy commented 7 years ago

So. Here's a crazy thought. What if the native capture bits were moved into openDCT as new producer(s), with openDCT becoming the new SageTV recorder service? This certainly would provide a lot more capability to sage, and 'normalize' pretty much any source (even custom feeds via the http and pipe methods) .

stuckless commented 7 years ago

I can't say much about the capture service stuff, but, I think for the reorganization, we should start with a new branch, and then work out some of the structural changes there. Later when we are happy with it, we can move it into the mast branch. That way we can continue to work on the master fixes and enhancements while we are working out the details on how the parts should be carved up.

If I get some time this weekend, I'll create the branch, and maybe start with a couple projects and go from there.

Sean.

On Mon, Jun 5, 2017 at 8:13 PM Christopher Piper notifications@github.com wrote:

So. Here's a crazy thought. What if the native capture bits were moved into openDCT as new producer(s), with openDCT becoming the new SageTV recorder service? This certainly would provide a lot more capability to sage, and 'normalize' pretty much any source (even custom feeds via the http and pipe methods) .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/google/sagetv/issues/300#issuecomment-306345535, or mute the thread https://github.com/notifications/unsubscribe-auth/ABAfKbNVLKBQ53RNjlDFMswVsQ5vORnYks5sBJmrgaJpZM4NvXRn .

--

Sent from Android

CraziFuzzy commented 7 years ago

well, really what I was describing wasn't even so much a 'move' of the capture stuff, as it was an expansion of openDCT's capabilities. Frankly, if it was made to support ffmpeg as a producer directly it could likely handle any source type available, and most the work would be in configuration and tuning - since ffmpeg itself supports v4l2, dshow, and vfw as sources. If this was done, the work in the core would be more a excision of the native capture bits, really just leaving in the network encoder bits.

enternoescape commented 7 years ago

I don't get warm feelings when we talk about accessing native things via executables. Unless it's for fairly simple things (and even then I don't like it), it feels very hacky when a native library could have been used because a library should give us a lot more flexibility down the road. I like the idea of using FFmpeg to replace some of the core remuxing logic only because it's one less thing we would need to maintain with in SageTV. I don't think FFmpeg supports as much as we actually need to properly tune/support some capture devices. I have worked with it a little in this area and it seemed very basic capability-wise. Also SageTV's V4L2 capabilities as far as I've been able to tell are fine and really don't need any serious attention.

OpenDCT has it's own flaws and it might be a good place to leap from for concepts and some code, but if we are actually going to design an engine/service for capture devices for SageTV external to the primary JVM, I would write something specifically for that purpose with the expectation that it might eventually fully replace the need for OpenDCT. We could also use OpenDCT for code that does not really need to be reinvented or only need minor changes.

I went a little overboard in some areas of OpenDCT resulting in some code that's really hard to understand why it's there at times, for example, I very strictly separate capture devices that can be used from capture devices that are disabled mostly so that you can rename them at runtime and to prevent them from creating a bunch of properties for a capture device you're not even using. SageTV simplifies this by instead of having two different classes, just having one class and a few methods to enable and disable inputs. The caveat is you get a lot of properties for devices you're not even using which shouldn't bother anyone only using the UI.

I have also grown a crazy lot in knowledge about the Java programming language over the past year. Most of my usable knowledge was from C# programming which helps, but it's still a different way of thinking at times. Having a better understanding of these things now would lead me down a slightly different design path and potentially an even more efficient solution.

I am working on a Seeker 2.0 kind of situation (that's actually good enough I wouldn't be worried about using it in my own production). I was considering adding CableCARD (Ceton and Prime) support in the process to help demo how multi-program support works. If we really want to better isolate the capture devices, maybe I should be gearing my efforts more in that direction with my design instead of shoehorning it into the current model.

Going one step further, I am in favor of using (seemingly little publicized) Dagger 2 or at the very least passing dependencies through constructors from factories instead of the singleton design. I really don't care for Guice or Spring in the sense that they add to runtime overhead to construct new objects because they have to use reflection and they add a level of obscurity to the code that makes refactoring and sometimes debugging difficult. I strongly believe that the singletons are one of the biggest barriers to making the code fully testable.

This is probably at least 3 more issues. I wish there was some way to create issues surrounding large initiatives like this so that we can isolate these issues from issues with the project everyone else is using. I guess a label or milestone would be best. I'm also thinking we might want to produce some ideas of generally where we thinks things belong so that we can all make sure we're on the same page.

CraziFuzzy commented 7 years ago

I was talking in generalities, really, when stating simply using openDCT. Yes, I feel some things would need to change in openDCT to make it into the proper/de facto sagetv recorder service - the least of which are more configuration options from within the sagetv ui via improvements to the network encoder interface on both ends. I also didn't specificly mean using ffmpeg.exe (or the linux build), but still using javacpp's avxxxx implementations. I think avdevice includes most the interaction with hardware input and output devices (but not absolutely sure, since it is pretty limited).

All that said, I'm not sure I understand the aversion to using an external executable. If launched properly, it can provide better process isolation, as well as avoidance of JNI complications, and provide for some level of built-in future-proofing. I feel it would be relatively simply to get the ffmpeg devs to include a mediaserver format into the core ffmpeg project (there are plenty of much less useful formats in there), if we developed one, which could ultimately mean the actions required to record a program would consist of a single commandline launch of ffmpeg, listing the input hardware options, and output directly to sage via stv://uploadid@server/path/to/file.ext as the output options.

stuckless commented 7 years ago

I spent several hours yesterday and today, just doing a trial run on separating sagetv into multiple projects. It's going to be a big task. As suspected, there are many cyclical dependencies, and even more suprising to me, is that some of the third_party/ projects have cyclical dependencies on sage core, and vice versa.

I think before I go all in on the project refactoring, I think I'm going to start with some clean up tasks and then move into the physical spliting apart, and I think it will help in the long run, based on the challenges that I found this weekend.

So to start, I think we should remove EMBEDDED and just simplify the code paths there. That's a simple one.

Next I'd like to look at the places where .exec (and dirivatives) are used. These really signify "native" access points and I think we could abstract those and then creates Linux/Win/Mac implementations where needed, and expose it through interfaces. This will likely lead into refactoring some classes like IOUtils into, IOUtils, IOUtilsLinux, IOUtilsWindows, etc.

The next major thing that I'd like to tackle is Sage.java. This is an immensely shared component and it does everything form native access, string utils, debugging, localization, clipping rectangles, scan channels, send windows messages, manage the startup, splash window, etc. It's basically a sink for anything. I think separting Sage.java into a separate Util type classes will greatly simply moving different classes to different projects, because right now, EVERYTHING depends on Sage.java in some way. Personally, I think refactoring Sage.java will do more for being able to refactor SageTV than anything else.

enternoescape commented 7 years ago

You might also be interested in Seeker.java since it does a lot of stuff beyond controlling the encoders. We also have some constructors reaching outside during construction.