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

Add Remuxing Support to MediaServer #138

Closed enternoescape closed 8 years ago

enternoescape commented 8 years ago

I just decided to tinker around with the proposed solution in the thread below. I was able to implement MPEGParser for use on the MediaServer last night. I also think I was able to overcome the random crashing mentioned in the thread and so far, I haven't seen any files being stuck until I restart SageTV. I able to make SageTV crash sometimes by closing the Remuxer twice, so the solution there was to just make it null once it has been closed. I also made it crash by overloading it with too much data at one time, which I solved by forcing the data to be fed in smaller chunks.

http://forums.sagetv.com/forums/showthread.php?t=62406

I was really impressed at how easy it was to get working. The bits I'm working on right now are just optimizing.

I noticed that MPEG-TS output only worked correctly when I fed it data in 188 byte increments. If I used anything bigger, the resulting video is playable, but corrupted. MPEG-PS output only complained when I went a lot higher than 64KB (around 2MB). Do these limitations sound familiar?

For setup, I used the suggestion of "REMUX_SETUP Format Channel InitSize." I implemented retaining the data used for detection, then once detection is successful, the data is written feed into pushData in 188 byte or 64KB chunks per the reasons in the above paragraph. I also added a check that if more than 5 seconds of time according to the container passed and the format has been detected to continue anyway even if we have not reached InitSize. That way you don't get stuck for a long time on low bitrate streams. Also if the InitSize is too little for the format to be detected, it will double in size up to 5MB before it will discard the buffered data and continue to buffer again. That should be way more than enough data based on my observations. After successful detection, the buffer is replaced with a 64KB buffer since that's the largest chunk it could possibly need to deal with when copying to the remuxer.

I was wondering about any suggestions on feedback to the network encoder so it can know when initialization is done and remuxing has actually started. This would be helpful to provide more seamless channel changes for example. Could I just add another command that allows the network encoder to query the current status of the remuxer?

The only downside I see to using the remuxing like this is if for some reason the network connection is disrupted. The remuxer doesn't appear to use any kind of offset, so it's unclear if you could actually add an offset as an option.

enternoescape commented 8 years ago

In the UNKNOWN_CHANNEL struct, is tsid equivalent to the program number or am I missing it's purpose completely? I noticed that when tune_string_type in the TUNE struct is set to 1 (program) that ConsolidateTuneParam() still uses the value of unkn.data1, but I don't think that's actually being used as the program number. I hope this isn't too confusing. I'm trying to make as much of this as is reasonable available to the JVM.

Narflex commented 8 years ago

I'm not sure exactly what's going on there...I didn't write that code and it's fairly complex so I'm not sure how that value is getting used in other areas (grep finds a lot of hits). Since you're only doing the remuxing after the proper TS should have already been extracted; you probably aren't going to gain much functionality wise from dealing with the TUNE struct. Personally, I'd just go with the flag for forcing it to the TV media type and not bother to make it more advanced than that....unless you already have use cases that would matter for that....then by all means go for it. :)

qianzhang5 commented 8 years ago

If there is only one program (one channel) in a file there should not be a problem using this code to remux. if there are multiple channels in data, picking up a correct channel replies on TUNE setting which needs program id, tsid or other parameter. because the code underneath handles ATSC, QAM, DVB-T/C/S standards, there is not one simple way to work for them all. it may confuse.

On Fri, Jun 10, 2016 at 7:19 AM, Joseph Shuttlesworth < notifications@github.com> wrote:

In the UNKNOWN_CHANNEL struct, is tsid equivalent to the program number or am I missing it's purpose completely? I noticed that when tune_string_type in the TUNE struct is set to 1 (program) that ConsolidateTuneParam() still uses the value of unkn.data1, but I don't think that's actually being used as the program number. I hope this isn't too confusing. I'm trying to make as much of this as is reasonable available to the JVM.

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

enternoescape commented 8 years ago

Understood. Usually the driver or hardware is filtering out the other programs on the same frequency. I was just trying to cover as much ground as is reasonable. I exposed what I'm reasonably certain I understand to the JVM. The code allows the person using the remuxer to decide just how deep they want to go based on what method they use to open the remuxer.

