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

Spelling and player for streaming video over UDP socket #3

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

chubby1968
Copy link

Fixed some spelling and added support for streaming video over socket.

Some solutions might be a little "hacky", e.g. framerate is set to 500 and used to check for new frames often and the actual framerate is used to "catch up" to the live portion of the stream by dropping frames.

The video is also set to start as part of new_udp() since it seemed more natural for streaming videos.

The feature can be tested using ffmpeg with the settings below.

ffmpeg -f lavfi -i mptestsrc -preset ultrafast -vcodec libx264 -tune zerolatency -b 900k -f mpegts udp://127.0.0.1:1234

Audio is untested.

Hopefully it can be of use.

@chubby1968
Copy link
Author

Also added a simple example with the Player.

@n00kii
Copy link
Owner

n00kii commented Oct 13, 2023

thanks for this! ill check it out and try to get it in along with the upcoming subtitle additions

@chubby1968
Copy link
Author

chubby1968 commented Oct 13, 2023

Sounds good. Let me know if you want me to make adjustments.

I couldn't find any way to determine if the stream is up to date, so that's why "catch up to stream" uses timing instead. (I might actually have made a calculation error since it should track ms since last packet which are equal to 1000/fps).

Also some choices are maybe more of a personal preference, e.g. setting FFMPEG log to Quiet. A better alternative is maybe to put that in a separate function.

The above has been removed, so Player::new_udp() is more similar to Player::new().

@n00kii
Copy link
Owner

n00kii commented Oct 27, 2023

sorry for the delay, just got through midterm season

tried out the branch, and it seems to work for the most part though there are some issues im not sure how to resolve due to my inexperience with streaming.

for example, trying to use audio causes panics with an "Unknown error" which i think is ffmpeg::Error::Unknown.

if the stream loops, playback framerate seems to get worse and worse for some reason. perhaps i need to flush the decoders upon loop, but i would need a signal from ffmpeg to do that, assuming ffmpeg gets a packet telling it to loop the stream (but im not sure if thats a thing)

duration doesn't need to be a value if the input if a udp stream, but what should this mean for seeking? we would need to buffer the frames to allow for seeking in the first place, but im not sure if seeking is even a thing that people want to do for udp streams; probably not, but correct me if im wrong.

perhaps there should also be options to set the input packet buffer size as well as the "refresh rate". let me know what you think

src/lib.rs Outdated
@@ -707,10 +705,10 @@ impl Player {
}
}

let is_subtitle_cyclable = self.subtitle_stream_info.1 > 1;
let is_audio_cyclable = self.audio_stream_info.1 > 1;
let is_subtitle_cycleable = self.subtitle_stream_info.1 > 1;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i believe "cyclable" is the correct spelling, atleast per wiktionary

src/lib.rs Outdated
}

