[Bug 732265] vaapidecode: h264: add support for decoding MVC base views only

[GStreamer (GNOME Bugzilla)]

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

--- Comment #22 from Orestis Floros <orestisf1993 at gmail.com> ---
(In reply to Víctor Manuel Jáquez Leal from comment #21)
> (In reply to Orestis Floros from comment #13)
> > I tried to follow submit patches in a way similar to bug 783588. I can also
> > squash them if this is too much.
>
> Squash them, please :)

Ok, I'll leave them like this until everything is good and then squash them.

(In reply to Víctor Manuel Jáquez Leal from comment #14)

> Review of attachment 356376 [details] [review]:
>
> ::: gst/vaapi/gstvaapidecode_props.c
> @@ +99,3 @@
> +  g_object_class_install_property (klass,
> GST_VAAPI_DECODER_H264_PROP_BASE_ONLY,
> +      g_param_spec_boolean ("base-only", "Decode base view only",
> +          "Only decode the base view of the stream: Any NAL units that do
> not pertain to the Annex.A set will be dropped",
>
> I would maker shorter this description.
>
> Perhaps just to: "Drop any NAL unit not defined in Annex.A"

Ok.

(In reply to Víctor Manuel Jáquez Leal from comment #15)
> Review of attachment 356377 [details] [review]:
>
> ::: gst-libs/gst/vaapi/gstvaapidecoder_h264.c
> @@ +4805,3 @@
> + *
> + * if @base_only is %TRUE only the base view of MVC encoded streams
> + * is decoded.
>
> is this property going to be only for MVC streams? not for SVC ones?
>
> AFAIU this property will be drop both NAL

I thought that I should not mention SVC until I am ready to submit patches in
bug 732266. Right now it doesn't affect SVC streams.

(In reply to Víctor Manuel Jáquez Leal from comment #18)
> Review of attachment 356380 [details] [review]:
>
> ::: gst/vaapi/gstvaapidecode_props.c
> @@ +115,3 @@
> +
> +gboolean
> +gst_vaapi_decode_get_base_only (GObject * object)
>
> from my point of view this function makes little sense. You can add in
> gstvaapidecode.c something like this:
>
> if (g_object_class_find_property(G_OBJECT_GET_CLASS(decode),"base-only")) {
>   g_object_get (decode, "base-only", &base_only, NULL)
> }

Ok. But:

(In reply to Víctor Manuel Jáquez Leal from comment #19)
> Review of attachment 356381 [details] [review]:
>
> ::: gst/vaapi/gstvaapidecode.c
> @@ +1169,3 @@
> +  caps_new = gst_caps_from_string ("video/x-h264");
> +  structure = gst_caps_get_structure (caps_new, 0);
> +  gst_structure_set (structure, "profile", G_TYPE_STRING, profile_name,
> NULL);
>
> Use this
>
> caps_new = gst_caps_new_simple ("video/x-h264", "profile", profile_name,
> NULL);
>
> @@ +1222,3 @@
> +  }
> +
> +  if (base_only && !have_mvc && have_high) {
>
> If va backend doesn't profile mvc profiles but has high profile, I would set
> base_only as TRUE mandatory.

Ok. This means that gstvaapidecode doesn't need to get the value of base_only
since it won't use it anywhere. It need to set base_only accordingly though.

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