Bugfix: prevent borked generated audio file if meetecho header is sent with no RTP data next

[SamyCookie]

For some reason I ignore, Janus server created a MJR file, with MEET..B?.T (4D 45 45 54 00 05 42 D5 00 54) for final data, aka a MEETECHO header with an expected of 84 bytes RTP packet data next. But this RTP packet data has not been written.

Consequences: postprocessing fails to generate a correct audio file, and some random extra silence audio data is badly inserted.

The current patch prevent this behaviour to happen by stopping MJR parsing if no data is reachable next.

[lminiero]

Thanks! Added a couple of notes inline. I see you signed the CLA already, so for me this can be merged as soon as the notes are addressed.

[atoppi]

Hi @SamyCookie ,
thanks for the contribution!
Can you add the same checks for data frames too?

[SamyCookie]

@atoppi done too !

[atoppi]

lgtm 👍

[lminiero]

Thanks for the quick fixes, merging then!

[SamyCookie]

@lminiero Should I write a ticket to reference the original problem ? (the fact that the server has written a MEETECHO header with no data next)

[lminiero]

I don't think there's any issue to address: it might be that a recording was closed while writing the last packet. As long as the postprocessor deals with it properly I think we're fine. As a side node, Janus doesn't write MEETECHO as a per-packet header anymore, but only MEET, as the other 4 bytes are used to store a partial timestamp of when we received the packet.

[SamyCookie]

I don't think there's any issue to address: it might be that a recording was closed while writing the last packet. As long as the postprocessor deals with it properly I think we're fine.

OK, but IMHO, this behaviour is unconsistant and should be fixed server side one day.

As a side node, Janus doesn't write MEETECHO as a per-packet header anymore, but only MEET, as the other 4 bytes are used to store a partial timestamp of when we received the packet.

You are right and it is a typo from me cause I already saw this in janus-pp-rec. Also, should the last Meetecho Janus Recordings specification here be updated to last version (aka MJR00002) ?

[SamyCookie]

And of course, thanks for your time and merging this !

[lminiero]

Good catch, I reflected that change in some slides recently (e.g., see slides 49-50 here, but forgot to update the docs. I'll do that shortly.