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

[Stefano Sabatini]

On date Monday 2010-06-14 15:03:12 -0400, Mike Scheutzow encoded:
> Both patches updated.
> 
> Stefano Sabatini wrote:
> >>+/* arg format is "output-stream-index:streamid-value". */
> >>+static void opt_streamid(const char *arg)
> >
> >void opt_streamid(const char *opt, const char *arg)
> >
> >then you can use opt in the error messages, rather than hardcode them.
> 
> Changed.
> 
> >>+{
> >>+    int idx, value;
> >>+    char *p;
> >>+
> >>+    idx = strtol(arg, &p, 0);
> >
> >parse_number_or_die()
> 
> I can't use this function for the first field. The ":" field
> separator will cause the function to exit the app.
> 
> >>+    if (*p != ':' || idx < 0 || idx >= MAX_STREAMS) {
> >>+        fprintf(stderr, "Invalid -streamid: '%s'\n", arg);
> >>+        av_exit(1);
> >>+    }
> >>+
> >>+    p++;
> >>+    value = strtol(p, &p, 0);
> >>+    if (*p || value < 0) {
> >>+        fprintf(stderr, "Invalid -streamid: '%s'\n", arg);
> >>+        av_exit(1);
> >>+    }
> >
> >ditto
> 
> Changed.
> 
> >>+ at item -streamid @var{output-stream-index}:@var{new-value}
> >>+Assign a new value to a stream's stream-id field in the next output file. All stream-id fields are reset to default for each output file.
> >>+
> >>+For example, to set the stream 0 PID to 33 and the stream 1 PID to 36 for an output mpegts file:
> >
> >Break long lines
> 
> Changed.
> 
> >>+            av_log(s, AV_LOG_ERROR, "error, stream id must be 16 to 8190\n");
> >
> >"Invalid stream id %d, must be in the range..."
> 
> Changed, message simplified.
> 
> >>+                av_log(s, AV_LOG_ERROR, "error, stream %d id conflicts with "
> >>+                       "stream %d (%d)\n", i, j, st->id);
> >
> >Confusing message.
> 
> Changed, message simplified.
> 
> >All these "error, " are ugly, just remove them.
> 
> Done.
> 
> 
> Suggested commit description for ffmpeg_set_streamid_v4.patch:
> 
> for ffmpeg.c, add command line option -streamid to set the value of
> AVStream.id for output streams.
> Patch by Mike Scheutzow mjs973 optonline net
> 
> Suggested commit description for mpegts_custom_pids_v6.patch:
> 
> for mpegts muxer, allow libav application to assign a custom pid
> value to an output stream.
> Patch by Mike Scheutzow mjs973 optonline net
> 
> 
> Mike Scheutzow
> 

> 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".

Rest of the patch looks fine to me.

Thanks, regards.
-- 
FFmpeg = Fascinating Fast Marvellous Powered Elected Guide