GNOME Bugzilla – Bug 791258
amcvideoenc: when flushing, better handle IllegalStateException received from getOutputBuffer
Last modified: 2017-12-06 14:44:00 UTC
I have been stress testing amcvideoenc in a pipeline which was being repeatedly built up and torn down by GstRTSPServer. I found that sometimes flushing did not complete as nicely as it should do, with the following in the logs: 11-04 03:21:41.153 1859 2691 I GStreamer+rtspstream: 0:05:26.696659656 0xb88998c0 rtsp-stream.c:3318:update_transport removing 10.0.0.4:42288-42289 11-04 03:21:41.160 1859 2714 D GStreamer+amcvideoenc: 0:05:26.703632531 0xb88cc290 gstamcvideoenc.c:1513:gst_amc_video_enc_handle_frame:<amcvideoenc-omxrkvideoencoderavc17> Queueing buffer 1: size 622080 time 4201972 flags 0x00000000 11-04 03:21:41.168 1859 2691 I GStreamer+rtspstream: 0:05:26.711672614 0xb88998c0 rtsp-stream.c:1763:caps_notify stream 0xb88a3198 received caps 0x0, (NULL) 11-04 03:21:41.175 1859 2717 D GStreamer+amcvideoenc: 0:05:26.717868198 0xb88fd150 gstamcvideoenc.c:1017:gst_amc_video_enc_loop:<amcvideoenc-omxrkvideoencoderavc17> Got output buffer at index 0: size 19653 time 4201972 flags 0x00000000 11-04 03:21:41.176 1859 2717 W GStreamer+amcvideoenc: 0:05:26.719513198 0xb88fd150 gstamcvideoenc.c:1147:gst_amc_video_enc_loop:<amcvideoenc-omxrkvideoencoderavc17> error: Failed to call Java method: java.lang.IllegalStateException 11-04 03:21:41.176 1859 2717 W GStreamer+amcvideoenc: java.lang.IllegalStateException 11-04 03:21:41.176 1859 2717 W GStreamer+amcvideoenc: at android.media.MediaCodec.getBuffer(Native Method) 11-04 03:21:41.176 1859 2717 W GStreamer+amcvideoenc: at android.media.MediaCodec.getOutputBuffer(MediaCodec.java:2886) 11-04 03:21:41.176 1859 2717 W GStreamer+amcvideoenc: 11-04 03:21:41.181 1859 2691 D GStreamer+amcvideoenc: 0:05:26.724001656 0xb88998c0 gstamcvideoenc.c:1197:gst_amc_video_enc_stop:<amcvideoenc-omxrkvideoencoderavc17> Stopping encoder 11-04 03:21:41.201 1859 2691 D GStreamer+amcvideoenc: 0:05:26.744296406 0xb88998c0 gstamcvideoenc.c:1225:gst_amc_video_enc_stop:<amcvideoenc-omxrkvideoencoderavc17> Stopped encoder 11-04 03:21:41.205 1859 2691 D GStreamer+amcvideoenc: 0:05:26.748330739 0xb88998c0 gstamcvideoenc.c:597:gst_amc_video_enc_close:<amcvideoenc-omxrkvideoencoderavc17> Closing encoder 11-04 03:21:41.211 1859 2691 D GStreamer+amcvideoenc: 0:05:26.754210448 0xb88998c0 gstamcvideoenc.c:613:gst_amc_video_enc_close:<amcvideoenc-omxrkvideoencoderavc17> Closed encoder In this case, the pipeline shut down, however, the encoder's being fed by v4l2src, one of the v4l2src buffers was not released from amcvideoenc, and starting up the pipeline the next time round fails, as the v4l2bufferpool did not properly close down the V4L2 device from before. Patch for the issue attached.
Created attachment 365022 [details] [review] Patch
Review of attachment 365022 [details] [review]: Thanks for your patch. The same is probably also necessary in the video decoder and the audio elements? Can you check and provide patches? ::: sys/androidmedia/gstamcvideoenc.c @@ +1021,3 @@ + if (self->flushing) { + GST_ELEMENT_ERROR_FROM_ERROR (self, err); + GST_ERROR_OBJECT (self, "Error encountered, but flushing anyway"); You don't want to send an error message here probably. It's flushing, not GST_FLOW_ERROR. Error messages are supposed to be fatal @@ +1026,3 @@ + goto failed_to_get_output_buffer; + } else if (!buf) + goto got_null_output_buffer; Please add some {} here
(In reply to Sebastian Dröge (slomo) from comment #2) > You don't want to send an error message here probably. It's flushing, not > GST_FLOW_ERROR. Error messages are supposed to be fatal I think there are two options: a) change the GST_ERROR_OBJECT to a GST_WARNING_OBJECT, reporting that the problem has occurred; b) remove the GST_ELEMENT_ERROR_FROM_ERROR/GST_ERROR_OBJECT and just do a g_clear_error, hiding the problem; It looks like b) may be more consistent with what other code is already doing when flushing and an error has been picked up. What do you think?
I agree, b)
Created attachment 365055 [details] [review] Patch, v2 Here's a revised version. I don't have a test case available which triggers the same Java exception that I've seen during encoding for playback, but I've performed the same changes for the affected code in the decoder elements.
Review of attachment 365055 [details] [review]: Looks good to me, I'll merge later. Thanks!
commit 236398ee3f603cfee05a026582ecbc0beda516ea (HEAD -> master) Author: Ursula Maplehurst <ursula@kangatronix.co.uk> Date: Fri Dec 1 13:02:12 2017 +0000 androidmedia: when flushing, better handle IllegalStateException received from getOutputBuffer 1. Similar to 880f3d8, don't consider not getting an output buffer as an error during flushing. I've seen the following sometimes when encoding: W GStreamer+amcvideoenc: java.lang.IllegalStateException W GStreamer+amcvideoenc: at android.media.MediaCodec.getBuffer(Native Method) W GStreamer+amcvideoenc: at android.media.MediaCodec.getOutputBuffer(MediaCodec.java:2886) 2. For amcvideodec/enc, call _find_nearest_frame (which grabs a fresh reference on a GstVideoCodecFrame) after we have an output buffer, so as to not leak the reference, in case getting an output buffer fails. Otherwise, if we get an error grabbing the output buffer, we leak the reference to the frame. This can cause issues with a v4l2bufferpool feeding the encoder not being able to clean itself up properly due to buffers still being marked as in-use. https://bugzilla.gnome.org/show_bug.cgi?id=791258