Encoder CLI uses internal API

[bradh]

I'm trying to duplicate the functionality of the command line app (encmain.c) using Java (to test a set of bindings using javacpp-presets).

However there appears to be a set of internal functions and constants that are called from that. For example, the padding determination requires CU_MIN_SIZE_PIXELS which isn't exposed. Similarly, there are places where the code requires values from the kvz_encoder opaque structure (e.g. to get bit depth, frame rate, format, etc) - some of that might be possible to avoid if we saved off the cfg but possibly not all.

Should it be possible to duplicate the command line encoder without internal API? If so, possibly the command line parts should be moved out of the src directory and only call API that is exposed through kvazaar.h.

Thoughts? Ideas?

[bradh]

I attempted to split out the CLI parts, and not use internal API.

The result is:

  CC       encmain.o
encmain.c: In function ‘get_padding’:
encmain.c:96:25: error: ‘CU_MIN_SIZE_PIXELS’ undeclared (first use in this function)
   96 |   if (width_or_height % CU_MIN_SIZE_PIXELS){
      |                         ^~~~~~~~~~~~~~~~~~
encmain.c:96:25: note: each undeclared identifier is reported only once for each function it appears in
encmain.c: At top level:
encmain.c:107:49: error: ‘PIXEL_MAX’ undeclared here (not in a function); did you mean ‘SIZE_MAX’?
  107 | static const double MAX_SQUARED_ERROR = (double)PIXEL_MAX * (double)PIXEL_MAX;
      |                                                 ^~~~~~~~~
      |                                                 SIZE_MAX
encmain.c: In function ‘compute_psnr’:
encmain.c:129:14: error: ‘COLOR_Y’ undeclared (first use in this function)
  129 |     if (c != COLOR_Y) {
      |              ^~~~~~~
encmain.c: At top level:
encmain.c:148:3: error: unknown type name ‘kvz_sem_t’
  148 |   kvz_sem_t* available_input_slots;
      |   ^~~~~~~~~
encmain.c:149:3: error: unknown type name ‘kvz_sem_t’
  149 |   kvz_sem_t* filled_input_slots;
      |   ^~~~~~~~~
encmain.c:155:9: error: unknown type name ‘encoder_control_t’
  155 |   const encoder_control_t *encoder;
      |         ^~~~~~~~~~~~~~~~~
encmain.c: In function ‘input_read_thread’:
encmain.c:216:50: error: request for member ‘cfg’ in something not a structure or union
  216 |                                     args->encoder->cfg.input_bitdepth,
      |                                                  ^~
encmain.c:217:50: error: request for member ‘bitdepth’ in something not a structure or union
  217 |                                     args->encoder->bitdepth,
      |                                                  ^~
encmain.c:235:56: error: request for member ‘cfg’ in something not a structure or union
  235 |                                           args->encoder->cfg.input_bitdepth,
      |                                                        ^~
encmain.c:236:56: error: request for member ‘bitdepth’ in something not a structure or union
  236 |                                           args->encoder->bitdepth,
      |                                                        ^~
encmain.c:256:22: error: request for member ‘cfg’ in something not a structure or union
  256 |     if (args->encoder->cfg.source_scan_type != 0) {
      |                      ^~
encmain.c:258:44: error: request for member ‘cfg’ in something not a structure or union
  258 |       frame_in->interlacing = args->encoder->cfg.source_scan_type;
      |                                            ^~
encmain.c:262:5: warning: implicit declaration of function ‘kvz_sem_wait’ [-Wimplicit-function-declaration]
  262 |     kvz_sem_wait(args->available_input_slots);
      |     ^~~~~~~~~~~~
encmain.c:267:5: warning: implicit declaration of function ‘kvz_sem_post’ [-Wimplicit-function-declaration]
  267 |     kvz_sem_post(args->filled_input_slots);
      |     ^~~~~~~~~~~~
encmain.c: In function ‘main’:
encmain.c:436:3: error: unknown type name ‘KVZ_CLOCK_T’
  436 |   KVZ_CLOCK_T encoding_start_real_time;
      |   ^~~~~~~~~~~
encmain.c:439:3: error: unknown type name ‘KVZ_CLOCK_T’
  439 |   KVZ_CLOCK_T encoding_end_real_time;
      |   ^~~~~~~~~~~
encmain.c:460:3: error: unknown type name ‘kvz_sem_t’
  460 |   kvz_sem_t *available_input_slots = NULL;
      |   ^~~~~~~~~
encmain.c:461:3: error: unknown type name ‘kvz_sem_t’
  461 |   kvz_sem_t *filled_input_slots = NULL;
      |   ^~~~~~~~~
encmain.c:468:3: warning: implicit declaration of function ‘CHECKPOINTS_INIT’ [-Wimplicit-function-declaration]
  468 |   CHECKPOINTS_INIT();
      |   ^~~~~~~~~~~~~~~~
encmain.c:531:9: error: unknown type name ‘encoder_control_t’
  531 |   const encoder_control_t *encoder = enc->control;
      |         ^~~~~~~~~~~~~~~~~
encmain.c:531:41: error: invalid use of incomplete typedef ‘kvz_encoder’
  531 |   const encoder_control_t *encoder = enc->control;
      |                                         ^~
encmain.c:535:17: error: request for member ‘in’ in something not a structure or union
  535 |          encoder->in.width, encoder->in.height,
      |                 ^~
encmain.c:535:36: error: request for member ‘in’ in something not a structure or union
  535 |          encoder->in.width, encoder->in.height,
      |                                    ^~
encmain.c:536:17: error: request for member ‘in’ in something not a structure or union
  536 |          encoder->in.real_width, encoder->in.real_height);
      |                 ^~
encmain.c:536:41: error: request for member ‘in’ in something not a structure or union
  536 |          encoder->in.real_width, encoder->in.real_height);
      |                                         ^~
encmain.c:546:5: warning: implicit declaration of function ‘KVZ_GET_TIME’ [-Wimplicit-function-declaration]
  546 |     KVZ_GET_TIME(&encoding_start_real_time);
      |     ^~~~~~~~~~~~
encmain.c:558:44: error: request for member ‘cfg’ in something not a structure or union
  558 |     const float framerate = ((float)encoder->cfg.framerate_num) / ((float)encoder->cfg.framerate_denom);
      |                                            ^~
encmain.c:558:82: error: request for member ‘cfg’ in something not a structure or union
  558 |     const float framerate = ((float)encoder->cfg.framerate_num) / ((float)encoder->cfg.framerate_denom);
      |                                                                                  ^~
encmain.c:565:46: error: ‘kvz_sem_t’ undeclared (first use in this function)
  565 |     available_input_slots = calloc(1, sizeof(kvz_sem_t));
      |                                              ^~~~~~~~~
encmain.c:567:5: warning: implicit declaration of function ‘kvz_sem_init’ [-Wimplicit-function-declaration]
  567 |     kvz_sem_init(available_input_slots, 0);
      |     ^~~~~~~~~~~~
encmain.c:669:41: error: request for member ‘cfg’ in something not a structure or union
  669 |           if (bits_this_second > encoder->cfg.max_bitrate) {
      |                                         ^~
encmain.c:673:22: error: request for member ‘cfg’ in something not a structure or union
  673 |               encoder->cfg.max_bitrate,
      |                      ^~
encmain.c:674:22: error: request for member ‘cfg’ in something not a structure or union
  674 |               encoder->cfg.high_tier ? "high" : "main",
      |                      ^~
encmain.c:675:29: error: request for member ‘cfg’ in something not a structure or union
  675 |               (float)encoder->cfg.level / 10.0f );
      |                             ^~
encmain.c:693:20: error: request for member ‘cfg’ in something not a structure or union
  693 |         if (encoder->cfg.calc_psnr && encoder->cfg.source_scan_type == KVZ_INTERLACING_NONE) {
      |                    ^~
encmain.c:693:46: error: request for member ‘cfg’ in something not a structure or union
  693 |         if (encoder->cfg.calc_psnr && encoder->cfg.source_scan_type == KVZ_INTERLACING_NONE) {
      |                                              ^~
encmain.c:725:65: error: request for member ‘cfg’ in something not a structure or union
  725 |         print_frame_info(&info_out, frame_psnr, len_out, encoder->cfg.calc_psnr,
      |                                                                 ^~
encmain.c:746:16: error: request for member ‘cfg’ in something not a structure or union
  746 |     if (encoder->cfg.calc_psnr && frames_done > 0) {
      |                ^~
encmain.c:759:26: warning: implicit declaration of function ‘KVZ_CLOCK_T_AS_DOUBLE’ [-Wimplicit-function-declaration]
  759 |       double wall_time = KVZ_CLOCK_T_AS_DOUBLE(encoding_end_real_time) - KVZ_CLOCK_T_AS_DOUBLE(encoding_start_real_time);
      |                          ^~~~~~~~~~~~~~~~~~~~~
encmain.c:765:44: error: request for member ‘cfg’ in something not a structure or union
  765 |       double sf_num       = (double)encoder->cfg.framerate_num;
      |                                            ^~
encmain.c:766:44: error: request for member ‘cfg’ in something not a structure or union
  766 |       double sf_den       = (double)encoder->cfg.framerate_denom;
      |                                            ^~
encmain.c:799:30: warning: implicit declaration of function ‘kvz_sem_destroy’ [-Wimplicit-function-declaration]
  799 |   if (available_input_slots) kvz_sem_destroy(available_input_slots);
      |                              ^~~~~~~~~~~~~~~
encmain.c:814:3: warning: implicit declaration of function ‘CHECKPOINTS_FINALIZE’ [-Wimplicit-function-declaration]
  814 |   CHECKPOINTS_FINALIZE();
      |   ^~~~~~~~~~~~~~~~~~~~
encmain.c: In function ‘get_padding’:
encmain.c:101:1: warning: control reaches end of non-void function [-Wreturn-type]
  101 | }
      | ^

If you'd like to see what it looks like, it is up at bradh@801fdb2

It will compile and run OK with use of:

#include "../src/checkpoint.h"
#include "../src/encoder.h"
#include "../src/kvazaar_internal.h"
#include "../src/threads.h"

[Jovasa]

All relevant people should be back in office next week. I'll try to start internal discussion on this then, but people might have quite a lot of stuff on their backlog after their holidays, so there might not be anything immediately regarding this issue.

[Jovasa]

We finally had time to discuss this and while we do agree that it should be possible to replicate the CLI with just including kvazaar.h. However, we unfortunately do not have time to do this in the foreseeable future. If you want to implement this I can give you some pointers how to get started and help if you get stuck. Overall, this shouldn't be a massive job but also not a trivial change.