[Bug 693000] codecparsers: mpeg2: fix default quantizer matrix for intra blocks

GStreamer (bugzilla.gnome.org) bugzilla at gnome.org
Wed Feb 6 04:16:32 PST 2013


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

Tim-Philipp Müller <t.i.m> changed:

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

--- Comment #15 from Tim-Philipp Müller <t.i.m at zen.co.uk> 2013-02-06 12:16:31 UTC ---
(From update of attachment 235201)
This looks fine in principle, but I would like to bikeshed a bit about the
function naming / argument order:

In general the name should match the argument order as far as that's possible,
so e.g. zigzag_to_raster (in_zigzag, out_raster) or _get_raster_from_zigzag
(out_raster, in_zigzag).

Also, the 'object' being manipulated should ideally be at the front of the
name, so gst_mpeg_video_quant_matrix_xyz(), even if it's not a real object or
struct.

I think it should either be:

gst_mpeg_video_quant_matrix_zigzag_to_raster (const guint8 quant_zigzag[64],
guint8 quant_raster[64]);

or the other way round with _from_*. Or just _{to,from}_{zigzag,raster} even.

Do we need the to_zigzag/from_raster variant at all, or is it merely for
completeness?

Also, please add the bugzilla link to the bottom of the commit message.

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


More information about the gstreamer-bugs mailing list