Skip to content
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

Metadata issue leading to out-of-sync audio after recording on server #1410

Closed
zachsimone opened this issue Apr 8, 2024 · 16 comments · Fixed by #1446
Closed

Metadata issue leading to out-of-sync audio after recording on server #1410

zachsimone opened this issue Apr 8, 2024 · 16 comments · Fixed by #1446
Milestone

Comments

@zachsimone
Copy link
Contributor

Describe the bug

When live streaming, we record the stream so we can send it off later to be transcoded and stored allowing people to re-watch it after the fact. We've noticed that since the HaishinKit 1.7.5 update, specifically commit 3052fce, the audio and video are out of sync on this recorded video. It is fine while live. When we revert commit 3052fce, things are fine again.

Using MediaInfo (I'll attach screenshots below), we notice there's an error Missing ID_END that shows on a file where the audio and video are out of sync. This could be a clue as to what's going wrong. Note that the error doesn't show using the earlier version (1.7.4) of HaishinKit.

Specify it as the second argument in the constructor of RTMPStream:

stream = RTMPStream(connection, fcPublishName)
Set a value for stream.fcPublishName.

I note the above suggestion from the 1.7.5 release notes, and have tried both of these methods to set the fcPublishName to what it was being set to, but with no success. I wonder if it's got to do with the order changing for when the publish command is sent.

To Reproduce

  1. Conduct live stream with 1.7.5, noting the new way of setting fcPublishName
  2. Capture live stream via an nginx server using the nginx-rtmp-module with the record all; option set in the nginx config
  3. Notice that video and audio are out of sync
  4. Repeat with v1.7.4 (or just by reverting commit 3052fce) and note things are fine.

Note that it doesn't happen every time, but it seems like it's more easily reproducible when the audio source is external (e.g. AirPods).

Expected behavior

Audio and video in sync on the recording, like it behaves when using 1.7.4.

Version

1.7.5

Smartphone info.

Reproducible on multiple devices running iOS 17.3, including iPhone 15 Pro, 15 Pro Max, iPhone 13 Pro, iPad Pro 11" 3rd gen.

Additional context

No response

Screenshots

Screenshots showing the MediaInfo differences when using 1.7.4 vs 1.7.5. Also attached 2x recordings (inside Recordings.zip) showing a recording with 1.7.4 vs 1.7.5.

CleanShot_2024-04-04_at_17 08 022x
CleanShot_2024-04-04_at_17 08 332x

Recordings.zip

Relevant log output

No response

@shogo4405
Copy link
Owner

I tried analyzing the received FLV file. It seems that there is one audio header information before and after the metadata. There seem to be cases where it works and cases where it doesn't. Could you please provide the FLV file that worked when it was successful?

スクリーンショット 2024-04-08 22 22 57

@shogo4405
Copy link
Owner

What will happen with specifying fcPublishName pattern in this branch? Will it be similar? https://github.com/shogo4405/HaishinKit.swift/pull/1414/files

@shogo4405
Copy link
Owner

I confirmed it with VideoLAN and ffplay, but I couldn't notice any delay between the video and audio. How are you playing it?

@zachsimone
Copy link
Contributor Author

Hi @shogo4405, thank you for the quick reply.

What will happen with specifying fcPublishName pattern in this branch? Will it be similar? https://github.com/shogo4405/HaishinKit.swift/pull/1414/files

Unfortunately I still see the issue when using this newer version.

I couldn't notice any delay between the video and audio. How are you playing it?

My apologies, I didn't check before uploading and uploaded the wrong files. Attaching new recordings below. One is definitely out of sync, and the other is working well. The problematic one is noted in the file name. I'm playing them back using VLC on macOS.

Please let me know if I can provide any more information that might be helpful here :-)

Recordings.zip

@shogo4405
Copy link
Owner

This is an image showing a dump of the contents of an FLV file. It can be observed that the timestamp of the data starts from 579, indicating a misalignment. Applications like VideoLAN tend to adhere to this timestamp, resulting in a discrepancy between the video and audio. It's implemented in https://github.com/shogo4405/HaishinKit.swift/tree/main/Examples/macOS.

スクリーンショット 2024-04-12 10 17 51

@shogo4405
Copy link
Owner

Unfortunately, I'm struggling to reproduce it on my end.
I'm testing it in the following environment. Would the same issue occur in the Example iOS?

  • iPhone15 Pro max + AirPods Pro + 44100.0 khz
  • main branch: 1c24433
  • Example iOS
