Ticket #1552 (closed defect: fixed)

Opened 1 month ago

Last modified 1 month ago

Regression: SAP sends all announcements to live555

Reported by: md Assigned to:
Priority: normal Milestone: 0.8.6-bugfix
Component: Services discovery Version:
Severity: normal Keywords: patch
Cc: md, xtophe, courmisch Platform(s): all
Difficulty: unknown Work status: 80%

Description

After upgrading to 0.8.6f, VLC is no longer able to parse SAP announcements for MPEG2 TS. Instead, it sends everything to live555.

Trac is broken - does not allow to browse 0.8.6-bugfix tree so I'm not able to investigate which commit broke this.

Change History

04/11/08 16:46:20 changed by md

  • milestone set to 0.8.6-bugfix.

04/12/08 14:36:05 changed by pdherbemont

Please use http://git.videolan.org to browse the branch.

Also, note that when you clone the git tree you can browse the full history locally with git gui, or qgit, or whatever.

(follow-up: ↓ 4 ) 04/12/08 20:20:50 changed by xtophe

And then, does it fails to play ? or is it just using live555 instead of the udp module ?

People at centrale have test with win and gentoo and reported it works. i need to ask them to check which module was used to parse the sap

(in reply to: ↑ 3 ; follow-up: ↓ 6 ) 04/12/08 20:55:30 changed by md

Replying to xtophe:

And then, does it fails to play ? or is it just using live555 instead of the udp module ?

It plays, but only for limited time period. Depending on the clock drift, after e.g. 30 minutes it starts skipping (buffer underruns). live555 does not use clock synchro adaptation, thus it breaks sooner or later.

On the other hand, native VLC's rtp input can play a stream for days without any clocking drift problems. Thus we definitely need to restore the behaviour from 0.8.6e.

Also git.videolan.org doesn't seem to help - vlc.git is again trunk and there's no project for branch.

04/12/08 21:18:28 changed by md

  • cc changed from md to md, xtophe.

The problem is most probably here (diff between 0.8.6e and 0.8.6f):

@@ -1041,31 +1041,33 @@
         if( psz_eof )
         {
             *psz_eof = '\0';
-            psz_proto = strdup( psz_parse );
-
-            psz_parse = psz_eof + 1;
-            p_sdp->i_media_type = atoi( psz_parse );

+            if( !strcmp( psz_parse, "RTP/AVP" ) )
+            {
+                psz_proto = "rtp";
+                p_sdp->i_media_type = atoi( psz_parse );
+            }

Apparently, the line

psz_parse = psz_eof + 1;

has been incorrectly deleted and now RTP media type is undefined.

(in reply to: ↑ 4 ) 04/13/08 07:52:09 changed by pdherbemont

Replying to md:

Also git.videolan.org doesn't seem to help - vlc.git is again trunk and there's no project for branch.

There is! See the different branch name at the bottom of the page. Here is a link to 0.8.6-bugfix:

http://git.videolan.org/?p=vlc.git;a=shortlog;h=0.8.6-bugfix

Pierre.

04/13/08 09:04:23 changed by md

  • cc changed from md, xtophe to md, xtophe, courmisch.

Oh, I see. So - as mentioned above, the problem was introduced in this commit:

http://git.videolan.org/?p=vlc.git;a=commitdiff;h=29a99a61010ec44c61ca6c1614c116e4436e8b12

04/17/08 00:49:38 changed by xtophe

  • keywords set to patch.
  • wip changed from Not started to 80%.

can you test the following patch. here it seems to fix the problem but testing only with one pc streaming to itself

diff --git a/modules/services_discovery/sap.c b/modules/services_discovery/sap.c
index 797099e..3d664de 100644
--- a/modules/services_discovery/sap.c
+++ b/modules/services_discovery/sap.c

@@ -1045,6 +1045,7 @@ static int ParseConnection( vlc_object_t *p_obj, sdp_t *p$
             if( !strcmp( psz_parse, "RTP/AVP" ) )
             {
                 psz_proto = "rtp";
+                psz_parse = psz_eof + 1;
                 p_sdp->i_media_type = atoi( psz_parse );
             }
             else

04/17/08 09:00:00 changed by md

Thanks xtophe, it completely fixes the problem.

04/17/08 21:05:01 changed by jb

  • status changed from new to closed.
  • resolution set to fixed.

Fixed in 5e015e

04/17/08 21:05:25 changed by jb