[FFmpeg-devel] [PATCH] Codec lookup: do not use codec_id

Nicolas George nicolas.george
Mon Jul 30 16:15:31 CEST 2007


L'octidi 8 thermidor, an CCXV, Michael Niedermayer a ?crit?:
> Hi

Hi.

I am sorry, I do not understand how I can have lost your reply.

> first part changes the global codec_id to strings, the second part changes
> ost->st->codec->codec_id to ost->st->codec->codec, these are 2 seperate
> changes and belong in seperate patches

This makes sense. Here is an updated patch. It passed the same series of
tests than the last one.

> whatever this is supposed to do, it doesnt do it so it is wrong

You are right. I did put it, experienced segfaults, disabled it, understood
why it should actually not be there, and then forgot to remove it.

For those who are interested, the reason why it must not be freed: a valid
command line can have several -acodec options (or -vcodec), and the values
are all stored in structures, and used after the command line parsing is
done.


Suggested log message:
Use the codec name instead of the codec id whenever possible.

Index: ffmpeg.c
===================================================================
--- ffmpeg.c	(revision 9832)
+++ ffmpeg.c	(working copy)
@@ -128,7 +128,7 @@
 static char *video_rc_eq="tex^qComp";
 static int video_disable = 0;
 static int video_discard = 0;
-static int video_codec_id = CODEC_ID_NONE;
+static char *video_codec_name = NULL;
 static int video_codec_tag = 0;
 static int same_quality = 0;
 static int do_deinterlace = 0;
@@ -146,12 +146,12 @@
 static float audio_qscale = QSCALE_NONE;
 static int audio_disable = 0;
 static int audio_channels = 1;
-static int audio_codec_id = CODEC_ID_NONE;
+static char  *audio_codec_name = NULL;
 static int audio_codec_tag = 0;
 static char *audio_language = NULL;
 
 static int subtitle_disable = 0;
-static int subtitle_codec_id = CODEC_ID_NONE;
+static char *subtitle_codec_name = NULL;
 static char *subtitle_language = NULL;
 
 static float mux_preload= 0.5;
@@ -2356,32 +2356,23 @@
     video_standard = av_strdup(arg);
 }
 
-static void opt_codec(int *pstream_copy, int *pcodec_id,
+static void opt_codec(int *pstream_copy, char **pcodec_name,
                       int codec_type, const char *arg)
 {
-    AVCodec *p;
-
-    if (!strcmp(arg, "copy")) {
+    *pstream_copy = 0;
+    *pcodec_name = NULL;
+    if(arg == NULL) {
+        /* nothing */
+    } else if (!strcmp(arg, "copy")) {
         *pstream_copy = 1;
     } else {
-        p = first_avcodec;
-        while (p) {
-            if (!strcmp(p->name, arg) && p->type == codec_type)
-                break;
-            p = p->next;
-        }
-        if (p == NULL) {
-            fprintf(stderr, "Unknown codec '%s'\n", arg);
-            exit(1);
-        } else {
-            *pcodec_id = p->id;
-        }
+        *pcodec_name = av_strdup(arg);
     }
 }
 
 static void opt_audio_codec(const char *arg)
 {
-    opt_codec(&audio_stream_copy, &audio_codec_id, CODEC_TYPE_AUDIO, arg);
+    opt_codec(&audio_stream_copy, &audio_codec_name, CODEC_TYPE_AUDIO, arg);
 }
 
 static void opt_audio_tag(const char *arg)
@@ -2427,12 +2418,12 @@
 
 static void opt_video_codec(const char *arg)
 {
-    opt_codec(&video_stream_copy, &video_codec_id, CODEC_TYPE_VIDEO, arg);
+    opt_codec(&video_stream_copy, &video_codec_name, CODEC_TYPE_VIDEO, arg);
 }
 
 static void opt_subtitle_codec(const char *arg)
 {
-    opt_codec(&subtitle_stream_copy, &subtitle_codec_id, CODEC_TYPE_SUBTITLE, arg);
+    opt_codec(&subtitle_stream_copy, &subtitle_codec_name, CODEC_TYPE_SUBTITLE, arg);
 }
 
 static void opt_map(const char *arg)
@@ -2495,6 +2486,26 @@
     input_ts_offset = parse_date(arg, 1);
 }
 
+static int find_codec_or_die(const char *name, int type, int encoder)
+{
+    AVCodec *codec;
+
+    if(name == NULL)
+        return(CODEC_ID_NONE);
+    codec = encoder ?
+        avcodec_find_encoder_by_name(name) :
+        avcodec_find_decoder_by_name(name);
+    if(codec == NULL) {
+        av_log(NULL, AV_LOG_ERROR, "Unknown codec '%s'\n", name);
+        exit(1);
+    }
+    if(codec->type != type) {
+        av_log(NULL, AV_LOG_ERROR, "Invalid codec type '%s'\n", name);
+        exit(1);
+    }
+    return codec->id;
+}
+
 static void opt_input_file(const char *filename)
 {
     AVFormatContext *ic;
@@ -2522,8 +2533,8 @@
     ap->pix_fmt = frame_pix_fmt;
     ap->channel = video_channel;
     ap->standard = video_standard;
-    ap->video_codec_id = video_codec_id;
-    ap->audio_codec_id = audio_codec_id;
+    ap->video_codec_id = find_codec_or_die(video_codec_name, CODEC_TYPE_VIDEO, 0);
+    ap->audio_codec_id = find_codec_or_die(audio_codec_name, CODEC_TYPE_AUDIO, 0);
     if(pgmyuv_compatibility_hack)
         ap->video_codec_id= CODEC_ID_PGMYUV;
 
@@ -2728,8 +2739,8 @@
         AVCodec *codec;
 
         codec_id = av_guess_codec(oc->oformat, NULL, oc->filename, NULL, CODEC_TYPE_VIDEO);
-        if (video_codec_id != CODEC_ID_NONE)
-            codec_id = video_codec_id;
+        if (video_codec_name != NULL)
+            codec_id = find_codec_or_die(video_codec_name, CODEC_TYPE_VIDEO, 1);
 
         video_enc->codec_id = codec_id;
         codec = avcodec_find_encoder(codec_id);
@@ -2837,8 +2848,7 @@
 
     /* reset some key parameters */
     video_disable = 0;
-    video_codec_id = CODEC_ID_NONE;
-    video_stream_copy = 0;
+    opt_video_codec(NULL);
 }
 
 static void new_audio_stream(AVFormatContext *oc)
@@ -2884,8 +2894,8 @@
                 av_set_double(audio_enc, opt_names[i], d);
         }
 