From the MediaServer perspective, the command is now REMUX_SETUP mode outputFormat parameters.... The only mode supported right now is AUTO and other modes could be added if they are needed for whatever reasons. The format for AUTO would be REMUX_SETUP AUTO outputFormat isTV. Ex. REMUX_SETUP AUTO PS TRUE

Narflex commented 8 years ago

Sounds good to me. :)

On Fri, Jun 10, 2016 at 11:40 AM, Joseph Shuttlesworth < notifications@github.com> wrote:

Understood. Usually the driver or hardware is filtering out the other programs on the same frequency. I was just trying to cover as much ground as is reasonable. I exposed what I'm reasonably certain I understand to the JVM. The code allows the person using the remuxer to decide just how deep they want to go based on what method they use to open the remuxer.

From the MediaServer perspective, the command is now REMUX_SETUP mode outputFormat parameters.... The only mode supported right now is AUTO and other modes could be added if they are needed for whatever reasons. The format for AUTO would be REMUX_SETUP AUTO outputFormat isTV. Ex. REMUX_SETUP AUTO PS TRUE

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

Jeffrey Kardatzke jkardatzke@google.com Google, Inc.

enternoescape commented 8 years ago

So I ran into something unexpected trying to find an I frame in some HD MPEG-2 TS files. It looks like it's only interlaced content. I can find the 4 byte header 0x00000100, but the only picture codes I'm getting are B and P, no I. Have you ever seen this before? Is this why MulticastCaptureDevice doesn't try to find a transition point for MPEG-2 files?

qianzhang5 commented 8 years ago

No, I haven't. you may use ffprobe with -show_frames to check.

On Sun, Jun 12, 2016 at 6:43 PM, Joseph Shuttlesworth < notifications@github.com> wrote:

So I ran into something unexpected trying to find an I frame in some HD MPEG-2 TS files. It looks like it's only interlaced content. I can find the 4 byte header 0x00000100, but the only picture codes I'm getting are B and P, no I. Have you ever seen this before? Is this why MulticastCaptureDevice doesn't try to find a transition point for MPEG-2 files?

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

Narflex commented 8 years ago

I haven't seen that problem before either, and the Java code didn't do it because we just never wrote it.

Jeff Kardatzke Sent from my Android On Jun 12, 2016 10:53 PM, "qianzhang5" notifications@github.com wrote:

No, I haven't. you may use ffprobe with -show_frames to check.

On Sun, Jun 12, 2016 at 6:43 PM, Joseph Shuttlesworth < notifications@github.com> wrote:

So I ran into something unexpected trying to find an I frame in some HD MPEG-2 TS files. It looks like it's only interlaced content. I can find the 4 byte header 0x00000100, but the only picture codes I'm getting are B and P, no I. Have you ever seen this before? Is this why MulticastCaptureDevice doesn't try to find a transition point for MPEG-2 files?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/google/sagetv/issues/138#issuecomment-225473873, or mute the thread < https://github.com/notifications/unsubscribe/AMQtgHjKS5S6EQYVrKVWRU-9AIoJHuNmks5qLLWtgaJpZM4IeL_U

.

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

enternoescape commented 8 years ago

@qianzhang5 Thanks for the suggestion. ffprobe found them and pointed me right at the offset I should be looking at. I was also able see what I was looking for in a hex editor.

I made a really dumb mistake in reading the test file...basically it wasn't reading new data and the I frame was just past the length of the buffer. Sorry for the hassle and thanks for the quick replies.

enternoescape commented 8 years ago

Are we aware that building libNativeCore.so with gcc 5.3.1 may result in what appears to be data alignment issues? I was able to compile a good copy with gcc 4.8.4. I thought I messed something up that simply didn't appear in Windows. I'm glad I tried compiling on Ubuntu 14.04. The resultant library from the newer compiler was writing out files with small amounts of seemingly random corruption.

qianzhang5 commented 8 years ago

Make sure your in new code variables are portable (such as int8_t, int32_t...) and test code with 64 bit and 32 bits compiler on windows and linux.

On Mon, Jun 13, 2016 at 8:49 PM, Joseph Shuttlesworth < notifications@github.com> wrote:

Are we aware that building libNativeCore.so with gcc 5.3.1 may result in what appears to be data alignment issues? I was able to compile a good copy with gcc 4.8.4. I thought I messed something up that simply didn't appear in Windows. I'm glad I tried compiling on Ubuntu 14.04. The resultant library from the newer compiler was writing out files with small amounts of seemingly random corruption.

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

