GNOME Bugzilla – Bug 690934
tsdemux: setting program-number has no effect
Last modified: 2013-08-14 10:31:42 UTC
Setting the program-number doesn't set the output program. Instead it seems the first found program is used. The full log for all elements (attached) shows the program-number being correctly parsed and all, but when tsdemux is first confronted with a candidate program, program-number is always -1, meaning unspecified. That first found program sets the program-number, staving off all others including the actually requested program. Thus: $ GST_DEBUG=tsdemux:5 gst-launch-1.0 dvbsrc bandwidth=8 frequency=578000000 \ ! tsdemux name=demux program-number=1200 demux. ! queue ! fakesink [...] 0:00:01.468676528 21922 0x20a1680 DEBUG tsdemux tsdemux.c:1107:gst_ts_demux_program_started: Current program -1, new program 1205 0:00:01.469441868 21922 0x20a1680 DEBUG tsdemux tsdemux.c:1107:gst_ts_demux_program_started: Current program 1205, new program 1200 0:00:01.469967816 21922 0x20a1680 DEBUG tsdemux tsdemux.c:1107:gst_ts_demux_program_started: Current program 1205, new program 1202 [...] This is using Ubuntu 12.10 packages, with both the default 1.0.1 and 1.0.3-1ubuntu1~ubuntu12.10.1~ppa1 from the gstreamer-developers ppa.
Created attachment 232441 [details] truncated gzipped GST_DEBUG=6 output for a command line similar to that in the description
Fluendo tsdemux http://core.fluendo.com/gstreamer/src/gst-fluendo-mpegdemux/ has this issue fixed. It extracts only the desired program number
Created attachment 238189 [details] [review] Patch to fix program-number property I have tested with a MPEG-TS file containing two separate programs with video and audio. PAT table: program-number 2: pmt-pid 33 prigram-number 1: pmt-pid 32 PMT table: program-number 2: pid 300 (stream-type 27) program-number 2: pid 200 (stream-type 15) program-number 1: pid 301 (stream-type 27) program-number 1: pid 201 (stream-type 15) Pipeline: filesrc location=double.ts ! tsdemux program-number=1 name=d \ d. ! h264parse ! queue ! decodebin ! autovideosink \ d. ! aacparse ! queue ! decodebin ! autoaudiosink Running with the current master checkout I can confirm that setting program-number does not change which program is being decoded. In my test case it is program 2. I have applied a patch that fixes the issue for my test case. The patch applies on the current master branch. If I set the program-number to 1 or 2 the correct program is being decoded. If no program-number is specified program 2 is decoded. I can't say if the patch breaks any intended behavior, but at least it fixes the issue for my particular test case.
With the patch it looks almost correct to me, but not fully. If I'm not mistaken, there are two cases: a) program-number not set via property. In that case it's -1 to start with, and then it will be set to the first program number that is found, until the end of the program or shutdown, when it will be reset to -1 again. b) program-number is set via property, then it should always stay as set and never be reset to -1, not even on shutdown or when program_stopped is called. With the patch, the resetting on shutdown is not handled properly any more in scenario (a), so if the element gets reused it will try to use the number that was last active. With the patch, it's still reset to -1 in program_stopped, so is not permanent across re-use or within the same stream even. (This might be on purpose, I'm not sure). I think we need two variables, one for the program number as set via the property, that is only ever changed via the property, and one for the 'current program number' that's reset based on the property setting (-1 or whatever was set).
Actually there is a current_program_number variable already. (tsdemux.c:1145) However, it seems that it is only referenced in that one line. Looking at the git log it seems that it is legacy from an earlier implementation.
Created attachment 238481 [details] [review] Patch to disentangle program_number
Created attachment 238482 [details] [review] remove unused current_program_number field Doesn't fix anything, but saves a few bytes
I am not sure that attachment 238481 [details] [review] works correctly through program_stopped (I'm not sure how to trigger program_stopped), but it seems to do the right things otherwise. These patches were made on a branch from the 1.0.5 tag, but they apply on master.
commit dfb101e0edf16733997dd33e4fb43071dc26d7b8 Author: Douglas Bagnall <douglas@paradise.net.nz> Date: Sun Mar 10 12:07:40 2013 +1300 tsdemux: disentangle requested program number from active program number The program_number attribute was overloaded, trying to indicate both the currently playing program, and the program requested via the "program-number" property. The end result was that setting the property didn't work (see #690934). I added a new requested_program_number field rather than reviving the current_program_number field because it seemed this would result in fewer changes overall and be less confusing. It breaks symmetry with the "program-number" property, but it retains parallels with the likes of program->program_number. Because gst_ts_demux_reset is called after the properties have been parsed, requested_program_number is initialised in gst_ts_demux_init. Whether this is exactly the right place, I don't know. commit 8e4f966018409592eca28014bdafaffb4ef59d0a Author: Jesper Larsen <jesper.larsen@ixonos.com> Date: Wed Mar 6 12:15:47 2013 +0100 tsdemux: fix program-number functionality Setting the program-number property does not affect which program is actually being demuxed. Moving the initialization of the program_number from gst_ts_demux_reset to gst_ts_demux_init seems to fix this issue. https://bugzilla.gnome.org/show_bug.cgi?id=690934
Hello Edward, Can this be fix be merged into stable 1.0.x branch too? Regards, Jagadish
Hi, I dont think this solves the problem fully. This just makes sure we choose the desired program. This will still cause problem in MPTS stream IMHO. Correct me if I'm wrong. This is because I see calculate_skew using PCR of non selected program. Because of this I see wrapping and resetting of timestamp quite often Not sure if this problem is specific to my sream. Dumps gave me an indication that wrong PCR is used for skew calculation To workaround for my problem, let me disable calculate_skew function for remaining program and see the result Let me know others thoughts here! Regards, Jagadish
PCR calculation are separated (see pcrtable mentions in mpegtspacketizer). Also: please ask those questions on the mailing list and not here.
Thanks Edward. I posted it here since I genuinely felt the patch doesn't handle MPTS stream properly. And as I had mentioned, calculate_skew function is being called for every program(This is easily reproducible using any live MPTS stream) I'm looking into the issue. If I find a better solution I'll post the patch. And if I need any clarification, I'll post it on mailing list Regards, Jagadish
Created attachment 251585 [details] [review] Patch for fixing MPTS PCR correction issue Hi, After a little bit of effort I could fix this. I have attached the patch here. There are two main changes that I have done 1. For MPTS, PCRs of all the programs were used to calculated request_program's clock skew. This is corrected. This issue will not show up unless you are running a live MPTS stream 2. If the user has set not set the program number(-1), I randomly choose one of the program from the PMT, rather than choosing the first one. I just got bored of getting same program everytime so added this change :) Regards, Jagadish
Forgot to mention, this is a patch over version 1.0.8
You do realize this is a closed bug ? And the patches you're providing are a (potentially) new issue ?
Hello Edward, I understand the bug is closed but as I have been saying it isn't fixed properly. Just setting the program-number is not the only thing that is needed to get a right program out of an MPTS, but it also important to make sure right PCRs are used for skew computation I leave it to you to take a call on whether reopen this bug, or take it as a separate bug. You are the best bet :) Let me know if you want me to open a newbug for this. I can do that! Cheers, Jagadish
This bug is *CLOSED*. Open a new bug report.