-        if (audio_codec_id != CODEC_ID_NONE)
-            codec_id = audio_codec_id;
+        if (audio_codec_name != NULL)
+            codec_id = find_codec_or_die(audio_codec_name, CODEC_TYPE_AUDIO, 1);
         audio_enc->codec_id = codec_id;
 
         if (audio_qscale > QSCALE_NONE) {
@@ -2905,8 +2915,7 @@
 
     /* reset some key parameters */
     audio_disable = 0;
-    audio_codec_id = CODEC_ID_NONE;
-    audio_stream_copy = 0;
+    opt_audio_codec(NULL);
 }
 
 static void new_subtitle_stream(AVFormatContext *oc)
@@ -2933,7 +2942,7 @@
              if(d==d && (opt->flags&AV_OPT_FLAG_SUBTITLE_PARAM) && (opt->flags&AV_OPT_FLAG_ENCODING_PARAM))
                  av_set_double(subtitle_enc, opt_names[i], d);
         }
-        subtitle_enc->codec_id = subtitle_codec_id;
+        subtitle_enc->codec_id = find_codec_or_die(subtitle_codec_name, CODEC_TYPE_SUBTITLE, 1);
     }
 
     if (subtitle_language) {
@@ -2943,8 +2952,7 @@
     }
 
     subtitle_disable = 0;
-    subtitle_codec_id = CODEC_ID_NONE;
-    subtitle_stream_copy = 0;
+    opt_subtitle_codec(NULL);
 }
 
 static void opt_new_audio_stream(void)
@@ -3013,9 +3021,9 @@
             exit(1);
         }
     } else {
-        use_video = file_oformat->video_codec != CODEC_ID_NONE || video_stream_copy || video_codec_id != CODEC_ID_NONE;
-        use_audio = file_oformat->audio_codec != CODEC_ID_NONE || audio_stream_copy || audio_codec_id != CODEC_ID_NONE;
-        use_subtitle = file_oformat->subtitle_codec != CODEC_ID_NONE || subtitle_stream_copy || subtitle_codec_id != CODEC_ID_NONE;
+        use_video = file_oformat->video_codec != CODEC_ID_NONE || video_stream_copy || video_codec_name != NULL;
+        use_audio = file_oformat->audio_codec != CODEC_ID_NONE || audio_stream_copy || audio_codec_name != NULL;
+        use_subtitle = file_oformat->subtitle_codec != CODEC_ID_NONE || subtitle_stream_copy || subtitle_codec_name != NULL;
 
         /* disable if no corresponding type found and at least one
            input file */
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 185 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070730/3b9a9a04/attachment.pgp>



More information about the ffmpeg-devel mailing list