stuckless commented 8 years ago

@joseph - is the gcc4.8.4 still 64bit? It is interesting that 5.3.1 would cause corruption, specially if they both 64bit. My docker container for building sagetv is ubuntu14..04... but that was mainly because I can't build mplayer with anything newer. At some point I'd like to use a newer gcc.

On Tue, 14 Jun 2016 at 01:52 qianzhang5 notifications@github.com wrote:

Make sure your in new code variables are portable (such as int8_t, int32_t...) and test code with 64 bit and 32 bits compiler on windows and linux.

On Mon, Jun 13, 2016 at 8:49 PM, Joseph Shuttlesworth < notifications@github.com> wrote:

Are we aware that building libNativeCore.so with gcc 5.3.1 may result in what appears to be data alignment issues? I was able to compile a good copy with gcc 4.8.4. I thought I messed something up that simply didn't appear in Windows. I'm glad I tried compiling on Ubuntu 14.04. The resultant library from the newer compiler was writing out files with small amounts of seemingly random corruption.

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

.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/google/sagetv/issues/138#issuecomment-225786385, or mute the thread https://github.com/notifications/unsubscribe/ABAfKW065fY3xNz4kSj-DvITjSe8PHtjks5qLkG2gaJpZM4IeL_U .

Sent from Android

enternoescape commented 8 years ago

For the most part I have matched the variables to exactly how they are defined within the existing SageTV libraries. I will change the code that I'm adding to use the platform independent variables. The signatures in the core libraries are often using int and unsigned long, so I will not be changing those.

I know the latest gcc compiler doesn't play nicely with several pieces of software that work fine on the older one. I don't know what the changes are that might be breaking them, but I've mostly seen it break A/V software. It causes the software to think something is wrong with the packets and it tries to fix them which just results in them being mangled or some of the original content being wiped. To me that sounds like somewhere we are using a variable that might be involved in some bit-math and the number of bytes in the variable in this compiler have changed.

enternoescape commented 8 years ago

@stuckless Yes, both compilers are 64-bit. The testing of the compiled libraries was done on a Fedora 23 64-bit VM. It's worth noting that SageTV started acting a little strangely with the libNativeCore.so compiled with the newer gcc. When I re-compiled everything with the older version of gcc, all was well again.

I was reading over some of the changes between 4 and 5 and this one stuck out: The default mode for C is now -std=gnu11 instead of -std=gnu89. libNativeCore.so from what I can tell is entirely C.

I compiled on Ubuntu 16.04 (gcc 5.3.1 64-bit) this morning and interestingly enough it doesn't have any issues. I guess Fedora 23 64-bit is doing something just different enough it breaks things. I know Fedora is a fairly on the edge distro, so maybe my compiler has a bug.

CraziFuzzy commented 8 years ago

What would it take to have an option for the remuxer to output other containers as well (not just m2ps or m2ts, but mkv maybe)?

enternoescape commented 8 years ago

Qian should have a better idea on this, but based on what I've seen, someone would need to add another path for the incoming packets to travel to go into an MKV container. Someone would also need to add code to the code on the Java side of things so the video playback would not get too far ahead or too far behind the live feed. If you're not arguing for live playback, then FFmpeg is the better route since that means no one on the SageTV side of things will have much to maintain.

Narflex commented 8 years ago

When you were doing the compiler comparisons, did you do a full rebuild of everything rather than just the one library? It may be worth checking another tree out just to be sure it is completely clean, although I don't believe there are any artifacts left behind after doing a clean that would cause problems....but it's possible.

Jeff Kardatzke Sent from my Android On Jun 14, 2016 8:54 AM, "Joseph Shuttlesworth" notifications@github.com wrote:

Qian should have a better idea on this, but based on what I've seen, someone would need to add another path for the incoming packets to travel to go into an MKV container. Someone would also need to add code to the push code on the Java side of things so the video playback would not get too far ahead or too far behind the live feed. If you're not arguing for live playback, then FFmpeg is the better route since that means no one on the SageTV side of things will have much to maintain.

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

enternoescape commented 8 years ago

