[gstreamer-bugs] [Bug 607798] x264enc needs updating to support new features
GStreamer (bugzilla.gnome.org)
bugzilla at gnome.org
Fri Jun 11 08:24:02 PDT 2010
https://bugzilla.gnome.org/show_bug.cgi?id=607798
GStreamer | gst-plugins-ugly | git
--- Comment #41 from Mark Nauwelaerts <mnauw at users.sourceforge.net> 2010-06-11 15:23:57 UTC ---
(In reply to comment #40)
> (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] [details] [review]
> > > > > Use x264 APIs to better configure libx264 in x264enc
> > > > >
<snip++>
> > > 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.
I am particularly talking about the following code fragment, which is the only
place where x264enc_defaults is actually used:
----
/* if no preset nor tuning, use property defaults */
if (!encoder->speed_preset && !encoder->tunings->len) {
if (gst_x264_enc_parse_options (encoder, x264enc_defaults) == FALSE) {
GST_DEBUG_OBJECT (encoder, "Failed to parse defaults string");
return FALSE;
}
encoder->x264param.b_deblocking_filter = 1;
encoder->x264param.i_deblocking_filter_alphac0 = 0;
encoder->x264param.i_deblocking_filter_beta = 0;
}
----
Hence, one need not consider the case where a preset is selected, because that
fails the if condition. So, again, in the above conditions, it is not clear
why some are set by a string in _parse_options, and the others set directly a
few lines below. [Suffice to say this is hardly critical/problematic, but the
above fragment does simply evoke "why?"]
>
> 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...?
See above, preset etc do not apply for x264enc_defaults' "scope".
--
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