[FFmpeg-devel] [PATCH] options to set stream id (custom TS pid)

[Stefano Sabatini]

On date Wednesday 2010-06-16 16:22:21 -0400, Mike Scheutzow encoded:
> Stefano Sabatini wrote:
> >>Index: ffmpeg.c
> >[...]
> >>+/* arg format is "output-stream-index:streamid-value". */
> >>+static void opt_streamid(const char *opt, const char *arg)
> >>+{
> >>+    int idx;
> >>+    char *p;
> >>+
> >>+    idx = strtol(arg, &p, 0);
> >>+    if (*p != ':' || idx < 0 || idx >= MAX_STREAMS) {
> >>+        fprintf(stderr, "Invalid %s '%s'\n", opt, arg);
> >>+        av_exit(1);
> >>+    }
> >
> >I'm still a bit unstatisfied about this, this will accept invalid
> >string, maybe you could pre-parse the string before. Anyway please use
> >a explicative message, for example:
> >"Invalid value '%s' for option '%s', not a number or not in the range %d - %d".
> 
> Implemented the pre-parsing in ffmpeg_set_streamid_v5.patch, and retested.
> 
> >Rest of the patch looks fine to me.
> 
> 
> Mike Scheutzow

> Index: ffmpeg.c
> ===================================================================
> --- ffmpeg.c	(revision 23373)
> +++ ffmpeg.c	(working copy)
> @@ -121,6 +121,9 @@
>  static AVMetaDataMap meta_data_maps[MAX_FILES];
>  static int nb_meta_data_maps;
>  
> +/* indexed by output file stream index */
> +static int streamid_map[MAX_STREAMS];
> +
>  static int frame_width  = 0;
>  static int frame_height = 0;
>  static float frame_aspect_ratio = 0;
> @@ -3326,7 +3329,7 @@
>      AVCodecContext *video_enc;
>      enum CodecID codec_id;
>  
> -    st = av_new_stream(oc, oc->nb_streams);
> +    st = av_new_stream(oc, streamid_map[oc->nb_streams]);
>      if (!st) {
>          fprintf(stderr, "Could not alloc stream\n");
>          av_exit(1);
> @@ -3463,7 +3466,7 @@
>      AVCodecContext *audio_enc;
>      enum CodecID codec_id;
>  
> -    st = av_new_stream(oc, oc->nb_streams);
> +    st = av_new_stream(oc, streamid_map[oc->nb_streams]);
>      if (!st) {
>          fprintf(stderr, "Could not alloc stream\n");
>          av_exit(1);
> @@ -3535,7 +3538,7 @@
>      AVStream *st;
>      AVCodecContext *subtitle_enc;
>  
> -    st = av_new_stream(oc, oc->nb_streams);
> +    st = av_new_stream(oc, streamid_map[oc->nb_streams]);
>      if (!st) {
>          fprintf(stderr, "Could not alloc stream\n");
>          av_exit(1);
> @@ -3603,6 +3606,27 @@
>      new_subtitle_stream(oc);
>  }
>  
> +/* arg format is "output-stream-index:streamid-value". */
> +static void opt_streamid(const char *opt, const char *arg)
> +{
> +    int idx;
> +    char *p;
> +    char idx_str[16];
> +
> +    strncpy(idx_str, arg, sizeof(idx_str));
> +    idx_str[sizeof(idx_str)-1] = '\0';
> +    p = strchr(idx_str, ':');
> +    if (!p) {
> +        fprintf(stderr,
> +                "Invalid value '%s' for option '%s', required syntax is 'index:value'\n",
> +                arg, opt);
> +        av_exit(1);
> +    }
> +    *p++ = '\0';
> +    idx = parse_number_or_die(opt, idx_str, OPT_INT, 0, MAX_STREAMS-1);
> +    streamid_map[idx] = parse_number_or_die(opt, p, OPT_INT, 0, INT_MAX);
> +}

Looks fine to me, just I prefer a simple 0 to '\0'. If Michael doesn't
give his explicit OK I'll apply the patch in few days.

Thanks, regards.
-- 
FFmpeg = Frightening and Fantastic MultiPurpose Experimenting Guru