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

[Michael Niedermayer]

Hi

On Mon, Jul 30, 2007 at 04:15:31PM +0200, Nicolas George wrote:
> 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.
[...]
> -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) {

if(!arg)


[...]
> +static int find_codec_or_die(const char *name, int type, int encoder)

the return type should be enum CodecID


> +{
> +    AVCodec *codec;
> +
> +    if(name == NULL)
> +        return(CODEC_ID_NONE);

if(!name)
    return CODEC_ID_NONE;


[...]
> @@ -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)

the != NULL is superflous


> +            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);
>  }

i think setting video_stream_copy and video_codec_name here is clearer than
a call to opt_video_codec() also its a more minimalistic change relative to
the current code


[...]
> @@ -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;

!= NULL is superflous

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I hate to see young programmers poisoned by the kind of thinking
Ulrich Drepper puts forward since it is simply too narrow -- Roman Shaposhnik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070805/71243456/attachment.pgp>