search

fHDHR / fHDHR_plugin_origin_ceton

Do What The F*ck You Want To Public License
1 stars 4 forks source link

Add rtp support for PCIe card(s) #23

Closed arrmo closed 2 years ago

arrmo commented 2 years ago

@DanAustinGH, the one I mentioned 😄

Adds rtp support for PCIe - like we had discussed in the (long) thread, needs to use the "pcie" IP address, not Ceton => rtp from 192.168.200.2 ... but made it a configurable variable, in case.

Yell if any concerns. Thanks!

DanAustinGH commented 2 years ago

How do you determine the PCIe IP address?

Did you get a chance to benchmark the time to tune a channel for each method and CPU load?

I do think working RTP is worth having, but I'd also like to keep the direct hardware method unless it has a noticeable performance hit...

arrmo commented 2 years ago

How do you determine the PCIe IP address?

I was wondering what to call this, to avoid confusion 🤣. I'm open to ideas. But it's the local PC IP address, on the same subnet as the Ceton => it's where the Ceton RTP streams to, to be able to feed it in from there. Make sense?

Did you get a chance to benchmark the time to tune a channel for each method and CPU load?

Not yet - but not forgotten! My issue is that I have been seeing error with the latest streaming updates - gradually getting worse with time running. This seems to work around it for now ... so making this change, then I can try to debug the dropped frames.

I do think working RTP is worth having, but I'd also like to keep the direct hardware method unless it has a noticeable performance hit...

Agreed! I want that as well, just need to resolve the dropped frames. This gives me breathing room to do that ... LOL.

arrmo commented 2 years ago

Hang on for a minute please - may have found something!

arrmo commented 2 years ago

OK, found a bit of a snag. The Ceton allows direct OR rtp, not both simultaneously. I see that if I enable rtp, I can't pull from the device (i.e. cat). That sort of makes sense, and easy to add the logic to not StartStop if using direct ... checked that here, and it works. Oh, and ... "why didn't this happen before?". Because - the IP and port were being rejected before (by Ceton) ... I corrected that, and now the conflict (as it actually does enable rtp).

