diff mbox

[FFmpeg-devel] avformat/mpegts: respect program number when merging streams

Message ID 20190709210723.29028-1-cus@passwd.hu
State Accepted
Commit 81d3d7dd44acc7ae7c57e99176d1d428fb40c353
Headers show

Commit Message

Marton Balint July 9, 2019, 9:07 p.m. UTC
merge_pmt_versions was not usable if multiple programs were present because
when it was searching for candidate streams it did not make sure that the PMT was
of the same program. This caused the streams of all programs to get merged into
a single (garbled) program.

This patch makes sure that the program number (service ID) is also matching
with the old streams when parsing the PMT making the feature useful for multi
program streams.

This change might cause issues for single program streams if the program number
changes as well, but I think it is acceptable because the goal of the option is
to make the parsing resilient to PID changes, and that is still working as
expected.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/mpegts.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Aman Karmani July 9, 2019, 11:39 p.m. UTC | #1
On Tue, Jul 9, 2019 at 2:07 PM Marton Balint <cus@passwd.hu> wrote:

> merge_pmt_versions was not usable if multiple programs were present because
> when it was searching for candidate streams it did not make sure that the
> PMT was
> of the same program. This caused the streams of all programs to get merged
> into
> a single (garbled) program.
>
> This patch makes sure that the program number (service ID) is also matching
> with the old streams when parsing the PMT making the feature useful for
> multi
> program streams.
>
> This change might cause issues for single program streams if the program
> number
> changes as well, but I think it is acceptable because the goal of the
> option is
> to make the parsing resilient to PID changes, and that is still working as
> expected.
>

Patch looks good to me. Thanks for catching this!

Aman


