ultravideo / kvazaar

An open-source HEVC encoder
BSD 3-Clause "New" or "Revised" License
838 stars 181 forks source link

GPAC/libavcodec integration #292

Open jeanlf opened 3 years ago

jeanlf commented 3 years ago

Greetings guys!

I've been playing with tile generation in GPAC new arch (see https://github.com/gpac/gpac/issues/1648#issuecomment-737387894) and came up with the following issues:

Specifying bitrate

When you specify the same set of kvazaar-paramsfor multiple encodings and want to vary only the bitrate through libavcodec b option, the config validation fails because target_bitrate is set but not rc_algorithm. The only workaround for now is to specify all arguments to each kvazaar instance which is quite painfull and error prone...

I patched that in my local libavcodec but it would be nicer to patch it in kvazaar to avoid updating libavcodec wrapper (maybe print a warning and turn on KVZ_LAMBDA).

Logging

It would be nice to be able to disable stderr logs seen by default (threads, MMX & co) , or redirect them (not urgent).

IDR forcing

I'm looking for a way to force an IDR at a specific frame but I can't find a place in the code where this could be done, it seems it can only be done based on the GOP length. The use case is a DASH segment start being forced somewhere in the middle of a GOP. Currently this is dealt with in GPAC by flushing/closing/opening the decoder, but that is a bit heavy (thread destroy/recreate). Any chance such a filter could be integrated in future versions ?

Cheers,

Jean.

jeanlf commented 3 years ago

For point 1, I have found a workaround by specifying rc-algorithm=lambda in the global args, see our HEVC tile adaptation wiki page.

Jovasa commented 3 years ago

Thanks for reaching to us!

For point 1 the cli version already defaults to KVZ_LAMBDA when bitrate is set. I'd personally prefer to do the same on the libavcodec side since if it is done inside the encoder constness would have to be removed from the kvz_config inside the encoder, and I'd prefer not to do that.

For point 2, we can do this if it's not too much of a work but I have a feeling that anything other than disabling at compile time is not going to happen, if even that.

For point 3, we are currently working on something that would allow arbitrary IDR placement but I don't have any promises when this will be ready and published.

We will have an internal discussion regarding these probably the next week.

jeanlf commented 3 years ago

Thanks for the feedback!

  1. OK with me, FYI my patch was

    diff --git a/libavcodec/libkvazaar.c b/libavcodec/libkvazaar.c
    index 9032547678..0ecef5f8e6 100644
    --- a/libavcodec/libkvazaar.c
    +++ b/libavcodec/libkvazaar.c
    @@ -109,6 +109,9 @@ static av_cold int libkvazaar_init(AVCodecContext *avctx)
         }
         av_dict_free(&dict);
     }
    +    if (cfg->target_bitrate && !cfg->rc_algorithm) {
    +               cfg->rc_algorithm = KVZ_LAMBDA;
    +    }
    
     ctx->encoder = enc = api->encoder_open(cfg);
     if (!enc) {
  2. sure I understand

  3. Good to hear this :) Don't hesitate to ping me if needed

Jovasa commented 3 years ago

The first issue is now fixed in https://github.com/FFmpeg/FFmpeg/commit/0cd8769207f1a89fc2236aab9da1c77f5a0b490a

jeanlf commented 3 years ago

great, thanks !