[Bug 796539] Vaapi encoding randomly fails with error: failed to allocate coded buffer

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=796539

--- Comment #1 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
(In reply to Ahmed K from comment #0)

I can submit a patch on request.

Yes, please.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=796539

--- Comment #2 from Ahmed K <***@live.com> ---
Created attachment 372603
--> https://bugzilla.gnome.org/attachment.cgi?id=372603&action=edit
Patch for gstreamer-vaapi-1.10.4

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=796539

--- Comment #3 from Nicolas Dufresne (ndufresne) <***@ndufresne.ca> ---
Review of attachment 372603:
--> (https://bugzilla.gnome.org/review?bug=796539&attachment=372603)

Ok, this isn't the right fix, but clearly a cond_wait without a condition loop
is buggus. g_cond_wait() can unblock for no reason, you must wrap it up in a
look that checks a condition. Is that still like this in 1.14+ ?

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=796539

Víctor Manuel Jáquez Leal <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Summary|Vaapi encoding randomly |vaapiencoder: (baseclass):
|fails with error: failed to |wrap in a while loop
|allocate coded buffer |g_cond_wait() as documented

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=796539

--- Comment #4 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
(In reply to Nicolas Dufresne (ndufresne) from comment #3)

Post by "GStreamer" (GNOME Bugzilla)
Ok, this isn't the right fix, but clearly a cond_wait without a condition
loop is buggus. g_cond_wait() can unblock for no reason, you must wrap it up
in a look that checks a condition. Is that still like this in 1.14+ ?

That code hasn't changed since. Thus, your patch looks promising. Would be
great if you do it for master and following the contributing rules:

https://gstreamer.freedesktop.org/documentation/contribute/index.html#patch-format

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=796539

--- Comment #5 from Ahmed K <***@live.com> ---
Okay, I will upload the right patch. Thanks!

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=796539

--- Comment #6 from Ahmed K <***@live.com> ---
Created attachment 372663
--> https://bugzilla.gnome.org/attachment.cgi?id=372663&action=edit
Patch for gstreamer-vaapi master branch

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=796539

Víctor Manuel Jáquez Leal <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #372663|none |needs-work
status| |

--- Comment #7 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
Review of attachment 372663:
--> (https://bugzilla.gnome.org/review?bug=796539&attachment=372663)

thanks a lot for both patches.

Is it possible to merge only one in both branches? to avoid, as possible, the
diversion.

One more thing, your patches don't follow the gstreamer code stylte:
https://gstreamer.freedesktop.org/documentation/frequently-asked-questions/developing.html#what-is-the-coding-style-for-gstreamer-code

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=796539

Víctor Manuel Jáquez Leal <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #372603|0 |1
is obsolete| |

--- Comment #8 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
Comment on attachment 372603
--> https://bugzilla.gnome.org/attachment.cgi?id=372603
Patch for gstreamer-vaapi-1.10.4

I assume this patch is obsolete, right?

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=796539

--- Comment #9 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
Review of attachment 372663:
--> (https://bugzilla.gnome.org/review?bug=796539&attachment=372663)

::: gst-libs/gst/vaapi/gstvaapiencoder.c
@@ +438,3 @@
+ g_cond_wait (&encoder->codedbuf_free, &encoder->mutex);
+ codedbuf_proxy = gst_vaapi_coded_buffer_proxy_new_from_pool (pool);
+ if (codedbuf_proxy)

I don't think codedbuf_proxy can be the flag to check if the cond signaled
falsely. We need an extra flag in _coded_buffer_proxy_released_notify() to
check in gst_vaapi_encoder_create_coded_buffer()

Though, that is not clean. We would need a new API in GstVaapiVideoPool to do
an async pop in its queue.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=796539

--- Comment #10 from Ahmed K <***@live.com> ---
(In reply to Víctor Manuel Jáquez Leal from comment #8)

Comment on attachment 372603 [details] [review]
Patch for gstreamer-vaapi-1.10.4
I assume this patch is obsolete, right?

Yes, the old patch is obsolete now.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=796539

GStreamer system administrator <***@gstreamer.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |OBSOLETE

--- Comment #11 from GStreamer system administrator <***@gstreamer.net> ---
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been
closed from further activity.

You can subscribe and participate further through the new bug through this link
to our GitLab instance:
https://gitlab.freedesktop.org/gstreamer/gstreamer-vaapi/issues/99.