>
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavformat/mpegts.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> index 8a84e5cc19..47d8d5f877 100644
> --- a/libavformat/mpegts.c
> +++ b/libavformat/mpegts.c
> @@ -2130,7 +2130,7 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc,
> AVStream *st, int stream_type
>      return 0;
>  }
>
> -static AVStream *find_matching_stream(MpegTSContext *ts, int pid,
> +static AVStream *find_matching_stream(MpegTSContext *ts, int pid,
> unsigned int programid,
>                                        int stream_identifier, int
> pmt_stream_idx)
>  {
>      AVFormatContext *s = ts->stream;
> @@ -2139,6 +2139,8 @@ static AVStream *find_matching_stream(MpegTSContext
> *ts, int pid,
>
>      for (i = 0; i < s->nb_streams; i++) {
>          AVStream *st = s->streams[i];
> +        if (st->program_num != programid)
> +            continue;
>          if (stream_identifier != -1) { /* match based on "stream
> identifier descriptor" if present */
>              if (st->stream_identifier == stream_identifier+1) {
>                  found = st;
> @@ -2309,7 +2311,7 @@ static void pmt_cb(MpegTSFilter *filter, const
> uint8_t *section, int section_len
>          if (ts->pids[pid] && ts->pids[pid]->type == MPEGTS_PES) {
>              pes = ts->pids[pid]->u.pes_filter.opaque;
>              if (ts->merge_pmt_versions && !pes->st) {
> -                st = find_matching_stream(ts, pid, stream_identifier, i);
> +                st = find_matching_stream(ts, pid, h->id,
> stream_identifier, i);
>                  if (st) {
>                      pes->st = st;
>                      pes->stream_type = stream_type;
> @@ -2331,7 +2333,7 @@ static void pmt_cb(MpegTSFilter *filter, const
> uint8_t *section, int section_len
>                  mpegts_close_filter(ts, ts->pids[pid]); // wrongly added
> sdt filter probably
>              pes = add_pes_stream(ts, pid, pcr_pid);
>              if (ts->merge_pmt_versions && pes && !pes->st) {
> -                st = find_matching_stream(ts, pid, stream_identifier, i);
> +                st = find_matching_stream(ts, pid, h->id,
> stream_identifier, i);
>                  if (st) {
>                      pes->st = st;
>                      pes->stream_type = stream_type;
> @@ -2353,7 +2355,7 @@ static void pmt_cb(MpegTSFilter *filter, const
> uint8_t *section, int section_len
>                  st = ts->stream->streams[idx];
>              }
>              if (ts->merge_pmt_versions && !st) {
> -                st = find_matching_stream(ts, pid, stream_identifier, i);
> +                st = find_matching_stream(ts, pid, h->id,
> stream_identifier, i);
>              }
>              if (!st) {
>                  st = avformat_new_stream(ts->stream, NULL);
> --
> 2.16.4
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Marton Balint July 12, 2019, 7:33 p.m. UTC | #2
On Tue, 9 Jul 2019, Aman Gupta wrote:

> On Tue, Jul 9, 2019 at 2:07 PM Marton Balint <cus@passwd.hu> wrote:
>
>> merge_pmt_versions was not usable if multiple programs were present because
>> when it was searching for candidate streams it did not make sure that the
>> PMT was
>> of the same program. This caused the streams of all programs to get merged
>> into
>> a single (garbled) program.
>>
>> This patch makes sure that the program number (service ID) is also matching
>> with the old streams when parsing the PMT making the feature useful for
>> multi
>> program streams.
>>
>> This change might cause issues for single program streams if the program
>> number
>> changes as well, but I think it is acceptable because the goal of the
>> option is
>> to make the parsing resilient to PID changes, and that is still working as
>> expected.
>>
>
> Patch looks good to me. Thanks for catching this!

Thanks, applied.

Regards,
Marton
diff mbox

Patch

diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
index 8a84e5cc19..47d8d5f877 100644
--- a/libavformat/mpegts.c
+++ b/libavformat/mpegts.c
@@ -2130,7 +2130,7 @@  int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type
     return 0;
 }
 
-static AVStream *find_matching_stream(MpegTSContext *ts, int pid,
+static AVStream *find_matching_stream(MpegTSContext *ts, int pid, unsigned int programid,
                                       int stream_identifier, int pmt_stream_idx)
 {
     AVFormatContext *s = ts->stream;
@@ -2139,6 +2139,8 @@  static AVStream *find_matching_stream(MpegTSContext *ts, int pid,
 
     for (i = 0; i < s->nb_streams; i++) {
         AVStream *st = s->streams[i];
+        if (st->program_num != programid)
+            continue;
         if (stream_identifier != -1) { /* match based on "stream identifier descriptor" if present */
             if (st->stream_identifier == stream_identifier+1) {
                 found = st;
@@ -2309,7 +2311,7 @@  static void pmt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len
         if (ts->pids[pid] && ts->pids[pid]->type == MPEGTS_PES) {
             pes = ts->pids[pid]->u.pes_filter.opaque;
             if (ts->merge_pmt_versions && !pes->st) {
-                st = find_matching_stream(ts, pid, stream_identifier, i);
+                st = find_matching_stream(ts, pid, h->id, stream_identifier, i);
                 if (st) {
                     pes->st = st;
                     pes->stream_type = stream_type;
@@ -2331,7 +2333,7 @@  static void pmt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len
                 mpegts_close_filter(ts, ts->pids[pid]); // wrongly added sdt filter probably
             pes = add_pes_stream(ts, pid, pcr_pid);
             if (ts->merge_pmt_versions && pes && !pes->st) {
-                st = find_matching_stream(ts, pid, stream_identifier, i);
+                st = find_matching_stream(ts, pid, h->id, stream_identifier, i);
                 if (st) {
                     pes->st = st;
                     pes->stream_type = stream_type;
@@ -2353,7 +2355,7 @@  static void pmt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len
                 st = ts->stream->streams[idx];
             }
             if (ts->merge_pmt_versions && !st) {
-                st = find_matching_stream(ts, pid, stream_identifier, i);
+                st = find_matching_stream(ts, pid, h->id, stream_identifier, i);
             }
             if (!st) {
                 st = avformat_new_stream(ts->stream, NULL);