GNOME Bugzilla – Bug 766265
opusdec with FEC breaks when packet sizes change
Last modified: 2016-05-20 14:59:36 UTC
Created attachment 327633 [details] preconstructed rtp packets that break decoder when frame sizes fluctuate, the gstreamer opus decoder breaks. I think it fails to handle the buffer too small error in FEC mode: running the following breaks the stream: gst-launch-1.0 multifilesrc do-timestamp=true location=./c%04d.rtp caps=application/x-rtp ! identity datarate=10000 ! rtpjitterbuffer do-lost=true ! rtpopusdepay ! opusparse ! opusdec plc=true use-inband-fec=true ! autoaudiosink while this works (FEC off): gst-launch-1.0 multifilesrc do-timestamp=true location=./c%04d.rtp caps=application/x-rtp ! identity datarate=10000 ! rtpjitterbuffer do-lost=true ! rtpopusdepay ! opusparse ! opusdec ! autoaudiosink Also works for reference: gst-launch-1.0 multifilesrc do-timestamp=true location=./c%04d.rtp caps=application/x-rtp ! identity datarate=10000 ! rtpjitterbuffer do-lost=true ! rtpopusdepay ! matroskamux ! filesink location=out.mkv ffplay out.mkv
This kind of usecase makes it really hard for the packet loss concealer to guess the size of the next packet.. You should really avoid fluctuating the number of frames per packet like that. But there seems to indeed be some kind of actual bug. I've been testing with the rtpjitterbuffer patch from bug #: gst-launch-1.0 multifilesrc location=./c%04d.rtp caps='application/x-rtp, payload=111, clock-rate=48000' ! rtpjitterbuffer do-lost=false ! udpsink port=8888 sync=1 gst-launch-1.0 udpsrc port=8888 ! application/x-rtp, payload=111, media-type=audio, encoding-name=OPUS, clock-rate=48000 ! rtpjitterbuffer do-lost=true ! rtpopusdepay ! opusparse ! opusdec plc=true use-inband-fec=1 ! pulsesink The patch I'm attaching here just prevents it from stopping, the jitterbuffer seems to still produce lost packet events with odd durations.
You need the rtpjitterbuffer patch from bug #9835479 to run the sender test pipoeline.
Created attachment 327879 [details] [review] opusdec: Use GST_AUDIO_DECODER_ERROR This way, the first invalid stream won't break all decoding.
Review of attachment 327879 [details] [review]: Makes sense ::: ext/opus/gstopusdec.c @@ +559,3 @@ gst_buffer_unref (outbuf); + GST_AUDIO_DECODER_ERROR (dec, 1, STREAM, DECODE, ("Error decoding stream"), + ("Decoding error (%d): %s", n, opus_strerror (n)), hret); hret? You probably mean ret :)
Created attachment 328215 [details] [review] opusdec: Use GST_AUDIO_DECODER_ERROR This way, the first invalid stream won't break all decoding.
Created attachment 328216 [details] [review] opus: Post error message on GST_FLOW_ERROR
Merged into master: commit 0bfed26f5685560f33c7e1a8759619e53d27a330 Author: Olivier Crête <olivier.crete@collabora.com> Date: Thu May 19 12:26:05 2016 -0400 opus: Post error message on GST_FLOW_ERROR https://bugzilla.gnome.org/show_bug.cgi?id=766265 commit 1a700c3ae671694c206bed7939b8462db652a9b1 Author: Olivier Crête <olivier.crete@collabora.com> Date: Sat May 14 14:41:28 2016 +0200 opusdec: Use GST_AUDIO_DECODER_ERROR This way, the first invalid stream won't break all decoding. https://bugzilla.gnome.org/show_bug.cgi?id=766265 And into 1.8: commit ac8ed8746d683d2d1696baacdc2ac6d7b21af31e Author: Olivier Crête <olivier.crete@collabora.com> Date: Sat May 14 14:41:28 2016 +0200 opusdec: Use GST_AUDIO_DECODER_ERROR This way, the first invalid stream won't break all decoding. https://bugzilla.gnome.org/show_bug.cgi?id=766265