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 793236 - test:msdkh264enc: get a fail by using attach unit test patch.
test:msdkh264enc: get a fail by using attach unit test patch.
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.13.91
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 789886
 
 
Reported: 2018-02-07 08:30 UTC by Fei
Modified: 2018-03-09 19:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
unit test patch for msdkh264enc. (5.42 KB, patch)
2018-02-07 08:30 UTC, Fei
none Details | Review
fix msdkh264enc gst push pad fail. (939 bytes, patch)
2018-02-09 05:52 UTC, Fei
none Details | Review
msdk: enc: fix missing some frames to be encoded (4.45 KB, patch)
2018-02-22 02:26 UTC, Hyunjun Ko
none Details | Review
enc: fix missing some frames to be encoded (4.78 KB, patch)
2018-03-07 10:48 UTC, Hyunjun Ko
committed Details | Review
msdkh264 encode unit test. (6.10 KB, patch)
2018-03-09 05:39 UTC, Fei
committed Details | Review

Description Fei 2018-02-07 08:30:05 UTC
Created attachment 367976 [details] [review]
unit test patch for msdkh264enc.

1. apply the attach patch to gst-plugins-bad master branch.
2. ./autogen
3. make check

Fail log:
FAIL: elements/msdkh264enc
==========================

Running suite(s): msdkh264enc
0:00:00.143456364 ^[[334m12537^[[00m      0x1f15270 ^[[36mINFO   ^[[00m ^[[00m             msdkenc msdk.c:344:msdk_open_session:^[[00m MSDK implementation: 0x0001 (SOFTWARE)
0:00:00.143490253 ^[[334m12537^[[00m      0x1f15270 ^[[36mINFO   ^[[00m ^[[00m             msdkenc msdk.c:345:msdk_open_session:^[[00m MSDK version: 1.23
0%: Checks: 1, Failures: 1, Errors: 0
elements/msdkh264enc.c:104:F:general:msdk_h264enc:0: Assertion 'gst_pad_push (srcpad, gst_buffer_ref (buffer)) == GST_FLOW_OK' failed
Check suite msdkh264enc ran in 0.056s (tests failed: 1)
Comment 1 Víctor Manuel Jáquez Leal 2018-02-07 08:41:55 UTC
Hi Fei,

I don't follow the purpose of this bug

1. To merge this unit test? 
2. To fix the test? 
3. Or to fix the encoder?
Comment 2 Fei 2018-02-07 08:56:06 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #1)
> Hi Fei,
> 
> I don't follow the purpose of this bug
> 
> 1. To merge this unit test? 
> 2. To fix the test? 
> 3. Or to fix the encoder?

@Victor, I wish this unit test could be merged first. And also, we need this bug to track the fail of encoder by this unit test.
Comment 3 Fei 2018-02-09 05:52:54 UTC
Created attachment 368172 [details] [review]
fix msdkh264enc gst push pad fail.
Comment 4 Fei 2018-02-09 05:54:40 UTC
With this patch, gst push pad success. But another fail:
elements/msdkh264enc.c:112:F:general:msdk_h264enc:0: 'g_list_length (buffers)' (9) is not equal to '10' (10)
Comment 5 Hyunjun Ko 2018-02-12 07:53:14 UTC
Confirmed, Fei.

Found a cause and preparing a patch but I need to wait until patches on #bug 790752 is fished.
Comment 6 Hyunjun Ko 2018-02-22 02:26:44 UTC
Created attachment 368738 [details] [review]
msdk: enc: fix missing some frames to be encoded

There was not handling the end of encoding sequence in encoder.
This patch does drain any remaining internal streams while decoder already does this.

Document says:
"To mark the end of the encoding sequence, call this function with a NULL surface
pointer. Repeat the call to drain any remaining internally cached bitstreams—one
frame at a time—until MFX_ERR_MORE_DATA is returned."
Comment 7 Hyunjun Ko 2018-02-22 03:43:25 UTC
Note that this patch is based on patches in bug #793413
Comment 8 Víctor Manuel Jáquez Leal 2018-02-26 20:56:21 UTC
Review of attachment 368738 [details] [review]:

::: sys/msdk/gstmsdkenc.c
@@ +793,3 @@
+  for (;;) {
+    task = thiz->tasks + thiz->next_task;
+    gst_msdkenc_finish_frame (thiz, task, FALSE);

what if gst_msdkenc_finish_frame() fails?
Comment 9 Hyunjun Ko 2018-03-01 08:21:45 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #8)
> Review of attachment 368738 [details] [review] [review]:
> 
> ::: sys/msdk/gstmsdkenc.c
> @@ +793,3 @@
> +  for (;;) {
> +    task = thiz->tasks + thiz->next_task;
> +    gst_msdkenc_finish_frame (thiz, task, FALSE);
> 
> what if gst_msdkenc_finish_frame() fails?

That is like gst_msdkdec_drain in decoder.
There is a message saying "failed to finish the task %p, but keep draining for the remaining frames".

Do you want to put this message also here?
Comment 10 Hyunjun Ko 2018-03-07 10:48:39 UTC
Created attachment 369405 [details] [review]
enc: fix missing some frames to be encoded

There was not handling the end of encoding sequence in encoder.
This patch does drain any remaining internal streams while decoder already does this.

Document says:
"To mark the end of the encoding sequence, call this function with a NULL surface
pointer. Repeat the call to drain any remaining internally cached bitstreams—one
frame at a time—until MFX_ERR_MORE_DATA is returned."

-------------------

rebased.
Comment 11 sreerenj 2018-03-08 02:27:56 UTC
@Fei: Please squash the patches. Also, add the support in meson.build
Comment 12 sreerenj 2018-03-08 19:46:38 UTC
Review of attachment 369405 [details] [review]:

Pushed as commit 491843085817ba9c5bc98f8d4087a1cc7ab4fad6
Comment 13 Fei 2018-03-09 05:39:22 UTC
Created attachment 369478 [details] [review]
msdkh264 encode unit test.

@Sree, squashed, and added the test unit to meson.build. Please have a review.
Comment 14 sreerenj 2018-03-09 19:43:21 UTC
Review of attachment 369478 [details] [review]:

Pushed as commit 1192a598eee41395f632cfebd2d8fb4bec08af7f