bbc / brave

Basic Real-time AV Editor - allowing you to preview, mix, and route live audio and video streams on the cloud
Apache License 2.0
656 stars 149 forks source link

latency max < min on rtmp output #48

Closed joaomcferreira closed 4 years ago

joaomcferreira commented 5 years ago

Hello, I'm using Brave and it works just fine.

I found an issue and I was able to fix it. But I don't really know if this would have other side impacts. So I'm stating it here in order to contribute with my finding and maybe the authors will find it useful.

In some situations I get the following gstreamer error and the RTMP output goes red with a clock error.

The solution I found was to reduce the "key-int-max" value from 60 to 30, on "brave/outputs/rtmp.py". The issue is not easily repeatable. I could only have it consistent on an Azure VM. On another machine it stopped happening (I don't know why). On other machines it never happens.

I'm attaching some screen-shots that I hope will clarify this matter. I can submit a pull request if you think that is appropriate.

I would appreciate if the authors of Brave could share their considerations about this finding and the fix I propose. I have no experience with gstreamer.

Thank you João

Screenshot_2019-04-09_19-52-56

Screenshot_2019-04-09_16-30-17

joaomcferreira commented 5 years ago

addding another screenshot showing Brave web interface when such error occurs. Screenshot_2019-04-02_17-03-43

matthew1000 commented 5 years ago

Hi João

Fascinating... we haven't seen a clock problem before.

Nice work finding a workaround. Yes this is useful to learn.

Setting a keyframe to once a second (assuming 30fps) should have no effect, other than being a little less efficient from a networking perspective. I wonder if we should make that the default for Brave. The YouTube and Facebook Live specs say they accept a 1 second keyframe. Though Twitch's docs suggests it mandates 2 seconds (https://help.twitch.tv/s/article/broadcast-requirements?language=en_US). You mentioned Twitch in your GStreamer post, did it work fine?

Great that you've asked the GStreamer list too (http://gstreamer-devel.966125.n4.nabble.com/Gstreamer-error-clock-problem-Azure-VM-td4690187.html). Given the explanation from Nicolas, I can't work out how a lower keyframe helps! More broadly, we have seen other types of timing issues between the video and audio. So it doesn't surprise me it's something in this area.

What would be interesting would be to see if the problem disappears if audio is disabled. If you were to add enable_audio: false to the config file, then Brave removes the audio part of the pipeline and the need to synchronise would disappear. Doing so would confirm that is a video/audio sync issue.

Thanks again for sharing this, Matthew

joaomcferreira commented 5 years ago

Hi Matthew,

:) my pleasure. it was hard to solve the problem. even more because I am not an expert in the topic. I am a software developer. we are building an application and Brave solves us the problem that we do not master gstreamer.

About your question: yes. we have been using Brave to stream to twitch, at 720p and 1080p, mostly with ffmpeg testsrc and testsrc2 signals (at 720p and 1080p), we overlay a simple PNG and it works very well. We have observed 48h continuous operation several times.

We have also streamed to youtube with success too.

Wa are also considering streaming with FFmpeg (because of authentication issues). But Brave is still, and will continue to be our main tool. what we do it push the result to nginx and from there we spread it, with FFmpeg, to several CDNs.

Presenty we have the sources on our repo. The only change we did so far is this one I reported. If this ever gets into the main code, we can consider cloning it again.

Anyway, it's ok.

Thank you for tis great piece of work. we love it. João

joaomcferreira commented 5 years ago

may be you'dd like to consider keeping 60 as default, but allowing the user to change that value through configuration (the data in the PUT that creates the rtmp output). For me it would be a fine solution.

moschopsuk commented 4 years ago

Closing as this is a stale issue.