Fixed handling of empty buffer in VP8

[mondain]

[jitsi-jenkins]

Can one of the admins verify this patch?

[bgrozev]

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).

[Andy--S]

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.

[Andy--S]

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

[zbettenbuk]

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

[mondain]

@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.

[ibauersachs]

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

[bgrozev]

@ibauersachs, the current code has problems, and I don't want to merge it in this state. I don't have the resources to fix it and test it myself, but I would rather leave it open because it is a valid problem.