After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 707673 - tsdemux: MPTS clock skew issue
tsdemux: MPTS clock skew issue
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
1.0.8
Other All
: Normal normal
: 1.3.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 702152 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2013-09-07 06:31 UTC by Baby octopus
Modified: 2014-06-22 13:09 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch for fixing MPTS PCR correction issue (10.48 KB, text/plain)
2013-09-07 06:31 UTC, Baby octopus
  Details
MPTS clock skew fix (8.80 KB, patch)
2013-12-18 12:39 UTC, Baby octopus
needs-work Details | Review

Description Baby octopus 2013-09-07 06:31:18 UTC
Created attachment 254331 [details]
Patch for fixing MPTS PCR correction issue

Hi,

For for an MPTS stream, to calculate clock skew, PCRs of all the programs used. This is a bug. Have added a patch which will make sure only that program's PCR is used for skew computation

Regards,
Jagadish
Comment 1 Edward Hervey 2013-09-07 10:27:20 UTC
Could you provide the patch as a git patch ? And remove changes (including whitespace ones) that shouldn't be in it ?
Comment 2 Edward Hervey 2013-10-29 10:01:09 UTC
ping ?
Comment 3 Baby octopus 2013-11-11 07:09:23 UTC
My apologies. Completely forgot this. Will send the git compatible patch soon

Thanks,
Jagadish
Comment 4 Baby octopus 2013-12-18 12:39:04 UTC
Created attachment 264474 [details] [review]
MPTS clock skew fix

Sorry for the hibernation :) Have attached the git compliant patch
Comment 5 Sebastian Dröge (slomo) 2014-01-03 09:10:14 UTC
Comment on attachment 264474 [details] [review]
MPTS clock skew fix

Please provide this in "git format-patch" format. And also make it gst-indent clean and the FIX comments look not very nice either :)

Edward, does this otherwise look good?
Comment 6 Edward Hervey 2014-01-06 07:05:58 UTC
I haven't had time to review it yet.

Do *NOT* push this without my approval, thankyou.
Comment 7 Edward Hervey 2014-01-06 07:20:19 UTC
Also ... I'm confused.

Where do you think we use random PCR PID (and not the program PCR PID) for skew calculation ?
Comment 8 Edward Hervey 2014-02-20 15:31:27 UTC
ping ?
Comment 9 Baby octopus 2014-02-26 15:43:48 UTC
Hi,

Let me explain

The function mpegts_packetizer_parse_adaptation_field_control() gets called for for every packet parsing, whether it belongs to my intended program(set through program-number property) or any other program which is present in the MPTS.

So essentially clock skew calculation involves (line:380 in git master) calling of get_pcr_table() which gets PCR of a TS of current program which might not be my intended program which causes wrong skew computation

And sorry about the patch. It might not be GIT compliant. I messed it up while creating it and now I don't know how to apply it to the original 1.0.8 version :(

Regards,
Jagadish
Comment 10 Edward Hervey 2014-02-26 15:59:56 UTC
(In reply to comment #9)
> Hi,
> 
> Let me explain
> 
> The function mpegts_packetizer_parse_adaptation_field_control() gets called for
> for every packet parsing, whether it belongs to my intended program(set through
> program-number property) or any other program which is present in the MPTS.
> 
> So essentially clock skew calculation involves (line:380 in git master) calling
> of get_pcr_table() which gets PCR of a TS of current program which might not be
> my intended program which causes wrong skew computation

  The clock skew calculation is completely decoupled from the programs.

  That is why you (might) have several PCRTable structures present, one per PCR stream.

  tsdemux can, based on the selected program (and therefore associated PCR PID), select which PCRtable is used to do the various conversions.

  If you look at all the mpegts_packetizer_*_to_*() functions, you will see they all have a pcr_pid field by which you can specify which PCRTable will be used.

  Does that clarify things ?
Comment 11 Baby octopus 2014-02-27 15:11:07 UTC
Edward,

What you are saying very much makes sense. But I still feel there is an implementation bug. Let me see if I can get an MPTS file and get the debug trace. 

Regards,
Jagadish
Comment 12 Edward Hervey 2014-02-27 15:35:06 UTC
Oh, I think I just realized the issue. You're talking about 1.0.8, which *might* have had that issue.

I only care about master here.
Comment 13 Baby octopus 2014-02-27 16:25:44 UTC
Could reproduce the issue with 1.0.8. Tried with 1.2.2. And I dont see any problem. Issue seems to have resolved

I will double check tomorrow and close this bug :)
Comment 14 Baby octopus 2014-03-07 08:23:33 UTC
Hi,

The bug no longer exists with git master. Hence I'm closing this bug

Thanks,
Jagadish
Comment 15 Edward Hervey 2014-03-07 09:15:37 UTC
*** Bug 702152 has been marked as a duplicate of this bug. ***