[gstreamer-bugs] [Bug 607798] x264enc needs updating to support new features

[GStreamer (bugzilla.gnome.org)]

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

--- Comment #40 from Robert Swain <robert.swain at gmail.com> 2010-06-11 14:51:11 UTC ---
(In reply to comment #39)
> (In reply to comment #38)
> > (In reply to comment #37)
> > > (In reply to comment #28)
> > > > Created an attachment (id=162313)
 View: https://bugzilla.gnome.org/attachment.cgi?id=162313
 Review: https://bugzilla.gnome.org/review?bug=607798&attachment=162313

[details] [review] [details] [review] [details] [review]
> > > > Use x264 APIs to better configure libx264 in x264enc
> > > > 
> <snip>
> > > * provided option-string is internally appended to option_string (which is also
> > > appended to by setting "normal properties").  So if option-string is set, and
> > > then again set to e.g. "", the previously set options still remain in effect.
> > > Intended semantics are likely achieved by keeping this option-string in some
> > > separate string (from the one where other options are appended to), and then
> > > only append option-string to the "normal properties" at actual configuration
> > > time.
> > 
> > I think, and please correct me if I'm wrong, that currently if one specifies
> > the same option twice, the latter will be respected as it will override the
> > former. This is precisely what I want to preserve.
> 
> Yes, this overriding applies to the "normal properties", but not to a custom
> user set option-string property.  Consider case where some (gui) code sets
> option-string property to some value, then sets it to empty again.  Clearly,
> the element should then act according to no option-string provided.  However,
> current implementation will still have the originally set option-string flying
> around [since only appending is ever done, and appending "" can hardly override
> anything previously, nor does overriding apply if the options set 2nd time are
> different than the 1st time ones].

Aha, I see now. Will fix this too.

> > Maybe I need to make sure a separator is also appended in the correct locations
> > to avoid miss-parsing due to concatenation of strings.
> > 
> > > * why is there a x264enc_defaults (string) variable?  It seems to be used to
> > > set x264param according to the defaults of the properties.  However:
> > 
> > This is so that if no preset or tuning are selected, the property defaults will
> > be used. I need to revert my changes to that string though so that it does
> > indeed reflect the property defaults and so that we don't use presets/tunings
> > by default.
> > 
> > > - why mixed ?, i.e. part are set using the x264enc_defaults string, and other
> > > parameter struct members are set directly.
> > 
> > The ones that are set directly should be either interdependent on other such
> > options or should be independent of presets/tunings. I think that there is no
> > need to handle these parameters with strings.
> > 
> 
> Well, those would not necessarily have to be handled with a string.  Rather,
> why is there a string at all (?), and why are not all such set directly in the
> struct (mainly the mix being strange/confusing here)?

Use of the strings is mandatory to get the correct order of assignment and to
only assign user-set options.

Consider the case where a preset is selected. If we assign values directly to
the struct, we have no idea if they were user-set or if they are property
defaults. So what would happen is we would call the x264 API with the preset to
initialise the defaults, then assign all property values to the x264 parameters
(whether user-set or prop defaults) which would override the preset values and
nullify the point of a preset.

However, if we use setprop to assign to the struct and append to a string, we
can just parse the user-set properties from the string and ignore the property
defaults so that the preset is respected.

Maybe get prop should read from the x264param directly rather than
GstX264Enc...?

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