The snag - it messes with your approach of checking STOPPED or PLAYING from the Ceton (if using direct, as rtp can't be started). The tuner info is still correct in fHDHR, but not for Ceton. So ... keep track of the state separately (for direct?).

Thoughts?

Thanks!

DanAustinGH commented 2 years ago

We have a method to determine the state for rtp and for direct, so we may just to fiddle with it a bit. I do not see an problem with the module only being able to support on mode or the other.

I considered trying to maintain tuner state, but since the standalone can be whacked by other clients or lose power when the fHDHR host does not, I realized it was going to get ugly. I think that is far less likely on the PCIe, I can still think of ways it might happen.

arrmo commented 2 years ago

We have a method to determine the state for rtp and for direct

Sorry, not sure I fully follow - how to detect for direct?

I can still think of ways it might happen.

Agreed, but also not sure there is really a way to avoid it - in either case (even rtp, another rogue device could take over, right?). I guess no matter what, no answer is really foolproof.

And actually, as fHDHR keeps it's own status - it won't allocate a tuner it thinks is in use. So not sure we really need to detect then at the Ceton level?

Thinking out loud, by all means disagree!

DanAustinGH commented 2 years ago

I thought we worked that out in the other thread, I will have to go re-read it.

RTP is the method most vulnerable to hijacking. If you want to try your hand at tuner tracking for the direct method that works for me.

arrmo commented 2 years ago

I thought we worked that out in the other thread, I will have to go re-read it.

We may have, and I just forgot - or didn't get it 🤣

RTP is the method most vulnerable to hijacking.

Agreed!

If you want to try your hand at tuner tracking for the direct method that works for me.

Sure, will take a look! The good thing is - fHDHR at the "top" level is tracking correctly.

arrmo commented 2 years ago

OK, think I found a way to detect tuner usage ... rtp and direct (device) both. Trying to code here, seems to work. Want me to push a PR, so you can test it out as well?

I also added some items to the web interface, to help with display during debugging 😄.

Thanks!

DanAustinGH commented 2 years ago

Definitely submit the changes and I will review as soon as I can...

arrmo commented 2 years ago

Will do! Slight redirect ... latest upstream changes break things - let me dig.

arrmo commented 2 years ago

Updating my code, not a huge issue, but ... stream_method seems to have gone away? Thinking it's a different name now, but ... @deathbybandaid - with your changes, not sure how I determine direct or rtp in the plugin now?

Thanks!

deathbybandaid commented 2 years ago

stream_args["method"​]

The stream_args dict is passed to the get_channel_stream function.

There's other information there that has value.

arrmo commented 2 years ago

Hmmm ... getting this,

  File "/mnt/ProgSSD/fHDHR/plugins/fHDHR_plugin_origin_ceton/origin/__init__.py", line 222, in get_channel_stream
    self.stream_method = stream_args["method"]
KeyError: 'method'
2022-03-02T03:46:59Z {'REMOTE_ADDR': '192.168.2.64', 'REMOTE_PORT': '36642', 'HTTP_HOST': 'linuxserver.rkmorris.home:5006', (hidden keys: 20)} failed with KeyError
arrmo commented 2 years ago

FYI, I removed my check for now - force it, but then,

  File "/mnt/ProgSSD/fHDHR/fHDHR/streammanager/stream_obj.py", line 24, in __init__
    self.setup_channel_stream()
  File "/mnt/ProgSSD/fHDHR/fHDHR/streammanager/stream_obj.py", line 91, in setup_channel_stream
    if self.tuner_needed:
  File "/mnt/ProgSSD/fHDHR/fHDHR/streammanager/stream_obj.py", line 43, in tuner_needed
    if self.stream_args["method"] == "passthrough":
KeyError: 'method'
2022-03-02T03:49:09Z {'REMOTE_ADDR': '192.168.2.64', 'REMOTE_PORT': '36690', 'HTTP_HOST': 'linuxserver.rkmorris.home:5006', (hidden keys: 20)} failed with KeyError

Thanks!

deathbybandaid commented 2 years ago

Toss a print line in or what stream_args show in ceton, also don't set self.method like that

arrmo commented 2 years ago

Funny, had done just exactly that 🤣. We are in sync!

{'channel': '593', 'channel_name': 'Golf HD', 'channel_callsign': 'Golf HD', 'origin': 'ceton', 'accessed': 'http://linuxserver.rkmorris.home:5006/api/tuners?method=stream&channel=65089df3-ff53-45b4-8656-9a0844d4edd8&origin=ceton', 'base_url': 'http://linuxserver.rkmorris.home:5006', 'client': '192.168.2.64', 'client_id': 'be7dc00c-5f63-4e9a-952d-a692d0559e6a', 'origin_quality': None, 'bytes_per_read': 1152000, 'buffer_size': 3, 'stream_restore_attempts': 3}
{'channel': '593', 'channel_name': 'Golf HD', 'channel_callsign': 'Golf HD', 'origin': 'ceton', 'accessed': 'http://linuxserver.rkmorris.home:5006/api/tuners?method=stream&channel=65089df3-ff53-45b4-8656-9a0844d4edd8&origin=ceton', 'base_url': 'http://linuxserver.rkmorris.home:5006', 'client': '192.168.2.64', 'client_id': 'c44ad89e-3af1-4edc-b34e-984e49e75ec9', 'origin_quality': None, 'bytes_per_read': 1152000, 'buffer_size': 3, 'stream_restore_attempts': 3}

Didn't want to set that, but I need it in startstop_ceton_tuner, and it's not available there 😞. So was adding this in to be able to use it in startstop.

deathbybandaid commented 2 years ago

Just pushed revert

arrmo commented 2 years ago

Yep! Fixes that part of it (though I can change to the dict if you want, no issue - the key just isn't there right now). Streaming works now (ffmpeg, need to test direct yet), but on close this happens now (didn't before the recent changes),

[2022-03-01 22:12:31,451] INFO - Stream Ended: Client has disconnected.
[2022-03-01 22:12:31,451] INFO - Removing Tuner Lock
[2022-03-01 22:12:31,452] INFO - Tuner #0 Released.
[2022-03-01 22:12:31,453] INFO - Removing 3 chunks from the buffer.
[2022-03-01 22:12:31,453] INFO - Running ceton close_stream method.
Traceback (most recent call last):
  File "/mnt/ProgSSD/fHDHR/venv/lib/python3.9/site-packages/gevent/pywsgi.py", line 946, in run_application
    self.process_result()
  File "/mnt/ProgSSD/fHDHR/venv/lib/python3.9/site-packages/gevent/pywsgi.py", line 932, in process_result
    self.write(data)
  File "/mnt/ProgSSD/fHDHR/venv/lib/python3.9/site-packages/gevent/pywsgi.py", line 775, in write
    self._write(data)
  File "/mnt/ProgSSD/fHDHR/venv/lib/python3.9/site-packages/gevent/pywsgi.py", line 760, in _write
    self._sendall(towrite)
  File "/mnt/ProgSSD/fHDHR/venv/lib/python3.9/site-packages/gevent/pywsgi.py", line 736, in _sendall
    self.socket.sendall(data)
  File "/mnt/ProgSSD/fHDHR/venv/lib/python3.9/site-packages/gevent/_socketcommon.py", line 699, in sendall
    return _sendall(self, data_memory, flags)
  File "/mnt/ProgSSD/fHDHR/venv/lib/python3.9/site-packages/gevent/_socketcommon.py", line 409, in _sendall
    timeleft = __send_chunk(socket, chunk, flags, timeleft, end)
  File "/mnt/ProgSSD/fHDHR/venv/lib/python3.9/site-packages/gevent/_socketcommon.py", line 338, in __send_chunk
    data_sent += socket.send(chunk, flags)
  File "/mnt/ProgSSD/fHDHR/venv/lib/python3.9/site-packages/gevent/_socketcommon.py", line 722, in send
    return self._sock.send(data, flags)
BrokenPipeError: [Errno 32] Broken pipe

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/mnt/ProgSSD/fHDHR/venv/lib/python3.9/site-packages/gevent/pywsgi.py", line 999, in handle_one_response
    self.run_application()
  File "/mnt/ProgSSD/fHDHR/venv/lib/python3.9/site-packages/gevent/pywsgi.py", line 951, in run_application
    close()
  File "/mnt/ProgSSD/fHDHR/venv/lib/python3.9/site-packages/werkzeug/wsgi.py", line 466, in close
    callback()
  File "/mnt/ProgSSD/fHDHR/venv/lib/python3.9/site-packages/werkzeug/wrappers/response.py", line 437, in close
    self.response.close()  # type: ignore
  File "/mnt/ProgSSD/fHDHR/venv/lib/python3.9/site-packages/flask/helpers.py", line 131, in generator
    yield from gen
  File "/mnt/ProgSSD/fHDHR/fHDHR/streammanager/stream/__init__.py", line 235, in buffer_generator
    self.stream_obj.origin_plugin.close_stream(self.tuner.number, self.stream_obj.stream_args)
  File "/mnt/ProgSSD/fHDHR/fHDHR/origins/origin.py", line 99, in close_stream
    self.method.close_stream()
TypeError: close_stream() missing 2 required positional arguments: 'instance' and 'args'
2022-03-02T04:12:31Z {'REMOTE_ADDR': '192.168.2.64', 'REMOTE_PORT': '36796', 'HTTP_HOST': 'linuxserver.rkmorris.home:5006', (hidden keys: 22)} failed with TypeError

[2022-03-01 22:12:31,465] INFO - 192.168.2.64 - - [2022-03-01 22:12:31] "GET /api/tuners?method=stream&channel=65089df3-ff53-45b4-8656-9a0844d4edd8&origin=ceton HTTP/1.1" 200 50690755 29.742824

Thanks!

arrmo commented 2 years ago

FYI, direct working also. Leaving a test running overnight now ... vlc, two windows - /dev directly into vlc, and one through fHDHR, with debug loglevel. Let's see 😄.

Thanks!

deathbybandaid commented 2 years ago

https://github.com/fHDHR/fHDHR/pull/296 fixes close_stream and also adds a new prime_stream for plugins that need a head-start before fHDHR starts pulling content

arrmo commented 2 years ago

That works, thanks! FYI, just one that I noticed ... combination type thing (only happens with loglevel = noob, and ffmpeg ... not with direct!), Similar to a recent fix you included, but perhaps just missed ffmpeg streaming?

[2022-03-02 07:12:59,291] NOOB - Set your [logging]level to INFO if you wish to see more logging output.
[2022-03-02 07:13:50,833] ERROR - Unable to Stream: TunerError: Tuner Setup Failed (lazily handled): Tuner Setup Failed (lazily handled):KeyError 'noob'- /mnt/ProgSSD/fHDHR/fHDHR/streammanager/stream/__init__.py line 81
[2022-03-02 07:13:51,594] ERROR - Unable to Stream: TunerError: Tuner Setup Failed (lazily handled): Tuner Setup Failed (lazily handled):KeyError 'noob'- /mnt/ProgSSD/fHDHR/fHDHR/streammanager/stream/__init__.py line 81
deathbybandaid commented 2 years ago

I have no idea why that is happening to you. I cannot recreate

deathbybandaid commented 2 years ago

@arrmo try pulling new push for ffmpeg plugin

https://github.com/fHDHR/fHDHR_plugin_stream_ffmpeg/pull/9

arrmo commented 2 years ago

That works! Apologies if this was me - I thought I pulled all the latest plugins, but I could have missed one 😞.

OK, back to this update ... LOL. Actually, it's almost fully working here, now, with updates to the UI as well. Will try to get the PR in today, so you can look it over.

FYI, I will open a new (top level) issue, related to dropped frames (as I have mentioned). Have some results to share now, but let me clean them up first.

Thanks again!

arrmo commented 2 years ago

@DanAustinGH OK, the updated code - enjoy! 😄. Definitely let me know your thoughts. A few items to help explain,

  1. this supports both rtp (udp) and direct for the pcie card
  2. checks are done for both (in use) ... rtp streaming, or direct access
  3. I tied in updating the UI, as it helped me to debug. By all means, propose naming changes if you want!
  4. like I had mentioned, I have to "block" starting rtp streaming, if direct is desired => they are mutually exclusive, the pcie card outputs to rtp OR /dev (direct), not both

@deathbybandaid, by all means comment as well.

Thanks!

PS: A little screenshot from the "updated" UI ... oh, and I see that my daughter is using Plex right now 🤣 image

DanAustinGH commented 2 years ago

Looks good. I may have not called the hw in use function if using RTP, but I see you dealt with it in the function. The only minor nit I have is your logging states that the device is PCIe, which can be assumed for direct hardware, but not RTP.

And also super minor, but in the screenshot it looks odd if Transport is 'Stopped' and in HW State is in Use. And that made me think what would my non-PCI tuner report for HW state. We might want it to manually set it to 'N/A' for remote devices or not display it at all.

Neither are enough to prevent me from merging, unless you want to tweak them before a merge.

arrmo commented 2 years ago

I may have not called the hw in use function if using RTP, but I see you dealt with it in the function.

"in use" is meant to indicate direct access, to the device. I'm all for different naming - just propose something and I'll change it. Really no issue at all!

The only minor nit I have is your logging states that the device is PCIe, which can be assumed for direct hardware, but not RTP.

True, Want me to change that note somehow? I was there as I was working my way through :smile:

And that made me think what would my non-PCI tuner report for HW state. We might want it to manually set it to 'N/A' for remote devices or not display it at all.

Good point! I could change HW State to N/A for remote, like you say. I'm definitely good with that!

Neither are enough to prevent me from merging, unless you want to tweak them before a merge.

Let's tweak it, no issue. I'm game 😄. Just let me know your thoughts on these, and I'll update.

DanAustinGH commented 2 years ago

How about merging Transport and HW and use these states (idle/RTP/Direct). For the logging just remove 'PCIe' fro line 254 in init.py

arrmo commented 2 years ago

Will do! Give me just a little bit - as I noticed, daughter is connected, can't restart it right now ... LOL.

arrmo commented 2 years ago

Like this? image

DanAustinGH commented 2 years ago

Yes! People with stand-alone devices will only See idle or RTP and the PCIe crowd will see what they are configured for.

arrmo commented 2 years ago

Agree with you! This does look better, appreciate the suggestion. Another set of eyes always good 👍

Pushed this update, I think we're aligned - but feel free to disagree.

Thanks!

DanAustinGH commented 2 years ago

I'm going to merge this and will try to get my stand-alone environment updated for testing today...

arrmo commented 2 years ago

Sounds great, thanks! If you see any issues - poke me. We'll get it working for both 👍

DanAustinGH commented 2 years ago

This broke the stand-alone device. The logic to not make the Start API call looks good, but my RTP streams failed with streaming set to ffmpeg or direct. I changed it to if not self.ceton_pcie and my device is working again.

arrmo commented 2 years ago

I changed it to if not self.ceton_pcie and my device is working again.

Do you mean in the Start API? Is it detecting your stand-alone device as PCIe for some reason? Perhaps print that variable out, so we can check it?

Other than this one thing - the rest working? BTW, from the Ceton UI => manually start one tuner streaming (to never never land 😜) ... the UI should detect it.

Thanks!

DanAustinGH commented 2 years ago

This is the problem line- if not (startstop and self.stream_method == 'direct'):

For the stand-alone device it has to call the API, and cannot use method = direct SO that logic should work startstop = 1 and the second evaluation (method = ffmpeg ) = 0 so that should deliver the result you want, but it did not.

arrmo commented 2 years ago

Dang it! I spent time thinking about that logic too ... LOL.

My thought => proceed to send the startstop "command", except for "start, and stream is direct". Agreed? I think you're saying that the logic makes sense to you as well.

I am seeing something odd here as well, given the bunch of other recent changes. I'm wondering if that URL, for example, isn't quite right. No, that can't be it ... you just changed that one line, and it works. Just a sec 😆.

arrmo commented 2 years ago

Just printed out the variables, and the output is exactly as expected. Then it hit me ... I did have to make some changes to adapt to upstream mods. Did you also pull in the latest from the ffmpeg plugin, and fHDHR itself? Meaning ... are those main branches fully updated on your local?

Thanks!

DanAustinGH commented 2 years ago

Yup I pulled those first, and then promptly forgot that my local copy has the ceton code pulled from my repo and not the fHDHR repos. So I was missing your changes. Once I figured that out I found my RTP streams would not start. Your logic looks good, so I am going to revert my tweak and try again...

arrmo commented 2 years ago

So glad it's not just me who does that 😜

Thanks!

DanAustinGH commented 2 years ago

And that is what I get for doing 10 things at the same time. Most repos pulled cleanly, but the core did not due to some small change I tested months ago, but never committed or reverted, and I did not spot that the pull failed. Your code is fine...

arrmo commented 2 years ago

No worries at all! I have done that way too many times myself. Glad it's all working for you!

deathbybandaid commented 2 years ago

Then there's me over here merging channels into the origins system.

Channels system worked great back when I only had to support a single origin.

DanAustinGH commented 2 years ago

I was worried the death of Locast would demotivate development around the emulator/proxy apps. I am happy to see that has not been the case....

arrmo commented 2 years ago

Agreed! I think this works really well - I even have some plugins I'd like get to, when I have time 😄

arrmo commented 2 years ago

BTW, not forgotten about the direct vs. rtp / ffmpeg startup timing. Trying to find a way to measure it automated. I'm open to suggestions!

DanAustinGH commented 2 years ago

Perfect is the enemy of good. Plex, emby and other players will all introduce some variance to the measurements. I'm just curious if one 'feels' quicker than the other. For me Emby usually tunes in 2~3 seconds. I've had a few .5 second tune and a few 5 second. I suspect that direct will be a bit quicker and a bit more consistent, having eliminated a bit of network traffic and maybe benefiting from the direct stream not having the UDP packet overhead.

arrmo commented 2 years ago

Perfect is the enemy of good.

Agreed! You must be an engineer (too) 😜

For me Emby usually tunes in 2~3 seconds.

Very interesting timing. I say that because ... the answer was staring me in the face, finally smacked me. Here is what I figured out (yell if you disagree of course!),

Use a client (I chose ffmpeg), that I can time => run a fixed duration of video, then remove that duration from the total time ... the remaining time is startup. Agreed so far? So I went with,

time ffmpeg -i 'http://192.168.2.64:5006/api/tuners?method=stream&channel=593&origin=ceton' -t 30 -f null /dev/null

This will time ffmpeg, connecting to the server, tuning to channel 593, and processing 30 seconds of video (dumping it to the trash). I started with 30 seconds in case of any startup artifacts / glitches, make sure the video is running smooth (which 30 seconds should ensure). The result?

real    0m33.337s
user    0m5.556s
sys     0m0.166s

Take off 30 seconds => startup is 3.3 seconds. Not too far from what you mentioned. Of course, different clients will be different, but that's on the client then 😆. I then tried 3 seconds of video, got,

real    0m6.500s
user    0m0.368s
sys     0m0.041s

So 3.5 seconds of startup ... works for me! Don't save this number quite yet - as I have been messing with code here, let me undo some things, get real numbers.

Oh, and something to noodle on. I was also thinking about CI/CD in the back of my mind, cases to run automated, confirm the build is good. Interestingly enough, the command above returns 0 ... as expected. If I pick a bogus channel, I get 1. Woohoo, this could be a CI/CD test, agreed?