/// Create new [`Player`] streaming from socket UDP
pub fn new_udp<T: ToSocketAddrs>(ctx: &egui::Context, socket: T) -> Result<Self> {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can rename this to from_udp (i renamed new_from_bytes to from_bytes)

@chubby1968
Copy link
Author

sorry for the delay, just got through midterm season

tried out the branch, and it seems to work for the most part though there are some issues im not sure how to resolve due to my inexperience with streaming.

for example, trying to use audio causes panics with an "Unknown error" which i think is ffmpeg::Error::Unknown.

I will test it out with audio and see if I can find the error. Maybe the panic is caused if no audio channels is part of the stream.

if the stream loops, playback framerate seems to get worse and worse for some reason. perhaps i need to flush the decoders upon loop, but i would need a signal from ffmpeg to do that, assuming ffmpeg gets a packet telling it to loop the stream (but im not sure if thats a thing)

Not something I've noticed. Is it noticeable after a short period of time (i.e. a couple of minutes)?

duration doesn't need to be a value if the input if a udp stream, but what should this mean for seeking? we would need to buffer the frames to allow for seeking in the first place, but im not sure if seeking is even a thing that people want to do for udp streams; probably not, but correct me if im wrong.

I think seeking can be skipped first since real-time streaming is more important imo. Later seeking could be enabled by buffering (up to some limit) and keeping track of the duration since first frame in buffer (calculated by using the streams fps).

perhaps there should also be options to set the input packet buffer size as well as the "refresh rate". let me know what you think

Both could be a good idea. Would an UDP_option struct with a sensible Default as part of the from_udp() parameters would be the best way? If so should the socket address?

@n00kii
Copy link
Owner

n00kii commented Oct 27, 2023

I will test it out with audio and see if I can find the error. Maybe the panic is caused if no audio channels is part of the stream.

add_audio (and subsequently with_audio) don't panic if no audio stream is detected; and atleast for the files that i tested, they did have an audio stream in the container

Not something I've noticed. Is it noticeable after a short period of time (i.e. a couple of minutes)?

it got worse with each loop for the files that i tested but ill try to test some different files in case it was something specific to those

I think seeking can be skipped first since real-time streaming is more important imo. Later seeking could be enabled by buffering (up to some limit) and keeping track of the duration since first frame in buffer (calculated by using the streams fps).

sounds good

Both could be a good idea. Would an UDP_option struct with a sensible Default as part of the from_udp() parameters would be the best way? If so should the socket address?

id think the socket address should be specified as a the function param of from_udp, since thats where you tell ffmpeg where to look (the same way path is a parameter for new)

i like the options struct idea. i can consolidate all the other player options there too

@chubby1968
Copy link
Author

chubby1968 commented Oct 29, 2023

I changed the approach to use the new function and only change options that are different for udp. Should be much simpler to keep up to date.

Changing some of the options specifically for udp streaming seemed to improve performance. I will investigate further and look at your other comments when I get time.

UDP options are described here: https://www.ffmpeg.org/ffmpeg-protocols.html#udp

Should I just keep the PR open or do you prefer another workflow?

@n00kii
Copy link
Owner

n00kii commented Oct 29, 2023

keeping it open is fine; thanks for all your work

@chubby1968
Copy link
Author

I will test it out with audio and see if I can find the error. Maybe the panic is caused if no audio channels is part of the stream.

add_audio (and subsequently with_audio) don't panic if no audio stream is detected; and atleast for the files that i tested, they did have an audio stream in the container

I think I've tracked down the problem with adding audio.

Panic can be avoided by setting udp option reuse=1, but then another problem occurs as described below.

New Problem

In Streamer::receive_next_packet() the packet is disregarded if the stream index does not match the index for the current Streamer. For streaming this seems to result in packages being thrown away and add_audio (and subsequently with_audio) will keep the stream from starting.

Possible solutions

  1. Change Streamer::receive_next_packet() to send the packet to the correct streamer instead of just disregarding some packets. Whether this could also be beneficial for non-streaming (I think it would mean less calls to receive_next_packet()) I'm not sure, but it would require a restructuring of the current structs that implements Streamer.

  2. Add a struct UDPStreamer that implements Streamer::receive_next_packet() differently from the main player, but the code needed for udp streaming would move further away from model used for file streaming.

What do you think?

@n00kii
Copy link
Owner

n00kii commented Oct 30, 2023

this makes sense. currently, streamers attempt to create their own input contexts so that "throwing away" packets isnt an issue. but for network streaming, they will share the same underlying udp context (unless resuse=1) which causes that problem.

the reason i went with the approach i did (rather than have centralized packet distribution) is that streamers consume packets at different rates at different intervals. there might be a video stream @ 10 frames/second while the audio stream is @ something more continuous and still a subtitle stream @ something more random. this on its own wouldnt be too bad, but in addition, decoders can get "full" with packets, needing to output a frame before accepting the next packet.

ideally the code for file streaming would work the same as network streaming (solution 1). so in order to properly distribute packets between streamers, we would need to

  1. unify the input context across streamers
    • currently, each streamer has its own input context
    • maybe this could take shape in a new PacketManager or InputManager etc struct that has one input context and uses channels to send packets to each streamer
  2. determine the rate at which we should be reading and distributing packets
    • currently, each streamer has a different behavior for the rate at which they read packets:
      • video: at every 1 / frame_rate interval, repeatedly consume packets until a frame can be decoded
      • audio: continuously, repeatedly consume packets until a frame can be decoded UNLESS the audio sample buffer is full or we are ahead of the video stream
      • subtitle: at every 1 / frame_rate interval, repeatedly consume packets until a frame can be decoded UNLESS we are ahead of the video stream
    • maybe there should be a target minimum number of packets in the queue for each stream, and we should continuously consume packets until that minimum is reached for each stream.
  3. determine a method for queuing packets to be decoded for each streamer
    • if channels are used for sending packets to each streamer, we could have a queue housed in the streamer struct for packet consumption. however, a 30 frames/second video does not translate into a 30 packets/second consumption rate
      • we could just consume from the queue all at once at every frame interval to generate a frame (like the current approach)
      • or we could have a frame buffer that consumes packets to a certain target number of frames, then frames are popped from the buffer at the right interval

some of the performance differences between each approach are not immediately obvious to me, so ill need to experiment and perform some benchmarks

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 this pull request may close these issues.

None yet

3 participants