I did a full rebuild; then a few partial while I was trying to figure out what went wrong. Basically after I confirmed that I was able to compile everything successfully in Linux and Windows, I committed all of my changes to GitHub, then used git clone in a new location on each platform to be sure it would compile from scratch.

enternoescape commented 8 years ago

I was starting to work on integrating the switching and size checking so that it doesn't even hit the network encoder. It looks like channel changing used to also be a part of switching, but we stopped doing that. Is that the case or do I need to account for this and pass the switching on to the network encoder if the channel changes? I don't think I've ever seen the channel change at the same time as a switch.

Narflex commented 8 years ago

We never change the channel the same time we do a SWITCH; that may have been an artifact from a long time ago (because in the old days, SWITCH used to mean something a little different).

On Thu, Jun 16, 2016 at 9:52 AM, Joseph Shuttlesworth < notifications@github.com> wrote:

I was starting to work on integrating the switching and size checking so that it doesn't even hit the network encoder. It looks like channel changing used to also be a part of switching, but we stopped doing that. Is that no longer the case or do I need to account for this and pass the switching on to the network encoder if the channel changes? I don't think I've ever seen the channel change at the same time as a switch.

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

Jeffrey Kardatzke jkardatzke@google.com Google, Inc.

enternoescape commented 8 years ago

Got it. That means that SWITCH can happen without even telling the network encoder. That will be a very cool feature since once this is added, any network encoder will be able to support SWITCH by simply using media server remuxer.

The way I am tying the remuxer into the network capture device is via a ConcurrentHashMap by File. The value returned is a MediaServerRemuxer which is a new class I added so I wasn't cluttering up the MediaServer class. When a MediaServerRemuxer is closed, the mapping is removed from the HashMap. When switching, the old File is removed from the HashMap and the new File is added with the same object reference.

GET_FILE_SIZE is trivial with the HashMap and is working great.

Narflex commented 8 years ago

Oh yeah, great point...that would remove the Network Encoder from having to do anything for SWITCH if it's using your remuxer. :)

On Thu, Jun 16, 2016 at 11:42 AM, Joseph Shuttlesworth < notifications@github.com> wrote:

Got it. That means that SWITCH can happen without even telling the network encoder. That will be a very cool feature since once this is added, any network encoder will be able to support SWITCH by simply using media server remuxer.

The way I am tying the remuxer into the network capture device is via a ConcurrentHashMap by File. The value returned is a MediaServerRemuxer which is a new class I added so I wasn't cluttering up the MediaServer class. When a MediaServerRemuxer is closed, the mapping is removed from the HashMap. When switching, the old File is removed from the HashMap and the new File is added with the same object reference.

GET_FILE_SIZE is trivial with the HashMap and is working great.

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

Jeffrey Kardatzke jkardatzke@google.com Google, Inc.

enternoescape commented 8 years ago

I have been using the new remuxer for several days in production (Windows) and on my testing VM's (Windows & Linux 64-bit). It's working great. Let me know if you need me to make any changes. It really seemed like this would be a bigger task than it actually turned out to be.

143

Narflex commented 8 years ago

Glad to hear it wasn't as bad as you thought. :) SageTV has LOTS of stuff inside of it...so quite often things can get done much easier than you may think. :D

I reviewed the code; the Java code I went through in more depth...my main goal is to ensure you don't break anything that's there. :) I glossed over the rest of it; and it looked fine to me. I did have a few comments on the Java code. I'm assuming you'll also have a flag in the network encoder code of yours to turn this on and off as another type of debugging assist if problems arise.

And thanks for taking this on! I greatly appreciate developers contributing to SageTV. Also, don't forget to add your name to the CONTRIBUTORS file if you haven't done that already.

enternoescape commented 8 years ago

OpenDCT has several ways to stream the video, you can stream it raw directly to a file or via the media server, remuxed or transcoded via FFmpeg directly to a file or via the media server and I just added another method which uses the remuxer I just added to the media server. So if you're counting, that's 5 different ways to record.

I also added a command to disable the network encoder "assistance" that allows SageTV to talk directly to the remuxer instead of the network encoder for SWITCH and GET_FILE_SIZE in case something odd is discovered.

I really really like this software. It has so much already and I really want to see it thrive. If full-time development was an option, I'd be all over it. I added myself to CONTRIBUTORS.

enternoescape commented 8 years ago

75 comments later... Done! :)