[Bug 762509] vaapidecoder: h264: decoder stores too many pictures in the DPB before output

[GStreamer (GNOME Bugzilla)]

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

Víctor Manuel Jáquez Leal <vjaquez at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #352653|none                        |reviewed
             status|                            |

--- Comment #28 from Víctor Manuel Jáquez Leal <vjaquez at igalia.com> ---
Review of attachment 352653:
 --> (https://bugzilla.gnome.org/review?bug=762509&attachment=352653)

Regardless Sree's comment about the spec problem this patch might bring, let me
comment the patch itself:

1. You included in the patch a change to the common submodule:
https://bugzilla.gnome.org/attachment.cgi?id=352653&action=diff#a/common_sec1
   :) I guess that's no needed in you patch.

::: gst-libs/gst/vaapi/gstvaapidecoder_h264.c
@@ +838,3 @@
+        found_picture = pic;
+        found_index = i;
+        found_poc = pic->base.poc;

thanks for removing this comma operator. It always made me wary.

@@ +1692,3 @@
+                  &can_output)) >= 0) && can_output) {
+    dpb_output (decoder, priv->dpb[found_index]);
+  }

IMO this code is a bit complex. I would rewrite it like this:

while (TRUE) {
  found_index = dpb_find_lowest_poc_for_output (decoder, priv->current_picture,
NULL, &can_output);
  if (found_output < 0 || !can_output)
    break;
  dpb_output (decoder, priv->dpb[found_index])
}

@@ +1727,3 @@
+  GST_DEBUG ("added %d to dpb", picture->base.poc);
+  /* TODO: only call the following if we're in optional
+     'low-latency' mode? */

we would need to add a property for all the decoders when it only would make
sense for H264, and that's confusing.

Another option would be, when registering the element, the property will be
added if the encoder is H264.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.