-
Notifications
You must be signed in to change notification settings - Fork 131
Support for MPEG-TS livestreaming via local TCP proxy server #1078
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@fronzbot I thought re-creating the PR from a different source branch would fix the coverage issue, but unfortunately it doesn't: |
|
@fronzbot The tests are now 100% complete and this PR is ready for review, but there is one caveat: Here again the code example to launch the MPEG-TS TCP server for a given camera: import asyncio
import os.path
from aiohttp import ClientSession
from blinkpy.blinkpy import Blink
from blinkpy.auth import Auth, BlinkTwoFARequiredError
from blinkpy.helpers.util import json_load
async def start():
session = ClientSession()
blink = Blink(session=session)
if os.path.exists("blink.json"):
auth = Auth(await json_load("blink.json"), session=session)
blink.auth = auth
try:
await blink.start()
except BlinkTwoFARequiredError:
await blink.prompt_2fa()
await blink.save("blink.json")
camera = blink.cameras["MyCamera"]
stream = await camera.init_livestream()
await stream.start()
print(stream.url)
await stream.feed()
await session.close()
asyncio.run(start())
I am looking forward to your feedback. |
855b3d5 to
8768fe4
Compare
|
@fronzbot Tests and CI are green now with the exception of the coverage due to an issue with CI: See https://github.com/fronzbot/blinkpy/actions/runs/16658736107/job/47150437518?pr=1078 |
|
@fronzbot I hope you are well, do you see any chance to move forward with this? I guess a lot of people would enjoy this, especially once I integrate this into the Home Assistant integration. |
fronzbot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh man, totally forgot about this PR, I'm really sorry.
Only a small comment to move a function to prevent duplicate code 👍
|
Waiting for this change! Hopefully we will be able to have a lot of new possibilities in HA! |
|
I am busy at the moment, but will update the PR as soon as possible. |
|
I can confirm this works (or rather worked before the login API changes). It was not super stable and the lag was maybe 10 seconds or so (I tried different stream / ffmpeg configurations but could no fix it), but at least there was something, so I would say it's a good base for improvement. I think I only tested it with Minis. @mback2k can you also rebase to current release? Is it supposed to work with doorbell/indoor/outdoor as well? Was it tested with these? If not do you need some help testing? |
4fa4bfe to
4a1ef43
Compare
Thanks for testing. In my tests I have the same ~6 second delay in stream initialization as with the Blink app itself, but once the stream is running it is quite smooth and stable.
I have re-based / merged the current dev changes and indeed it works again. I have successfully tested Outdoor 3, Doorbell and Mini. Coverage testing still fails the CI due to the lack of some token, would you mind taking a look at that @fronzbot? |
|
@mback2k yeah the coverage stuff broke awhile back and I haven't gotten around to fixing it, so it can just be ignored. One point I want to clarify: you mentioned |
|
Thanks, I originally had the timeout in the code, but removed it since Blink servers will close the connection anyway if there is an issue with streaming. So, client side timeout enforcement is not necessarily needed as far as my testing showed. |
|
Of course I can follow up with a separate PR for the timeout if needed. |
|
One quick question, well, two indeed (sorry if they are stupid questions...):
|
|
The change is in the development version, but not in any version available on pip at the moment. Once I make a release, there will need to be extra work done to include it into home-assistant so it might be some time before that happens. |
|
Ok, thank you for the clarification! |
|
I already have a working updated HA integration locally and will work on a PR once this feature is released. |
I had to create a fresh branch and PR due to being unable to check coverage with a PR originating from a branch named
dev.Description:
Support livestreaming of MPEG-TS based on the work described in #343:
Related issue (if applicable): fixes #343
Replaces #1051
Checklist:
toxrun successfully PR cannot be meged unless tests pass