ralfbiedert / openh264-rs

Idiomatic Rust wrappers around OpenH264.
71 stars 35 forks source link

stream based NAL parser #60

Closed mixaal closed 2 months ago

mixaal commented 2 months ago

This might be more a question on the API rather than a bug.

I have a code that receives bytes from the UDP network socket. In this case the NAL mark (0,0,1) can be spread across the recv buffer, like in this example: https://github.com/mixaal/rust-sdl-ui/blob/main/src/video.rs#L165-L166

Not sure how can I use the nal_units() method in this case. When I used:


  loop {
            let r = self.video_conn.recv(&mut buff);
            if r.is_err() {
                tracing::warn!(method_name, "udp read error: {}", r.unwrap_err());
                continue;
            }

            let nread = r.unwrap();
            if nread<2 {
                continue;
            }

            let recv_buffer = buff[2..nread].to_vec();
            for packet in nal_units(&recv_buffer) {
                 // Do some stuff here to decode the image
            }
   }

I typically threw away frames that were on the recv_buffer boundary. However it took 10-20 frames till I got next valid frame. This way most packets were skipped. Also this issue becomes worse when the receive buffer is small (less then 8-16K). With buffer like 2K it makes frame decoding impossible.

I implemented a solution to this problem: https://github.com/mixaal/rust-sdl-ui/blob/main/src/video.rs The main idea is that NalParser tries to search for NAL mark, if not found asks for more data, if found demands next call to get the packet. In case no more NAL marks are found in the current buffer, it asks for more data. In case it finds next NAL mark and has last NAL mark it returns the image packet for processing.

In case what I described is possible with the current API, please disregard this request.

Thanks, Michal

ralfbiedert commented 2 months ago

This sounds useful and I'd be happy to accept a PR for that.

mixaal commented 2 months ago

@ralfbiedert : Ok, so I assume we put this one into openh264/src/utils.rs ? Sounds good?

ralfbiedert commented 2 months ago

Yes, that would be a good place, thanks!

mixaal commented 2 months ago

Honestly, in the end I added stream.rs since it looked better when using the classes.

mixaal commented 2 months ago

I think we can close this one now.