[FFmpeg-devel] [PATCH] avcodec/libvpxenc: add VP8 temporal scalability configuration options

[James Zern]

On Mon, Nov 12, 2018 at 10:37 AM Ard Oerlemans
<ardoerlemans-at-google.com at ffmpeg.org> wrote:
>
> Thanks for the suggestion. I have updated the patch.
>

Try to keep the original patch message so it's easier to apply.

> Ard
>
> ---
>  doc/encoders.texi      | 28 +++++++++++++++
>  libavcodec/libvpxenc.c | 79 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 107 insertions(+)
>

This looks OK, just some minor comments.

> [...]
> +static void vp8_ts_parse_int_array(int* dest, char *value, size_t value_len, int max_entries)

'*' is associated with the variable.

> [...]
> +{
> +    int dest_idx = 0;
> +    char *value_end = value + value_len;
> +    while (value < value_end && dest_idx < max_entries) {
> +        dest[dest_idx] = strtoul(value, &value, 10);
> +        while (value < value_end && (*value == ',' || av_isspace(*value)))
> +            value++;

Could this just be av_strtok?

> [...]
> @@ -1113,6 +1191,7 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket *pkt,
>  static const AVOption vp8_options[] = {
>      COMMON_OPTIONS
>      { "cpu-used",        "Quality/Speed ratio modifier",                OFFSET(cpu_used),        AV_OPT_TYPE_INT, {.i64 = 1}, -16, 16, VE},
> +    { "ts-parameters",   "Temporal scaling configuration using a :-separated list of key=value parameters", OFFSET(vp8_ts_parameters), AV_OPT_TYPE_STRING, {.str=NULL},  0,  0, VE},

This doesn't apply.