[gstreamer-bugs] [Bug 621349] [theoraenc] Implement two-pass encoding

[GStreamer (bugzilla.gnome.org)]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #165185|none                        |reviewed
             status|                            |

--- Comment #2 from Tim-Philipp Müller <t.i.m at zen.co.uk> 2010-07-04 16:11:21 UTC ---
Review of attachment 165185:
 --> (https://bugzilla.gnome.org/review?bug=621349&attachment=165185)

I think we should bump the libtheora requirement to 1.1 and do away with all
the #ifdef stuff in this patch.

The next -base release is not before September, at which point 1.1 will have
been out for ca. a year, and IIRC there are great quality differences between
the 1.0 and 1.1 encoder, so we really want people to upgrade.

Also, that way apps can figure out from the presence of the properties if
multipass encoding is actually supported or not, seems much better to me.

New properties look ok. We should aim to standardise these properties in 0.11.

::: ext/theora/gsttheoraenc.c
@@ +92,3 @@
+  static GType multipass_mode_type = 0;
+  static const GEnumValue multipass_mode[] = {
+    {MULTIPASS_MODE_SINGLE_PASS, "Single pass (default)", "single-pass"},

The "(default)" bit is superfluous here IMHO. The default is documented via the
GParamSpec and shouldn't be part of the enum description.

@@ +247,3 @@
   gobject_class->finalize = theora_enc_finalize;

+  g_object_class_install_property (gobject_class, PROP_CENTER,

The ARG_* => PROP_* renaming should really be done in a separate commit :-)

@@ +408,3 @@
+    gst_object_unref (enc->multipass_cache_adapter);
+    enc->multipass_cache_adapter = NULL;
+  }

Maybe all this enc->multipass_* cleanup should be put into a separate function,
since the same code is repeated again below IIRC.

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