[FFmpeg-devel] [PATCH] lavc,lavf: add libxavs2 encoder

[Moritz Barsnick]

On Mon, May 28, 2018 at 19:16:49 +0800, hwren wrote:
> >Is that really the name of the codec? Otherwise, "Chinese" seems
> >awkward to me. (Same for the decoder.)
> 
> Changed.

I still see it in your newly pushlished patches.

> >Proper documentation of the encoder's options would be appreciated!
> 
> I'm sorry to say that xavs2 could only use '-h' to get some
> information about its options. We'll make a documentation soon.

I don't understand. You explicitly implemented these options:

+static const AVOption options[] = {
+   { "i_lcurow_threads",           "number of parallel threads for rows"     ,       OFFSET(i_lcurow_threads),  AV_OPT_TYPE_INT,    {.i64 =  5 }, 1,   8,  VE },
+    { "i_frame_threads" ,           "number of parallel threads for frames"   ,       OFFSET(i_frame_threads) ,  AV_OPT_TYPE_INT,    {.i64 =  1 }, 1,   4,  VE },
+    { "i_initial_qp"    ,           "Quantization parameter",       OFFSET(i_initial_qp)    ,  AV_OPT_TYPE_INT,    {.i64 = 34 }, 1,  63,  VE },
+    { "preset_level"    ,           "Speed level"           ,       OFFSET(preset_level)    ,  AV_OPT_TYPE_INT,    {.i64 =  0 }, 0,   9,  VE },
+    { "intra_period"    ,           "Intra period"          ,       OFFSET(intra_period)    ,  AV_OPT_TYPE_INT,    {.i64 =  4 }, 3, 100,  VE },
+    { "hierarchical_ref",           "hierarchical reference",       OFFSET(b_hierarchical_reference)    ,  AV_OPT_TYPE_INT,    {.i64 =  1 }, 0, 1,  VE },
+    { "num_bframes"     ,           "number of B frames"    ,       OFFSET(num_b_frames)    ,  AV_OPT_TYPE_INT,    {.i64 =  7 }, 0,  15,  VE },
+    { "xavs2-params",    "set the xavs2 configuration using a :-separated list of key=value parameters", OFFSET(xavs2_opts), AV_OPT_TYPE_STRING, { 0 }, 0, 0, VE },
+    { NULL },
+};

Their description (incl. ranges with explanations, if possible) should
also go into doc/*.texi. At least mention them there initially, if you
don't know what they actually do.

> Thanks for suggestions. We've put them into EXTERNAL_LIBRARY_GPL_LIST
> and added the gpl dependency. I'm not sure if I did right, new
> patches are submitted.

Let's see what the other say.

Cheers,
Moritz