Re: [FFmpeg-devel] [PATCH v5] avcodec/libvpxenc: optimize parsing vpx_svc_ref_frame_config

2021-02-19 Thread James Zern
On Thu, Feb 18, 2021 at 1:17 PM James Zern  wrote:
>
> On Thu, Feb 18, 2021 at 11:01 AM Wonkap Jang
>  wrote:
> >
> > Getting rid of unnecessary use of AVDictionary object in parsing
> > vpx_svc_ref_frame_config.
> > ---
> >  libavcodec/libvpxenc.c | 73 +++---
> >  1 file changed, 55 insertions(+), 18 deletions(-)
> >
>
> lgtm, just a couple of cosmetics I addressed locally. I'll submit this
> soon. Thanks for the suggestions Nicolas.
>

applied, thanks.

> > diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> > index 284cb9a108..dfa0763fff 100644
> > --- a/libavcodec/libvpxenc.c
> > +++ b/libavcodec/libvpxenc.c
> > @@ -42,6 +42,7 @@
> >  #include "libavutil/mathematics.h"
> >  #include "libavutil/opt.h"
> >  #include "libavutil/pixdesc.h"
> > +#include "libavutil/avstring.h"
> >
>
> This is already included, in general the includes can be sorted.
> Deleted locally.
>
> > [...]
> > -av_log(avctx, AV_LOG_WARNING,
> > -   "Error parsing option '%s = %s'.\n",
> > -   en2->key, en2->value);
> > +int ret;
> > +ret = 
> > vpx_parse_ref_frame_config(>ref_frame_config,
> > +   
> > enccfg->ss_number_layers, en->value);
>
> I joined these two lines locally.
>
> > +if (ret < 0) {
> > +av_log(avctx, AV_LOG_WARNING,
> > +   "Error parsing ref_frame_config option 
> > %s.\n", en->value);
> > +return ret;
> >  }
> >
> >  codecctl_intp(avctx, VP9E_SET_SVC_REF_FRAME_CONFIG, 
> > (int *)>ref_frame_config);
> >  } else {
> >  av_log(avctx, AV_LOG_WARNING,
> > -   "Error using option ref-frame-config for a 
> > non-VP9 codec\n");
> > +   "Using option ref-frame-config for a non-VP9 
> > codec\n");
>
> I changed 'Using' to 'Ignoring' locally.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v5] avcodec/libvpxenc: optimize parsing vpx_svc_ref_frame_config

2021-02-18 Thread Wonkap Jang
On Thu, Feb 18, 2021 at 1:26 PM James Zern 
wrote:

> On Thu, Feb 18, 2021 at 11:01 AM Wonkap Jang
>  wrote:
> >
> > Getting rid of unnecessary use of AVDictionary object in parsing
> > vpx_svc_ref_frame_config.
> > ---
> >  libavcodec/libvpxenc.c | 73 +++---
> >  1 file changed, 55 insertions(+), 18 deletions(-)
> >
>
> lgtm, just a couple of cosmetics I addressed locally. I'll submit this
> soon. Thanks for the suggestions Nicolas.
>
> > diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> > index 284cb9a108..dfa0763fff 100644
> > --- a/libavcodec/libvpxenc.c
> > +++ b/libavcodec/libvpxenc.c
> > @@ -42,6 +42,7 @@
> >  #include "libavutil/mathematics.h"
> >  #include "libavutil/opt.h"
> >  #include "libavutil/pixdesc.h"
> > +#include "libavutil/avstring.h"
> >
>
> This is already included, in general the includes can be sorted.
> Deleted locally.
>
> > [...]
> > -av_log(avctx, AV_LOG_WARNING,
> > -   "Error parsing option '%s = %s'.\n",
> > -   en2->key, en2->value);
> > +int ret;
> > +ret =
> vpx_parse_ref_frame_config(>ref_frame_config,
> > +
>  enccfg->ss_number_layers, en->value);
>
> I joined these two lines locally.
>
> > +if (ret < 0) {
> > +av_log(avctx, AV_LOG_WARNING,
> > +   "Error parsing ref_frame_config option
> %s.\n", en->value);
> > +return ret;
> >  }
> >
> >  codecctl_intp(avctx, VP9E_SET_SVC_REF_FRAME_CONFIG,
> (int *)>ref_frame_config);
> >  } else {
> >  av_log(avctx, AV_LOG_WARNING,
> > -   "Error using option ref-frame-config for a
> non-VP9 codec\n");
> > +   "Using option ref-frame-config for a non-VP9
> codec\n");
>
> I changed 'Using' to 'Ignoring' locally.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Thank you all.

Wonkap

>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v5] avcodec/libvpxenc: optimize parsing vpx_svc_ref_frame_config

2021-02-18 Thread James Zern
On Thu, Feb 18, 2021 at 11:01 AM Wonkap Jang
 wrote:
>
> Getting rid of unnecessary use of AVDictionary object in parsing
> vpx_svc_ref_frame_config.
> ---
>  libavcodec/libvpxenc.c | 73 +++---
>  1 file changed, 55 insertions(+), 18 deletions(-)
>

lgtm, just a couple of cosmetics I addressed locally. I'll submit this
soon. Thanks for the suggestions Nicolas.

> diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> index 284cb9a108..dfa0763fff 100644
> --- a/libavcodec/libvpxenc.c
> +++ b/libavcodec/libvpxenc.c
> @@ -42,6 +42,7 @@
>  #include "libavutil/mathematics.h"
>  #include "libavutil/opt.h"
>  #include "libavutil/pixdesc.h"
> +#include "libavutil/avstring.h"
>

This is already included, in general the includes can be sorted.
Deleted locally.

> [...]
> -av_log(avctx, AV_LOG_WARNING,
> -   "Error parsing option '%s = %s'.\n",
> -   en2->key, en2->value);
> +int ret;
> +ret = vpx_parse_ref_frame_config(>ref_frame_config,
> +   enccfg->ss_number_layers, 
> en->value);

I joined these two lines locally.

> +if (ret < 0) {
> +av_log(avctx, AV_LOG_WARNING,
> +   "Error parsing ref_frame_config option 
> %s.\n", en->value);
> +return ret;
>  }
>
>  codecctl_intp(avctx, VP9E_SET_SVC_REF_FRAME_CONFIG, (int 
> *)>ref_frame_config);
>  } else {
>  av_log(avctx, AV_LOG_WARNING,
> -   "Error using option ref-frame-config for a 
> non-VP9 codec\n");
> +   "Using option ref-frame-config for a non-VP9 
> codec\n");

I changed 'Using' to 'Ignoring' locally.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v5] avcodec/libvpxenc: optimize parsing vpx_svc_ref_frame_config

2021-02-18 Thread Nicolas George
Wonkap Jang (12021-02-18):
> Getting rid of unnecessary use of AVDictionary object in parsing
> vpx_svc_ref_frame_config.
> ---
>  libavcodec/libvpxenc.c | 73 +++---
>  1 file changed, 55 insertions(+), 18 deletions(-)

No more remarks from me. Thanks.

Regards,

-- 
  Nicolas George


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v5] avcodec/libvpxenc: optimize parsing vpx_svc_ref_frame_config

2021-02-18 Thread Wonkap Jang
Getting rid of unnecessary use of AVDictionary object in parsing
vpx_svc_ref_frame_config.
---
 libavcodec/libvpxenc.c | 73 +++---
 1 file changed, 55 insertions(+), 18 deletions(-)

diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
index 284cb9a108..dfa0763fff 100644
--- a/libavcodec/libvpxenc.c
+++ b/libavcodec/libvpxenc.c
@@ -42,6 +42,7 @@
 #include "libavutil/mathematics.h"
 #include "libavutil/opt.h"
 #include "libavutil/pixdesc.h"
+#include "libavutil/avstring.h"
 
 /**
  * Portion of struct vpx_codec_cx_pkt from vpx_encoder.h.
@@ -127,7 +128,6 @@ typedef struct VPxEncoderContext {
 int roi_warned;
 #if CONFIG_LIBVPX_VP9_ENCODER && 
defined(VPX_CTRL_VP9E_SET_MAX_INTER_BITRATE_PCT)
 vpx_svc_ref_frame_config_t ref_frame_config;
-AVDictionary *vpx_ref_frame_config;
 #endif
 } VPxContext;
 
@@ -561,18 +561,13 @@ static int vpx_ts_param_parse(VPxContext *ctx, struct 
vpx_codec_enc_cfg *enccfg,
 }
 
 #if CONFIG_LIBVPX_VP9_ENCODER && 
defined(VPX_CTRL_VP9E_SET_MAX_INTER_BITRATE_PCT)
-static int vpx_ref_frame_config_parse(VPxContext *ctx, const struct 
vpx_codec_enc_cfg *enccfg,
-  char *key, char *value, enum AVCodecID 
codec_id)
+static int vpx_ref_frame_config_set_value(vpx_svc_ref_frame_config_t 
*ref_frame_config,
+  int ss_number_layers, char *key, 
char *value)
 {
 size_t value_len = strlen(value);
-int ss_number_layers = enccfg->ss_number_layers;
-vpx_svc_ref_frame_config_t *ref_frame_config = >ref_frame_config;
 
 if (!value_len)
-return -1;
-
-if (codec_id != AV_CODEC_ID_VP9)
-return -1;
+return AVERROR(EINVAL);
 
 if (!strcmp(key, "rfc_update_buffer_slot")) {
 vp8_ts_parse_int_array(ref_frame_config->update_buffer_slot, value, 
value_len, ss_number_layers);
@@ -600,6 +595,49 @@ static int vpx_ref_frame_config_parse(VPxContext *ctx, 
const struct vpx_codec_en
 
 return 0;
 }
+
+static int vpx_parse_ref_frame_config_element(vpx_svc_ref_frame_config_t 
*ref_frame_config,
+  int ss_number_layers, const char 
**buf)
+{
+const char key_val_sep[] = "=";
+const char pairs_sep[] = ":";
+char *key = av_get_token(buf, key_val_sep);
+char *val = NULL;
+int ret;
+
+if (key && *key && strspn(*buf, key_val_sep)) {
+(*buf)++;
+val = av_get_token(buf, pairs_sep);
+}
+
+if (key && *key && val && *val)
+ret = vpx_ref_frame_config_set_value(ref_frame_config, 
ss_number_layers, key, val);
+else
+ret = AVERROR(EINVAL);
+
+av_freep();
+av_freep();
+
+return ret;
+}
+
+static int vpx_parse_ref_frame_config(vpx_svc_ref_frame_config_t 
*ref_frame_config,
+  int ss_number_layers, const char *str)
+{
+int ret = 0;
+
+while (*str) {
+ret =
+vpx_parse_ref_frame_config_element(ref_frame_config, 
ss_number_layers, );
+if (ret < 0)
+return ret;
+
+if (*str)
+str++;
+}
+
+return ret;
+}
 #endif
 
 #if CONFIG_LIBVPX_VP9_ENCODER
@@ -1594,20 +1632,19 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket 
*pkt,
 
 if (en) {
 if (avctx->codec_id == AV_CODEC_ID_VP9) {
-AVDictionaryEntry* en2 = NULL;
-av_dict_parse_string(>vpx_ref_frame_config, 
en->value, "=", ":", 0);
-
-while ((en2 = av_dict_get(ctx->vpx_ref_frame_config, "", 
en2, AV_DICT_IGNORE_SUFFIX))) {
-if (vpx_ref_frame_config_parse(ctx, enccfg, en2->key, 
en2->value, avctx->codec_id) < 0)
-av_log(avctx, AV_LOG_WARNING,
-   "Error parsing option '%s = %s'.\n",
-   en2->key, en2->value);
+int ret;
+ret = vpx_parse_ref_frame_config(>ref_frame_config,
+   enccfg->ss_number_layers, 
en->value);
+if (ret < 0) {
+av_log(avctx, AV_LOG_WARNING,
+   "Error parsing ref_frame_config option %s.\n", 
en->value);
+return ret;
 }
 
 codecctl_intp(avctx, VP9E_SET_SVC_REF_FRAME_CONFIG, (int 
*)>ref_frame_config);
 } else {
 av_log(avctx, AV_LOG_WARNING,
-   "Error using option ref-frame-config for a non-VP9 
codec\n");
+   "Using option ref-frame-config for a non-VP9 
codec\n");
 }
 }
 #endif
-- 
2.30.0.617.g56c4b15f3c-goog

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email