diff mbox series

[FFmpeg-devel] New Ticket #9006

Message ID CAF2oTtsaf2+gbCeSrUmdqJ7C3LTUh3iOEEwgV7Esv9xyRXoi2w@mail.gmail.com
State New
Headers show
Series [FFmpeg-devel] New Ticket #9006 | expand

Checks

Context Check Description
andriy/x86_make_warn warning New warnings during build
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Deep Thought Nov. 23, 2020, 11:17 p.m. UTC 
I have opened a ticket explaining a problem with the code for
merge_pmt_versions.

Attached is also a patch which fixes this problem. The main idea is the
following:


for the INITIAL pmt, the old code finds matching streams by
1) checking the stream_identifier. If the current stream has one,
and if an earlier processed stream has the same one, it is assumed
that the two streams should be merged (I am not sure what this really
means, but the result is playback problems).

This behavior is wrong for streams having multiple audio streams with the
same stream_identifier

2) for streams without a stream identifier, it will look for a stream which
has the same position in the pmt as the current stream and will obviously
not find one. This is correct

So the patch solves the problem in 1) as follows:
1) if a stream has a stream identifier, it first computes a new variable
pmt_stream_index which will equal for the first stream with that
identifier, 1 for the second one with that identifier and so on
2) if a stream has no stream_identifier, pmt_stream_index will equal 0 for
the first such stream, 1 for the second such stream ... This is the same
as 1) but with stream_identifier==-1

find_matching_stream then compares streams based on a DUAL criterion:
pmt_stream_index must match, and stream_type must match.

The result is that for the INITIAL pmt all audi streams will be considered
different. If later the PMT changes, streams will be matched based on
stream identifier as the primary criterion and based on pmt_stream_index as
the secondary criterion.

Comments

Marton Balint Nov. 24, 2020, 10:03 p.m. UTC | #1 
On Tue, 24 Nov 2020, Deep Thought wrote:

> I have opened a ticket explaining a problem with the code for
> merge_pmt_versions.
>
> Attached is also a patch which fixes this problem. The main idea is the
> following:
>
>
> for the INITIAL pmt, the old code finds matching streams by
> 1) checking the stream_identifier. If the current stream has one,
> and if an earlier processed stream has the same one, it is assumed
> that the two streams should be merged (I am not sure what this really
> means, but the result is playback problems).
>
> This behavior is wrong for streams having multiple audio streams with the
> same stream_identifier
>
> 2) for streams without a stream identifier, it will look for a stream which
> has the same position in the pmt as the current stream and will obviously
> not find one. This is correct
>
> So the patch solves the problem in 1) as follows:
> 1) if a stream has a stream identifier, it first computes a new variable
> pmt_stream_index which will equal for the first stream with that
> identifier, 1 for the second one with that identifier and so on
> 2) if a stream has no stream_identifier, pmt_stream_index will equal 0 for
> the first such stream, 1 for the second such stream ... This is the same
> as 1) but with stream_identifier==-1
>
> find_matching_stream then compares streams based on a DUAL criterion:
> pmt_stream_index must match, and stream_type must match.
>
> The result is that for the INITIAL pmt all audi streams will be considered
> different. If later the PMT changes, streams will be matched based on
> stream identifier as the primary criterion and based on pmt_stream_index as
> the secondary criterion.

It looks complicated and your patch was using tabs instead of spaces. I 
have attached a patch (untested) which should resolve this in a more 
starightforward way. Can you please test and comment?

Thanks,
Marton
diff mbox series

Patch

diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
index a2003c6632..c4f1377d97 100644
--- a/libavformat/mpegts.c
+++ b/libavformat/mpegts.c
@@ -2200,6 +2200,22 @@  int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type
     return 0;
 }
 
