GNOME Bugzilla – Bug 762489
rtpjpegdepay may push buffers before setting output caps
Last modified: 2016-06-02 11:08:27 UTC
Created attachment 321893 [details] unit test When rtpjpegdepay starts receiving a stream, it will set the output caps only when the fragment offset is 0, which signifies the beginning of a JPEG image. In case reception starts from a fragment of an image that is not the first fragment (offset=0), then rtpjpegdepay will still collect the rest of the image and push it downstream but without setting the output caps. I have written a unit test (attached) that illustrates the problem. The test ensures that the output of jpegpay will be fragmented by setting mtu to a low value, it drops the first buffer using a pad probe and requires that the plugged decodebin (the decodebin will internally plug rtpjpegdepay) has caps set on the exposed decode pad. When a buffer is pushed before caps, decodebin exposes the pad prematurely and the assertion fails.
Created attachment 321894 [details] [review] rtpjpegdepay: do not push any buffers out until output caps have been set Here is a proposed patch. The idea is to drop the first, incomplete output image and wait for the second one that will also set the output caps.
Comment on attachment 321894 [details] [review] rtpjpegdepay: do not push any buffers out until output caps have been set You can use gst_pad_has_current_caps() instead probably. But shouldn't we also check if frames are actually incomplete, and if so mark them as such (there's a buffer flag for that)
I don't think there's any point in pushing out a JPEG frame that's missing the beginning/header.
True, but missing end of a frame should work fine with most decoders. Or replacing a part of the frame in the middle with zeroes or garbage (are we doing that or do we just drop missing parts in the middle?).
I think this has been fixed independently by commit e20a6877375ea41eb36abd44dc487960c52a88fb Author: Nirbheek Chauhan <nirbheek@centricular.com> Date: Tue Mar 15 03:25:26 2016 +0530 rtpjpegdepay: Don't send invalid frames downstream after packet loss or a DISCONT After clearing the adapter due to a DISCONT, as might happen when some packet(s) have been lost, the depayloader was pushing data into the adapter (which had no header due to the clear), creating a headerless frame out of it, and sending it downstream. The downstream decoder would then usually ignore it; unless there were lots of DISCONTs from the jitterbuffer in which case the decoder would reach its max_errors limit and throw an element error. Now we just discard that data. The test passes. commit 03e2655f7090f743bbf4f71b5ca5b191c4b2d376 Author: Tim-Philipp Müller <tim@centricular.com> Date: Mon Apr 4 17:42:03 2016 +0100 tests: add unit test for jpeg depayloader packet loss handling Make sure it always outputs something that looks like a valid JPEG frame, ie. starts with an SOI marker and ends with an EOI marker. Should probably pick it into 1.8 too.