[Bug 733322] parser: mpeg4: problem with mpeg4_dmv_size_vlc_table values

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=733322
  GStreamer | gst-plugins-bad | unspecified

--- Comment #5 from Fabrice Bellet <fabrice at bellet.info> 2014-07-18 21:49:26 UTC ---
In my case, the file plays fine with ffmpeg  but fails with a pipeline
containing vaapidecode from gstreamer-vaapi. The difficulty to pinpoint this
bug is that _only_ the vaapidecode element invokes
gst_mpeg4_parse_video_object_plane(), which is the function that triggers this
error inside parse_sprite_trajectory() . Using a pipeline with the
mpeg4videoparse element only  wont invoke this function, so the problem will
stay hidden in this case. 

The error is related to the way VLC (variable length code) data is parsed for
the sprite trajectory for frame #345 of this sample file iirc. 

I spent some time wondering why the parsing of the _same_ bitfield was correct
with ffmpeg, and returned buggy values with gst, finally failing with a bad
marker bit detected by gst.

The bit stream should contain first the number of bits to be read using the VLC
algorithm , secondly a value coded on the  number of bits computed earlier, and
finally a marker bit that should be one. 

I discovered that the implementation of the VLC algorithm was different between
ffmpeg (get_vlc2()) and gst-plugins-bad (decode_vlc()). I had the feeling that
the ffmpeg implementation was correct, and the implementation of gst had
problem, because starting at the same location in the bitstream, gst version
fails when finding a wrong marker bit (0 instead of 1). 

So clearly the number of bits returned by decode_vlc() was wrong, the following
value in the bitstream was read with a bad offset and a bad length, and the
marker bit was checked at the wrong place.

Then I tried to understand what decode_vlc() was supposed to do. ffmpeg
implementation in get_vlc2(), even if correct, was difficult to read due to
heavy optimizations for speed. Until a found this older commit of the same
function, that explains the role of each value of the VLC table a bit better :
http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=7f4cf50496ef6797aaba819d0924bb5c4baf09a4

So, in the current implementation of decode_vlc(), in the VLC table for
mpeg4_dmv_size_vlc_table , you should have values in this order :
+  {0, 0x00, 2},
+  {1, 0x02, 3},
+  {2, 0x03, 3},
+  {3, 0x04, 3},
+  {4, 0x05, 3}

the definition of the table being :
struct _VLCTable
{
  guint value;
  guint cword;
  guint cbits;
};

The algorithm of decode_vlc() being :
. for each triplet of the VLCTable:
  if the "cbits" first bits of the bitstream (3rd row) match the
  "cword" value (2nd row), then the decoded value to be returned should
  be "value" value (1st row), else continue.

(the equivalent table in ffmpeg is ff_sprite_trajectory_tab[15][2] in
libavcodec/mpeg4data.h)

-- 
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.