+
+#define MAX_NUM_STREAMS (32)
+
+
+static char compute_pmt_stream_idx(char stream_identifier,
+																	 char stream_identifiers[MAX_NUM_STREAMS], int num_stream_identifiers)
+{
+	int i;
+	int pmt_stream_idx =0;
+	for(i=0;i <  num_stream_identifiers && i < MAX_NUM_STREAMS; ++i)
+		if(stream_identifiers[i]== stream_identifier)
+			pmt_stream_idx++;
+	return pmt_stream_idx;
+
+}
+
 static AVStream *find_matching_stream(MpegTSContext *ts, int pid, unsigned int programid,
                                       int stream_identifier, int pmt_stream_idx)
 {
@@ -2210,17 +2226,14 @@  static AVStream *find_matching_stream(MpegTSContext *ts, int pid, unsigned int p
     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;
-                break;
-            }
-        } else if (st->pmt_stream_idx == pmt_stream_idx) { /* match based on position within the PMT */
-            found = st;
-            break;
-        }
-    }
+					continue;
+				if (st->stream_identifier == stream_identifier+1
+						&& st->pmt_stream_idx == pmt_stream_idx) {
+					found = st;
+					break;
+				}
+		}
+
 
     if (found) {
         av_log(ts->stream, AV_LOG_VERBOSE,
@@ -2360,6 +2373,8 @@  static void pmt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len
 
     set_pmt_found(ts, h->id);
 
+		char stream_identifiers[MAX_NUM_STREAMS];
+		int pmt_stream_idx=-1;
 
     for (i = 0; ; i++) {
         st = 0;
@@ -2374,14 +2389,18 @@  static void pmt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len
         if (pid == ts->current_pid)
             goto out;
 
-        if (ts->merge_pmt_versions)
-            stream_identifier = parse_stream_identifier_desc(p, p_end);
+        if (ts->merge_pmt_versions) {
+	         stream_identifier = parse_stream_identifier_desc(p, p_end);
+          	pmt_stream_idx = compute_pmt_stream_idx(stream_identifier,stream_identifiers, i);
+					if(i< MAX_NUM_STREAMS)
+						stream_identifiers[i] =  stream_identifier;
+				}
 
         /* now create stream */
         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, h->id, stream_identifier, i);
+                st = find_matching_stream(ts, pid, h->id, stream_identifier, pmt_stream_idx);
                 if (st) {
                     pes->st = st;
                     pes->stream_type = stream_type;
@@ -2395,7 +2414,7 @@  static void pmt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len
                 pes->st->id = pes->pid;
                 pes->st->program_num = h->id;
                 pes->st->pmt_version = h->version;
-                pes->st->pmt_stream_idx = i;
+                pes->st->pmt_stream_idx = pmt_stream_idx;
             }
             st = pes->st;
         } else if (is_pes_stream(stream_type, prog_reg_desc)) {
@@ -2403,8 +2422,8 @@  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, h->id, stream_identifier, i);
-                if (st) {
+							st = find_matching_stream(ts, pid, h->id, stream_identifier, pmt_stream_idx);
+							if (st) {
                     pes->st = st;
                     pes->stream_type = stream_type;
                     pes->merged_st = 1;
@@ -2417,7 +2436,7 @@  static void pmt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len
                 st->id = pes->pid;
                 st->program_num = h->id;
                 st->pmt_version = h->version;
-                st->pmt_stream_idx = i;
+								st->pmt_stream_idx = pmt_stream_idx;
             }
         } else {
             int idx = ff_find_stream_index(ts->stream, pid);
@@ -2425,7 +2444,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, h->id, stream_identifier, i);
+							st = find_matching_stream(ts, pid, h->id, stream_identifier, pmt_stream_idx);
             }
             if (!st) {
                 st = avformat_new_stream(ts->stream, NULL);
@@ -2434,7 +2453,7 @@  static void pmt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len
                 st->id = pid;
                 st->program_num = h->id;
                 st->pmt_version = h->version;
-                st->pmt_stream_idx = i;
+								st->pmt_stream_idx = pmt_stream_idx;
                 st->codecpar->codec_type = AVMEDIA_TYPE_DATA;
                 if (stream_type == 0x86 && prog_reg_desc == AV_RL32("CUEI")) {
                     mpegts_find_stream_type(st, stream_type, SCTE_types);