[FFmpeg-devel] [PATCH 1/4] Audio Video pid number pass through

[Nicolas George]

Le sextidi 26 messidor, an CCXX, Sailaja Mahendrakar a écrit :
> Ffmpeg currently puts out static pid numbers for the audio and video pids
> in transcoded TS output.
> 
> In the case of transcoding/trans-rating with transport Streams/files,
> passing through the pid numbers from input to output  will preserve the
> source information than having static pid numbers.
> 
> This feature passes down the pid numbers for audio and video from input to
> output.

Thanks for the work. I do not know MPEG-TS enough to comment on the actual
changes.

> Current email is attached with changes related to ffmpeg.c.

Changes should be self-contained, so that, for example, applying the first
one only (while the next one is somewhat reworked for example) will give
something that works. If all your changes are a single logical unit, just
send them together.

> diff --git a/ffmpeg.c b/ffmpeg.c

It looks like you used git diff on the individual files. This is not the
optimal way of using git. You should commit your changes, with a message
looking like this: "compnent: short description", then possibly an empty
line and a longer description (with explicit line breaks at ~72 chars). Do
not hesitate to imitate the style of other commit messages.

If your changes can be separated into several logical units, make one commit
by unit. I suggest you create a branch before that (checkout -b). If you
need to rearange your patches (for example squash a small fixup to an
earlier one), use "git rebase -i master". Do that also if some time has
passed and you need to update your patches for the new versions.

When you are done, use git format-patch to create patches files for each of
your commits; or git send-email to do that and send them directly.

> old mode 100644
> new mode 100755

I do not think that should be part of the patch.

> index cf53b91..6e23577
> --- a/ffmpeg.c
> +++ b/ffmpeg.c
> @@ -2850,7 +2850,18 @@ static int transcode_init(void)
>              return AVERROR(EINVAL);
>          }
>      }
> -
> +#if CONFIG_AV_PID_NUMBER_PASSTHRU

I do not think this is a good idea to make it a build-time option. It
probably should be a run-time option:

ffmpeg -copy-pid -i a.ts -c copy b.ts

Or maybe (again, I do not know MPEG-TS enough to know if it is a good idea)
enable it always.

> +    // If ingest is TS: transfer pid numbers from input format to output format context
> +    // other than TS: 0x2000 as identification, pid number starts with start_pid i.e 256
> +	for(i=0;i<nb_output_streams;i++) 
> +    {
> +        ost = output_streams[i];
> +        if(!strcmp(input_files[input_streams[ost->source_index]->file_index]->ctx->iformat->name,"mpegts"))
> +           output_files[ost->file_index]->ctx->pids_num[ost->index] = input_files[input_streams[ost->source_index]->file_index]->ctx->pids_num[input_streams[ost->source_index]->st->index];
> +        else
> +           output_files[ost->file_index]->ctx->pids_num[ost->index] = 0x2000;
> +    }

Please try to use a coding-style consistent with the surrounding code. Tabs
are strictly forbidden; use 4 spaces for indentation. As for more cosmetic
details, give some space around operators, and between C keywords and
parentheses. Also, opening braces should go on the same line than the
loop/condition.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120713/ae2f813b/attachment.asc>