diff mbox series

[FFmpeg-devel,1/2] lavc/libaomenc: Show encoder config as a warning in case of failed initialization

Message ID a8fef793-9ccc-3a53-c12f-c2016ba74851@mail.de
State Accepted
Headers show
Series [FFmpeg-devel,1/2] lavc/libaomenc: Show encoder config as a warning in case of failed initialization
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Thilo Borgmann June 12, 2021, 7:10 p.m. UTC
Hi,

if init fails, it's likely originating from the library's config. This and 2/2 are for easier debugging in that case instead of having to run again with loglevel DEBUG.

-Thilo
From c165947450383da4e81ef0b0a1ec82509c698479 Mon Sep 17 00:00:00 2001
From: Matthieu Patou <mpatou@fb.com>
Date: Sat, 12 Jun 2021 20:59:29 +0200
Subject: [PATCH 1/2] lavc/libaomenc: Show encoder config as a warning in case
 of failed initialization

Suggested-By: ffmpeg@fb.com
---
 libavcodec/libaomenc.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

James Zern June 14, 2021, 5:39 p.m. UTC | #1
On Sat, Jun 12, 2021 at 12:10 PM Thilo Borgmann <thilo.borgmann@mail.de> wrote:
>
> Hi,
>
> if init fails, it's likely originating from the library's config. This and 2/2 are for easier debugging in that case instead of having to run again with loglevel DEBUG.
>

> ---
>  libavcodec/libaomenc.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>

lgtm.

> [...]
> -    dump_enc_cfg(avctx, &enccfg);
>      /* Construct Encoder Context */
>      res = aom_codec_enc_init(&ctx->encoder, iface, &enccfg, flags);
>      if (res != AOM_CODEC_OK) {
> +        dump_enc_cfg(avctx, &enccfg, AV_LOG_WARNING);
>          log_encoder_error(avctx, "Failed to initialize encoder");
>          return AVERROR(EINVAL);
> +    } else {

This else could be removed since the other branch returns.

> +        dump_enc_cfg(avctx, &enccfg, AV_LOG_DEBUG);
>      }
>
Thilo Borgmann June 14, 2021, 6:10 p.m. UTC | #2
Am 14.06.21 um 19:39 schrieb James Zern:
> On Sat, Jun 12, 2021 at 12:10 PM Thilo Borgmann <thilo.borgmann@mail.de> wrote:
>>
>> Hi,
>>
>> if init fails, it's likely originating from the library's config. This and 2/2 are for easier debugging in that case instead of having to run again with loglevel DEBUG.
>>
> 
>> ---
>>  libavcodec/libaomenc.c | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>
> 
> lgtm.
> 
>> [...]
>> -    dump_enc_cfg(avctx, &enccfg);
>>      /* Construct Encoder Context */
>>      res = aom_codec_enc_init(&ctx->encoder, iface, &enccfg, flags);
>>      if (res != AOM_CODEC_OK) {
>> +        dump_enc_cfg(avctx, &enccfg, AV_LOG_WARNING);
>>          log_encoder_error(avctx, "Failed to initialize encoder");
>>          return AVERROR(EINVAL);
>> +    } else {
> 
> This else could be removed since the other branch returns.
> 
>> +        dump_enc_cfg(avctx, &enccfg, AV_LOG_DEBUG);
>>      }

It would change existing behavior because currently, the cfg is always printed.
If we make that change, it would not be printed at all in that case. Not?

Thanks,
Thilo
Thilo Borgmann June 14, 2021, 6:13 p.m. UTC | #3
Am 14.06.21 um 20:10 schrieb Thilo Borgmann:
> Am 14.06.21 um 19:39 schrieb James Zern:
>> On Sat, Jun 12, 2021 at 12:10 PM Thilo Borgmann <thilo.borgmann@mail.de> wrote:
>>>
>>> Hi,
>>>
>>> if init fails, it's likely originating from the library's config. This and 2/2 are for easier debugging in that case instead of having to run again with loglevel DEBUG.
>>>
>>
>>> ---
>>>  libavcodec/libaomenc.c | 10 ++++++----
>>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>>
>>
>> lgtm.
>>
>>> [...]
>>> -    dump_enc_cfg(avctx, &enccfg);
>>>      /* Construct Encoder Context */
>>>      res = aom_codec_enc_init(&ctx->encoder, iface, &enccfg, flags);
>>>      if (res != AOM_CODEC_OK) {
>>> +        dump_enc_cfg(avctx, &enccfg, AV_LOG_WARNING);
>>>          log_encoder_error(avctx, "Failed to initialize encoder");
>>>          return AVERROR(EINVAL);
>>> +    } else {
>>
>> This else could be removed since the other branch returns.
>>
>>> +        dump_enc_cfg(avctx, &enccfg, AV_LOG_DEBUG);
>>>      }
> 
> It would change existing behavior because currently, the cfg is always printed.
> If we make that change, it would not be printed at all in that case. Not?

Ah I misunderstood. OK will apply with that change :)

Thanks,
Thilo
diff mbox series

Patch

diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
index 779714fdaa..468f2ba775 100644
--- a/libavcodec/libaomenc.c
+++ b/libavcodec/libaomenc.c
@@ -209,10 +209,10 @@  static av_cold void log_encoder_error(AVCodecContext *avctx, const char *desc)
 }
 
 static av_cold void dump_enc_cfg(AVCodecContext *avctx,
-                                 const struct aom_codec_enc_cfg *cfg)
+                                 const struct aom_codec_enc_cfg *cfg,
+                                 int level)
 {
     int width = -30;
-    int level = AV_LOG_DEBUG;
 
     av_log(avctx, level, "aom_codec_enc_cfg\n");
     av_log(avctx, level, "generic settings\n"
@@ -612,7 +612,7 @@  static av_cold int aom_init(AVCodecContext *avctx,
             return AVERROR(EINVAL);
         }
 
-    dump_enc_cfg(avctx, &enccfg);
+    dump_enc_cfg(avctx, &enccfg, AV_LOG_DEBUG);
 
     enccfg.g_w            = avctx->width;
     enccfg.g_h            = avctx->height;
@@ -746,12 +746,14 @@  static av_cold int aom_init(AVCodecContext *avctx,
     if (res < 0)
         return res;
 
-    dump_enc_cfg(avctx, &enccfg);
     /* Construct Encoder Context */
     res = aom_codec_enc_init(&ctx->encoder, iface, &enccfg, flags);
     if (res != AOM_CODEC_OK) {
+        dump_enc_cfg(avctx, &enccfg, AV_LOG_WARNING);
         log_encoder_error(avctx, "Failed to initialize encoder");
         return AVERROR(EINVAL);
+    } else {
+        dump_enc_cfg(avctx, &enccfg, AV_LOG_DEBUG);
     }
 
     // codec control failures are currently treated only as warnings