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

[Baptiste Coudurier]

Hi Mike,

On 05/29/2010 12:53 PM, Mike Scheutzow wrote:
> Baptiste Coudurier wrote:
>> All these suggestions are great, a patch implementing what you suggest
>> would be great.
>>
> Here are two patches that, together, allow a user to set mpegts PID
> values from the command line. The syntax looks like:
>
> ffmpeg -i infile -streamid 0:33 -streamid 1:36 out.ts
>
> The first patch, ffmpeg_set_streamid_v1.patch, adds general support to
> ffmpeg.c to set the AVStream.id field from the command line. Backward
> compatibility for .id values is preserved. The patch also includes a
> documentation update for the ffmpeg-doc.texi file.
>
> The second patch, mpegts_custom_pids_v2.patch, adds support for the
> mpegts mux to use AVStream.id as the stream's PID value. One line
> differs slightly from Baptiste's original in order to provide better
> backward compatibility (discussed earlier in this thread.)
>
> Please let me know what you think.
>
> thanks,
>
>
> mpegts_custom_pids_v2.patch
>
>
> Index: libavformat/mpegtsenc.c
> ===================================================================
> --- libavformat/mpegtsenc.c	(revision 23373)
> +++ libavformat/mpegtsenc.c	(working copy)
 >
> [...]
 >
> @@ -420,6 +425,25 @@
>           st->priv_data = ts_st;
>           ts_st->service = service;
>           ts_st->pid = DEFAULT_START_PID + i;
> +        if (st->id>= 16) {
> +            if (st->id == service->pmt.pid) {
> +                av_log(s, AV_LOG_ERROR, "error, stream id conflicts with PMT (%d)\n",
> +                       service->pmt.pid);
> +                goto fail;
> +            } else if (st->id<  16 || st->id == 0x1FFF) {

The < 16 check is useless.

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

Well, I'm not sure what we want to do

avformat.h:
  * @param id file-format-dependent stream ID

I don't know why we want to set it to stream index here.
IMHO I would set it to 0 assuming it has no side effect.
I quickly check before the last patch and I didn't see st->id usage, but 
I could have missed it.
No strong feeling, initing streamid_map just for this seems a waste.

> [...]
 >
> Index: doc/ffmpeg-doc.texi
> ===================================================================
> --- doc/ffmpeg-doc.texi	(revision 23373)
> +++ doc/ffmpeg-doc.texi	(working copy)
> @@ -783,6 +783,13 @@
>   Set the maximum demux-decode delay.
>   @item -muxpreload @var{seconds}
>   Set the initial demux-decode delay.
> + 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:
> + at example
> +ffmpeg -i in.ts -streamid 0:33 -streamid 1:36 out.ts

ffmpeg -i in.ts -vcodec copy -streamid 0:33 -streamd 1:36 -acodec mp2 
out1.ts -vcodec copy -acodec ac3 out2.ts

Would also set out2 stream ids, it might not be wanted, not sure,
Patch looks good otherwise.

-- 
Baptiste COUDURIER
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org