After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 791258 - amcvideoenc: when flushing, better handle IllegalStateException received from getOutputBuffer
amcvideoenc: when flushing, better handle IllegalStateException received from...
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
Other Linux
: Normal normal
: 1.12.4
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Reported: 2017-12-05 12:27 UTC by Ursula Maplehurst
Modified: 2017-12-06 14:44 UTC
See Also:
GNOME target: ---
GNOME version: ---

Patch (2.39 KB, patch)
2017-12-05 12:29 UTC, Ursula Maplehurst
none Details | Review
Patch, v2 (4.24 KB, patch)
2017-12-05 18:38 UTC, Ursula Maplehurst
committed Details | Review

Description Ursula Maplehurst 2017-12-05 12:27:52 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
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 Method)
11-04 03:21:41.176  1859  2717 W GStreamer+amcvideoenc:         at
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.
Comment 1 Ursula Maplehurst 2017-12-05 12:29:48 UTC
Created attachment 365022 [details] [review]
Comment 2 Sebastian Dröge (slomo) 2017-12-05 14:02:23 UTC
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
Comment 3 Ursula Maplehurst 2017-12-05 14:35:14 UTC
(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?
Comment 4 Sebastian Dröge (slomo) 2017-12-05 16:15:53 UTC
I agree, b)
Comment 5 Ursula Maplehurst 2017-12-05 18:38:41 UTC
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.
Comment 6 Sebastian Dröge (slomo) 2017-12-05 19:09:10 UTC
Review of attachment 365055 [details] [review]:

Looks good to me, I'll merge later. Thanks!
Comment 7 Sebastian Dröge (slomo) 2017-12-06 08:32:49 UTC
commit 236398ee3f603cfee05a026582ecbc0beda516ea (HEAD -> master)
Author: Ursula Maplehurst <>
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
       W GStreamer+amcvideoenc: java.lang.IllegalStateException
       W GStreamer+amcvideoenc:         at Method)
       W GStreamer+amcvideoenc:         at
    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
       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.