[jitsi-dev] [libjitsi] Fixed handling of empty buffer in VP8 (#93)


#1

You can view, comment on, or merge this pull request online at:

  https://github.com/jitsi/libjitsi/pull/93

-- Commit Summary --

  * Fixed handling of empty buffer

-- File Changes --

    M src/org/jitsi/impl/neomedia/codec/video/vp8/DePacketizer.java (38)

-- Patch Links --

https://github.com/jitsi/libjitsi/pull/93.patch
https://github.com/jitsi/libjitsi/pull/93.diff

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/93


#2

Can one of the admins verify this patch?

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/93#issuecomment-191945319


#3

I don't think we should merge this in its current state. If the copy fails we should return failure and not just set# frameLength (which should follow the total size of #data). We should also ensure that the parameters to arraycopy are correct rather than handling an exception (which is simple enough in this case).

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/93#issuecomment-205495364


#4

there are two ways to fix this.
Method one in fmj:

FMJ BasicFilterModule could set the buffer to discard when the length is ZERO before passing it to the next 'process' method, or not pass it at all to the next process module which could be this one in question.

Method two attempted here in jitsi,
VP8 depacketizor could verify both that the buffer is not set 'discard' and that the buffer actually has 'length' before trying to do anything.

Ignoring the copy/sanity checks we provided, the issue is a length of ZERO for the incoming buffer. Since the VP8 method to probe the buffer does nothing in that case, the copy methods then throw an error and trigger the filter module shut down sequence.

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/93#issuecomment-205688314


#5

So, basically, we should remove the try/catch handler since our issue is handled by the earlier length check? That makes sense.

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/93#issuecomment-205691338


#6

I've also ran into this issue and did a quick fix but eagerly waiting for this solution

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/93#issuecomment-206582123


#7

@bgrozev could you make the changes you are suggesting? You've kinda lost me and I'm not sure I follow. We need this to work and apparently we're not alone.

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/93#issuecomment-206589298


#8

@bgrozev Can you please give this a look - or close the PR if you're not able to merge it.

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/93#issuecomment-232190509