[Bug 722760] Add VP8 parser in codecparsers submodule

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=722760
  GStreamer | gst-plugins-bad | 1.x

Sebastian Dröge (slomo) <slomo> changed:

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

--- Comment #8 from Sebastian Dröge (slomo) <slomo at coaxion.net> 2014-01-23 09:41:15 UTC ---
Review of attachment 267015:
 --> (https://bugzilla.gnome.org/review?bug=722760&attachment=267015)

A parser element based on this would be good to have :) Also do you plan the
same for VP9 too?

::: gst-libs/gst/codecparsers/gstvp8parser.c
@@ +34,3 @@
+#include "string.h"
+
+// $13.4

no // comments, and some explanation what this number means (section of the
spec) might be useful :)

@@ +644,3 @@
+
+/**
+ * gst_vp8_parse:

parse_frame_header

@@ +664,3 @@
+  guint16 tmp_16;
+  gint pos, i, j;
+

You might want to init the frame_hdr with 0 in the beginning

@@ +700,3 @@
+  pos = gst_byte_reader_get_pos (&Br);
+  range_decoder =
+      gst_vp8_range_decoder_new (data + pos, size - pos);

Or maybe you want the range decoder to be stack allocatable too? Much simpler

::: gst-libs/gst/codecparsers/gstvp8rangedecoder.c
@@ +24,3 @@
+#define GST_VP8_RD_VALUE_SIZE ((gint)sizeof(GstVP8RDValue)*CHAR_BIT)
+
+// The range is [RANGE_MIN, RANGE_MAX)

No // comments

@@ +86,3 @@
+    return NULL;
+
+  rd = g_new0 (GstRangeDecoder, 1);

g_slice_new0()

@@ +94,3 @@
+
+gint
+gst_vp8_range_decoder_get_prob (GstRangeDecoder * rd, guint8 probability)

These public functions all need some gtk-doc documentation... how they're
supposed to be used, what they do. Or you make the range decoder private and
don't install the header. But it might be interesting to use for other code.

::: tests/parser/vp8-parser.c
@@ +1,1 @@
+/* vp8-parser.c

Put this file in tests/icles I guess

Also please provide a real unit test for the parser, see the existing ones for
h264 and others.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.