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 722761 - Add VP8 decoder to gstreamer-vaapi
Add VP8 decoder to gstreamer-vaapi
Status: RESOLVED FIXED
Product: gstreamer-vaapi
Classification: Other
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: gstreamer-vaapi maintainer(s)
gstreamer-vaapi maintainer(s)
Depends on: 722760
Blocks: 720305
 
 
Reported: 2014-01-22 10:04 UTC by Zhao, Halley
Modified: 2014-04-19 08:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
0001-vaapidecode-add-vp8-support-in-configure.ac-and-Make (4.61 KB, patch)
2014-01-22 10:05 UTC, Zhao, Halley
committed Details | Review
0002-vaapidecode-add-vp8-support-in-common-files (9.17 KB, patch)
2014-01-22 10:05 UTC, Zhao, Halley
committed Details | Review
0003-vaapidecode-add-vp8-decoder-files (28.60 KB, patch)
2014-01-22 10:05 UTC, Zhao, Halley
needs-work Details | Review

Description Zhao, Halley 2014-01-22 10:04:57 UTC
Add VP8 decoder to gstreamer-vaapi
Comment 1 Zhao, Halley 2014-01-22 10:05:14 UTC
Created attachment 266963 [details] [review]
0001-vaapidecode-add-vp8-support-in-configure.ac-and-Make
Comment 2 Zhao, Halley 2014-01-22 10:05:30 UTC
Created attachment 266964 [details] [review]
0002-vaapidecode-add-vp8-support-in-common-files
Comment 3 Zhao, Halley 2014-01-22 10:05:49 UTC
Created attachment 266965 [details] [review]
0003-vaapidecode-add-vp8-decoder-files
Comment 4 Gwenole Beauchesne 2014-04-19 05:55:10 UTC
Review of attachment 266963 [details] [review]:

LGTM
Comment 5 Gwenole Beauchesne 2014-04-19 05:55:24 UTC
Review of attachment 266964 [details] [review]:

LGTM
Comment 6 Gwenole Beauchesne 2014-04-19 08:26:32 UTC
Review of attachment 266965 [details] [review]:

Tracking persistent frame data was better to be pushed to parser side, and not handle half of it in the decoder. This fixed a couple of more issues (clips). Resolutions could easily be tracked too, instead of relying on the very first one from GstCaps.

::: gst-libs/gst/vaapi/gstvaapidecoder_vp8.c
@@ +275,3 @@
+        temp_index < 0 ? 0 : (temp_index >
+        MAX_QI_INDEX ? MAX_QI_INDEX : temp_index);
+    iq_matrix->quantization_index[i][5] = temp_index;

This could have been much cleaner with standard CLAMP() macros.

@@ +353,3 @@
+  // reject frames for upscale, simple accept the downscale frames
+  if (priv->frame_width > priv->width || priv->frame_height > priv->height)
+    return FALSE;

horizontal_scale and vertical_scale are only useful for display and shall not affect the decoding process. In practice, it is not possible yet to honour the target display rect.
Comment 7 Gwenole Beauchesne 2014-04-19 08:27:06 UTC
commit 029bae0b6a62a5bf46116ec73c2747f4692f5c51
Author: Zhao, Halley <halley.zhao@intel.com>
Date:   Fri Dec 27 07:18:24 2013 +0800

    Add initial VP8 decoder.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=722761
    
    [complete overhaul, fixed support for resolution changes]
    Signed-off-by: Gwenole Beauchesne <gwenole.beauchesne@intel.com>