<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<meta http-equiv="Content-Style-Type" content="text/css" />
<meta name="generator" content="pandoc" />
<title></title>
<style type="text/css">code{white-space: pre;}</style>
</head>
<body>
<p>Hi Hugo,</p>
<p>On 2017-04-10 17:39, Hugo Beauzée-Luyssen wrote:</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> By removing some long time deprecated options
---
modules/codec/avcodec/encoder.c | 31 ++++++++++++++++++++++---------
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/modules/codec/avcodec/encoder.c b/modules/codec/avcodec/encoder.c
index 29627011bd..b6691d38b6 100644
--- a/modules/codec/avcodec/encoder.c
+++ b/modules/codec/avcodec/encoder.c
@@ -555,6 +555,8 @@ int OpenEncoder( vlc_object_t *p_this )
}
}
free( psz_val );
+ AVDictionary *options = NULL;
+ char option_buff[128];
if( p_enc->fmt_in.i_cat == VIDEO_ES )
{
@@ -586,7 +588,8 @@ int OpenEncoder( vlc_object_t *p_this )
p_context->lumi_masking = p_sys->f_lumi_masking;
p_context->dark_masking = p_sys->f_dark_masking;
p_context->p_masking = p_sys->f_p_masking;
- p_context->border_masking = p_sys->f_border_masking;
+ sprintf(option_buff, "%f", p_sys->f_border_masking);</code></pre>
</blockquote>
<p>Personally I would rest a little easier if <code>snprintf</code> was used instead of <code>sprintf</code>, not that I am aware of any platform where the string-representation of a <code>float</code> can yield a string longer than <code>128</code>, but better safe than sorry.</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> + av_dict_set(&options, "border_mask", option_buff, 0);
if( p_sys->i_key_int > 0 )
p_context->gop_size = p_sys->i_key_int;
@@ -699,12 +702,16 @@ int OpenEncoder( vlc_object_t *p_this )
if( p_sys->i_qmin > 0 )
{
p_context->qmin = p_sys->i_qmin;
- p_context->mb_lmin = p_context->lmin = p_sys->i_qmin * FF_QP2LAMBDA;
+ p_context->mb_lmin = p_sys->i_qmin * FF_QP2LAMBDA;
+ sprintf(option_buff, "%d", p_context->mb_lmin);
+ av_dict_set(&options, "lmin", option_buff, 0);
}
if( p_sys->i_qmax > 0 )
{
p_context->qmax = p_sys->i_qmax;
- p_context->mb_lmax = p_context->lmax = p_sys->i_qmax * FF_QP2LAMBDA;
+ p_context->mb_lmax = p_sys->i_qmax * FF_QP2LAMBDA;
+ sprintf(option_buff, "%d", p_context->mb_lmax);
+ av_dict_set(&options, "lmax", option_buff, 0);
}
p_context->max_qdiff = 3;
@@ -717,7 +724,7 @@ int OpenEncoder( vlc_object_t *p_this )
}
else
{
- p_context->rc_qsquish = 1.0;
+ av_dict_set(&options, "qsquish", "1.0", 0);
/* Default to 1/2 second buffer for given bitrate unless defined otherwise*/
if( !p_sys->i_rc_buffer_size )
{
@@ -731,7 +738,8 @@ int OpenEncoder( vlc_object_t *p_this )
/* This is from ffmpeg's ffmpeg.c : */
p_context->rc_initial_buffer_occupancy
= p_sys->i_rc_buffer_size * 3/4;
- p_context->rc_buffer_aggressivity = p_sys->f_rc_buffer_aggressivity;
+ sprintf(option_buff, "%f", p_sys->f_rc_buffer_aggressivity);
+ av_dict_set(&options, "rc_buffer_aggressivity", option_buff, 0);
}
}
else if( p_enc->fmt_in.i_cat == AUDIO_ES )
@@ -868,19 +876,25 @@ int OpenEncoder( vlc_object_t *p_this )
if( !var_GetInteger( p_enc, ENC_CFG_PREFIX "qmin" ) )
{
p_context->qmin = 10;
- p_context->mb_lmin = p_context->lmin = 10 * FF_QP2LAMBDA;
+ p_context->mb_lmin = 10 * FF_QP2LAMBDA;
+ sprintf(option_buff, "%d", p_context->mb_lmin);
+ av_dict_set(&options, "lmin", option_buff, 0);
}
if( !var_GetInteger( p_enc, ENC_CFG_PREFIX "qmax" ) )
{
p_context->qmax = 42;
- p_context->mb_lmax = p_context->lmax = 42 * FF_QP2LAMBDA;
+ p_context->mb_lmax = 42 * FF_QP2LAMBDA;
+ sprintf(option_buff, "%d", p_context->mb_lmax);
+ av_dict_set(&options, "lmax", option_buff, 0);
}
} else if( !var_GetInteger( p_enc, ENC_CFG_PREFIX "qmin" ) )
{
p_context->qmin = 1;
- p_context->mb_lmin = p_context->lmin = FF_QP2LAMBDA;
+ p_context->mb_lmin = FF_QP2LAMBDA;
+ sprintf(option_buff, "%d", p_context->mb_lmin);
+ av_dict_set(&options, "lmin", option_buff, 0);
}</code></pre>
</blockquote>
<p>Further thoughts:</p>
<ul>
<li><p>Since the usage of <code>sprintf</code> + <code>av_dict_set</code> is used in so many places, maybe it is worth introducing a helper that does both in one call?</p></li>
<li><p>It would be appropriate to check whether <code>av_dict_set</code> was indeed successful where it is called, since it does return an error-code.</p></li>
</ul>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> @@ -913,7 +927,6 @@ int OpenEncoder( vlc_object_t *p_this )
int ret;
char *psz_opts = var_InheritString(p_enc, ENC_CFG_PREFIX "options");
- AVDictionary *options = NULL;
if (psz_opts) {
options = vlc_av_get_options(psz_opts);</code></pre>
</blockquote>
<p>Leak of the previously created <code>AVDictionary</code> as <code>sout-avcodec-options</code>, and other options now resulting in a write to <code>options</code>, are not mutually exclusive?</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> free(psz_opts);
--
2.11.0
_______________________________________________
vlc-devel mailing list
To unsubscribe or modify your subscription options:
https://mailman.videolan.org/listinfo/vlc-devel</code></pre>
</blockquote>
</body>
</html>