% brew info rtmp-nginx-module
==> marcqualie/nginx/rtmp-nginx-module: stable 1.2.2-r1
NGINX-based Media Streaming Server
https://github.com/sergey-dryabzhinsky/nginx-rtmp-module
/opt/homebrew/Cellar/rtmp-nginx-module/1.2.2-r1_3 (94 files, 1.4MB) *
  Built from source on 2023-11-02 at 01:54:03
From: https://github.com/marcqualie/homebrew-nginx/blob/HEAD/Formula/rtmp-nginx-module.rb

@zachsimone
Copy link
Contributor Author

Thanks @shogo4405. Just a quick note to let you know I haven't forgotten about this and I'm trying to find a good way to reproduce that is decoupled from our streaming infrastructure. I'll let you know when I have more info.

@levs42
Copy link
Contributor

levs42 commented May 7, 2024

Just a thought after looking at the change, before the change FCPublish was called right after cleaning the timestamps with dataTimestamps.removeAll(). Now it's triggered from from createStream by handling NetStream.Connect.Success event. Maybe if there's a longer network delay before the createStream is triggered, the timestamps will accumulate the delay? Manually delaying createStream call would confirm that.

@alexmck
Copy link

alexmck commented May 8, 2024

Thank you @levs42. That gives me something to test to confirm this theory, and actually makes some sense as to why we might be seeing this issue.

@alexmck
Copy link

alexmck commented May 10, 2024

I believe I now have a method to consistently reproduce this error. I'm going to sanity check it with Zach and we will come back.

@alexmck
Copy link

alexmck commented May 13, 2024

I now have a way to reliably reproduce this issue. I've created a simple project that runs nginx-rtmp-module inside of a Docker container and a simple Node server to validate RTMP stream requests: https://github.com/alexmck/docker-nginx-rtmp

There are two RTMP end points in this project, /live which does not send a request to authorise a stream, and /delay which adds an artificial 2 second delay while it authorises a stream.

@zachsimone was able to confirm my findings. Recordings made to the /live RTMP endpoint had no audio and video sync issues, while some recordings to the /delay RTMP endpoint did exhibit audio and video sync issues. I should note that in our testing, it wasn't every stream to the /delay endpoint. In my testing with the sample HaishinKit app, I found that a second stream would often have a recording that would exhibit the audio and video sync issues. Zach said in his testing it was anywhere from the 2nd to 6th test stream.

The significance here is that many live streaming platforms authorise an RTMP stream before allowing the user to stream, while users who are streaming with 4G and poor latency may also exhibit the same problem even without authorisation from the server.

Please let me know if there is anything else I can do to help resolve this issue or if you have any questions about my testing methodology.

@shogo4405
Copy link
Owner

shogo4405 commented May 13, 2024

If dataTimestamps is the cause, I believe I can improve it with this commit. There is a race condition because lockQueue is used within send(), but I believe we have a 100% reset understanding here. Please confirm.
main...feature/fix-outof-sync
https://github.com/shogo4405/HaishinKit.swift/tree/feature/fix-outof-sync

If possible, could you provide the code to delay nginx? My email is shogo4405@gmail.com.

@alexmck
Copy link

alexmck commented May 15, 2024

Thanks for your attempted fix. I have tested this branch but unfortunately the bug still exists. Zach was also still able to reproduce this issue with this branch.

To delay the stream in nginx, I am using the on_publish directive in my nginx.conf. This on_publish method sends an HTTP request to a simple Node HTTP server I set up which adds a 2 second delay before allowing the stream to continue. My server.js file runs the simple HTTP server that adds this 2 second delay.

Please let me know if I can provide you with any more details or answer anymore questions.

shogo4405 added a commit that referenced this issue May 19, 2024
@shogo4405
Copy link
Owner

@alexmck I was able to reproduce the issue using the method you introduced. Thank you. #1410 (comment)

I have confirmed that the issue is improved with this PR on my end. Please check if it works the same in your environment.#1446

@alexmck
Copy link

alexmck commented May 20, 2024

@shogo4405 your PR #1446 fixes the issue in my testing.

Thank you for your patience, diligence, and perseverance in getting this bug fixed. 🙏 It certainly was a tricky one to reproduce.

shogo4405 added a commit that referenced this issue May 21, 2024
@shogo4405 shogo4405 added this to the 1.8.1 milestone May 21, 2024
@shogo4405
Copy link
Owner

Thank you for your confirmation and for providing the reproduction steps. I have merged the revised version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants