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

Jitterbuffer overflow causing up to 3000 lost frame packets #26

Closed
mholt-dv opened this issue Jun 18, 2018 · 8 comments
Closed

Jitterbuffer overflow causing up to 3000 lost frame packets #26

mholt-dv opened this issue Jun 18, 2018 · 8 comments
Labels
bug Something isn't working question Further information is requested

Comments

@mholt-dv
Copy link

Hi

I'm testing a webrtc to tensorflow process which uses aiortc version 0.5.0 to connect to a kurento media server. I've discovered what looks like a bug in the RTCRtpReceiver and JitterBuffer when video processing. If a frame consists of more frames that the jitter buffer capacity the process discards further packets until MAX_DROPOUT is reached. Then starts processing again.

I have created a local fix to this by

  1. Adding a resize method to the jiffer buffer
  2. When a max is reached just processing the frame contents anyhow.

In testing I discovered that max frame size I was seeing was 85 packets,

My code changes are as follows:

In rtcrtpreceiver.py:

I've changed:

line 100 in the released file (of async def _handle_rtp_packet(self, packet))

if (got_frame: self._jitter_buffer.remove(count) for video_frame in decoder.decode(*payloads): await self._track._queue.put(video_frame)

to:

if (got_frame or (count == self._jitter_buffer.capacity-1 and self._jitter_buffer.fixed())): self._jitter_buffer.remove(count) for video_frame in decoder.decode(*payloads): await self._track._queue.put(video_frame)

I have also added the following methods so jitterbuffer.py:

`def fixed(self):
return self._capacity >= MAX_CAPACITY

def __resize(self, capacity):
    if (capacity>self._capacity):
        frames = [None for i in range(capacity)]
        head = self._head
        for i in range(self._capacity):
            frames[i]=self._frames[head]
            head = (head + 1) % self._capacity
        self._capacity=capacity
        self._frames=frames  
        self._head=0`

and changed line 36 (of def add(self, payload, sequence_number, timestamp) from:

if delta >= self._capacity: if delta > MAX_DROPOUT: self.__reset() self._origin = sequence_number delta = 0 else: return

to

if delta >= self._capacity: if not self.fixed() and delta < (self._capacity + self._capacity/2): self.__resize(self._capacity*2) elif delta > MAX_DROPOUT: self.__reset() self._origin = sequence_number delta = 0 else: return

I currently have MAX_CAPACITY set to 256.

This seems to fix the problem.

@jlaine
Copy link
Collaborator

jlaine commented Jun 18, 2018

Hi, the changes are pretty hard to read could you possibly provide a diff against master?

@mholt-dv
Copy link
Author

Sure - I've just looked at the comment - seems like the markup didn't keep my formatting.

@jlaine
Copy link
Collaborator

jlaine commented Jun 18, 2018

Also, for my understanding how many packets does a single frame span at most? What kind of resolution are we talking about?

FYI the jitter buffer implementation draws inspiration from pjsip

@mholt-dv
Copy link
Author

rtcrtpreceiver-fix+diff.zip

Attached is a zip of the changed files and the diff from the 0.5.0 tag. Which I assume the release i installed cam from.

For your info the max packet packet count on a 10/20 min run from a 1280x720 camera feed varied between around 40 and 84 packets per frame.

@jlaine
Copy link
Collaborator

jlaine commented Jun 18, 2018

OK thanks, those are some interesting metrics. For these kind of videos a capacity of 32 is clearly insufficient as we won't even accommodate a full frame.

I'm not sure I'm entirely comfortable with the dynamic resizing as it introduces some additional state, and unless I'm mistaken we never shrink the buffer.

So that we have a common basis to discuss I've applied a slightly reformatted version of your patch on this branch:

https://github.com/jlaine/aiortc/tree/jitterbuffer-mholt-dv

@jlaine
Copy link
Collaborator

jlaine commented Jun 23, 2018

@mholt-dv have you had a chance to look at the comments I posted on the code?

@jlaine jlaine added bug Something isn't working question Further information is requested labels Jun 27, 2018
@jlaine
Copy link
Collaborator

jlaine commented Jul 18, 2018

Without feedback on this issue I'll be closing this issue soon.

@jlaine
Copy link
Collaborator

jlaine commented Feb 19, 2021

Should be fixed by 42f4e0f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants