Hi Lukasz

On 11/11/2014 04:31 AM, Lukasz Marek wrote:
This commit allows to set codec's private option.
As side effect this commit also improves preset support.

s/this commit also/, it/

[..]
+static int ffserver_save_avoption(AVCodecContext *ctx, const char *opt, const 
char *arg,
+                                  AVDictionary **dict, int type, 
FFServerConfig *config, int line_num);
+static void vreport_config_error(const char *filename, int line_num, int 
log_level, int *errors, const char *fmt, va_list vl);
+static void report_config_error(const char *filename, int line_num, int 
log_level, int *errors, const char *fmt, ...);
+

These random length line breaks are getting a bit messy, I don't
mind if you don't wana break at 80 but please be consistent.

[..]
@@ -293,16 +295,31 @@ static int ffserver_opt_preset(const char *arg,
              ret = AVERROR(EINVAL);
              break;
          }
-        if (audio_id && !strcmp(tmp, "acodec")) {
-            *audio_id = opt_codec(tmp2, AVMEDIA_TYPE_AUDIO);
-        } else if (video_id && !strcmp(tmp, "vcodec")){
-            *video_id = opt_codec(tmp2, AVMEDIA_TYPE_VIDEO);
-        } else if(!strcmp(tmp, "scodec")) {
-            /* opt_subtitle_codec(tmp2); */
-        } else if (avctx && (ret = ffserver_opt_default(tmp, tmp2, avctx, type)) 
< 0) {
-            fprintf(stderr, "%s: Invalid option or argument: '%s', parsed as "
-                    "'%s' = '%s'\n", filename, line, tmp, tmp2);
-            break;
+        if ((!strcmp(tmp, "acodec") && avctx->codec_type == 
AVMEDIA_TYPE_AUDIO) ||
+             !strcmp(tmp, "vcodec") && avctx->codec_type == AVMEDIA_TYPE_VIDEO)
+        {
+            if (ffserver_set_codec(avctx, tmp2, config, line_num) < 0)
+                break;

Factor in previous if() condition?

+        } else if (!strcmp(tmp, "scodec")) {
+            /* nothing to do */

Not sure why are we leaving this one there? Proly drop and add
an informative comment if needed.

+        } else {
+            int type;
+            AVDictionary **opts;

Add blank line please (Can you do it for other similar occurrences?)

[..]
@@ -406,27 +424,67 @@ static int ffserver_set_float_param(float *dest, const 
char *value, float factor
      return AVERROR(EINVAL);
  }

-static int ffserver_save_avoption(const char *opt, const char *arg, 
AVDictionary **dict,
+static int ffserver_save_avoption(AVCodecContext *ctx, const char *opt, const 
char *arg, AVDictionary **dict,
                                    int type, FFServerConfig *config, int 
line_num)
  {
+    static int hinted = 0;
      int ret = 0;
      AVDictionaryEntry *e;
-    const AVOption *o = av_opt_find(config->dummy_ctx, opt, NULL, type | 
AV_OPT_FLAG_ENCODING_PARAM, AV_OPT_SEARCH_CHILDREN);
+    const AVOption *o = NULL;
+    const char *option = NULL;
+    const char *codec_name = NULL;
+
+    if (strchr(opt, ':')) {
+        //explicit private option
+        char buff[1024];

See above (there are others though)

+        snprintf(buff, sizeof(buff), "%s", opt);

Pointless sizeof but it's OK.

+        codec_name = buff;
+        option = strchr(buff, ':');
+        buff[option - buff] = '\0';

Can't strchr() return NULL and wreak havoc here?

+        option++;
+        if ((ret = ffserver_set_codec(ctx, codec_name, config, line_num)) < 0)
+            return ret;
+        if (!ctx->codec || !ctx->priv_data)
+            return -1;
+    } else {
+        option = opt;
+    }
+
+    o = av_opt_find(ctx, option, NULL, type | AV_OPT_FLAG_ENCODING_PARAM, 
AV_OPT_SEARCH_CHILDREN);
      if (!o) {
          report_config_error(config->filename, line_num, AV_LOG_ERROR,
-                &config->errors, "Option not found: %s\n", opt);
-    } else if ((ret = av_opt_set(config->dummy_ctx, opt, arg, 
AV_OPT_SEARCH_CHILDREN)) < 0) {
+                            &config->errors, "Option not found: %s\n", opt);
+        if (!hinted && ctx->codec_id == AV_CODEC_ID_NONE) {
+            enum AVCodecID id;
+            hinted = 1;
+            switch(ctx->codec_type) {
+            case AVMEDIA_TYPE_AUDIO:
+                id = config->guessed_audio_codec_id != AV_CODEC_ID_NONE ? 
config->guessed_audio_codec_id : AV_CODEC_ID_AAC;
+                break;
+            case AVMEDIA_TYPE_VIDEO:
+                id = config->guessed_video_codec_id != AV_CODEC_ID_NONE ? 
config->guessed_video_codec_id : AV_CODEC_ID_H264;
+                break;
+            default:
+                break;
+            }
+            report_config_error(config->filename, line_num, AV_LOG_ERROR, NULL,
+                                "If '%s' is codec private option, then prefix it 
with codec name, "

is a

Rest looks OK at a first glance. Thanks a lot

Bests,


--
Reynaldo H. Verdejo Pinochet
Open Source Group
Samsung Research America / Silicon Valley
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to