[FFmpeg-devel] New patch for mpegts.c

JULIAN GARDNER joolzg at btinternet.com
Wed Oct 24 21:30:34 CEST 2012









>________________________________
> From: Reimar Döffinger <Reimar.Doeffinger at gmx.de>
>To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org> 
>Sent: Wednesday, 24 October 2012, 20:33
>Subject: Re: [FFmpeg-devel] New patch for mpegts.c
> 
>On 24 Oct 2012, at 07:25, JULIAN GARDNER <joolzg at btinternet.com> wrote:
>> ----- Original Message -----
>>> From: Michael Niedermayer <michaelni at gmx.at>
>>> To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
>>> Cc: 
>>> Sent: Wednesday, 24 October 2012, 5:26
>>> Subject: Re: [FFmpeg-devel] New patch for mpegts.c
>>> 
>>> On Wed, Oct 17, 2012 at 05:51:28PM +0100, JULIAN GARDNER wrote:
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> ----- Original Message -----
>>>>> From: Michael Niedermayer <michaelni at gmx.at>
>>>>> To: FFmpeg development discussions and patches 
>>> <ffmpeg-devel at ffmpeg.org>
>>>>> Cc: 
>>>>> Sent: Wednesday, 17 October 2012, 17:36
>>>>> Subject: Re: [FFmpeg-devel] New patch for mpegts.c
>>>>> 
>>>>> On Wed, Oct 17, 2012 at 08:46:07AM +0100, JULIAN GARDNER wrote:
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>>   ----- Original Message -----
>>>>>>   > From: Michael Niedermayer <michaelni at gmx.at>
>>>>>>   > To: FFmpeg development discussions and patches 
>>>>> <ffmpeg-devel at ffmpeg.org>
>>>>>>   > Cc: 
>>>>>>   > Sent: Wednesday, 17 October 2012, 1:41
>>>>>>   > Subject: Re: [FFmpeg-devel] New patch for mpegts.c
>>>>>>   > 
>>>>>>   > Hi
>>>>>>   > 
>>>>>>   > On Tue, Oct 16, 2012 at 08:34:27PM +0100, JULIAN GARDNER 
>>> wrote:
>>>>>>   >> 
>>>>>>   >>  >________________________________
>>>>>>   >>  > From: Michael Niedermayer <michaelni at gmx.at>
>>>>>>   >>  >To: FFmpeg development discussions and patches 
>>>>>>   > <ffmpeg-devel at ffmpeg.org> 
>>>>>>   >>  >Sent: Tuesday, 16 October 2012, 17:16
>>>>>>   >>  >Subject: Re: [FFmpeg-devel] New patch for mpegts.c
>>>>>>   >>  > 
>>>>>>   >>  >On Tue, Oct 16, 2012 at 02:33:01PM +0100, JULIAN 
>>> GARDNER 
>>>>> wrote:
>>> [...]
>>>>>>   > If its about spec compliance, please quote the spec that 
>>> requires
>>>>>>   > this behavior.
>>>>>>   > 
>>>>>>   ETS 300468  5.1.1 d  
>>>>> 
>>> http://www.etsi.org/deliver/etsi_en/300400_300499/300468/01.11.01_60/en_300468v011101p.pdf
>>>>> 
>>>>> doesnt say one should discard things with equal version numbers
>>>>> 
>>>> 
>>>> "When the characteristics of the TS described in the SI given in the 
>>> present document change (e.g. new events start, different composition of 
>>> elementary streams for a given service), then new SI data shall be sent 
>>> containing the updated information. A new version of the SI data is signalled by 
>>> sending a sub_table with the same identifiers as the previous sub_table 
>>> containing the relevant data, but with the next value of version_number."
>>>> 
>>>> "document change" " the next value of version_number", 
>>> so to me this means that if the data is THE SAME then the version number will be 
>>> the same, and i guess that the guys who wrote the spec did this so you DONT have 
>>> to process the SAME data over and over again, what it was added for was hardware 
>>> filtering, which if you have spent 15+ years writing code for sat receivers etc 
>>> you will know this. Why would you want to process possibly millions of sections 
>>> which contain the SAME data, to just produce the same tables?.
>>>> 
>>>> Now if you keep on banging on about the .00000001% of people who 
>>> concatenate 2 ts streams and do not follow the spec and either make sure the SI 
>>> data is the same, which means the version number the same, or make the version 
>>> number follow by becoming the next version then we are at an impasse, because i 
>>> think your wrong on this, but as your in charge i will await your decision.
>>>> 
>>>> And what are we discard, we are just not processing THE SAME DATA over and 
>>> over again.
>>> [...]
>>>>>>   Again we either process xxxx thousand sections or we do as the 
>>> spec says 
>>>>> and process only new/updated?
>>>>> 
>>>>> The spec doesnt say that, it says:
>>>>>     d) version_number:
>>>>>    -     When the characteristics of the TS described in the SI given 
>>> in the 
>>>>> present document change (e.g. new
>>>>>          events start, different composition of elementary streams for 
>>> a given 
>>>>> service), then new SI data shall be
>>>>>          sent containing the updated information. A new version of the 
>>> SI data 
>>>>> is signalled by sending a sub_table
>>>>>          with the same identifiers as the previous sub_table 
>>> containing the 
>>>>> relevant data, but with the next value
>>>>>          of version_number.
>>>>>    -     For the SI tables specified in the present document, the 
>>> version_number 
>>>>> applies to all sections of a
>>>>>          sub_table.
>>>>> 
>>>>> That speaks about "when X then ... shall be sent ...", 
>>> sending happens
>>>>> on the muxer side, not the demuxer. The text quoted says nothing about
>>>>> what a demuxer should or should not do with what it receives. It
>>>>> simply describes what a demuxer can expect from a valid DVB stream.
>>>>> A concatenated stream as you already explained is not a valid DVB
>>>>> stream ...
>>>> 
>>>> So your now saying that the spec is only for the muxer, can you show me the 
>>> spec for the demuxer then that is different, again i come back to the "New 
>>> version .... but with the next value of version number". This spec if for 
>>> DVB compatible systems, both muxers and demuxers.
>>>> 
>>>> So have you tested what happens when you play a concatenated stream on a 
>>> DVB compliant receiver, you will get the same as with my modifications. As you 
>>> always seem to tell people, fix the problem 1st. The only problem i can see is 
>>> that you want the decoder NOT to follow the spec, but instead work for the NON 
>>> COMPATIBLE streams, when instead the concatenate should be fixed to produce 
>>> correct TS streams.
>>>> 
>>>> I will leave it now and let you decide, as this circle will just keep going 
>>> round and round. If it will be included in any future ffmpeg release then great 
>>> if not then i will just keep it in my local tree.
>>> 
>>> Patches must provide an overall improvment to be accepted.
>>> Noone has shown a meassureable improvment for it yet and
>>> the patch breaks seeking and breaks concatenated streams
>>> If you want to keep it in its current form in your local tree then
>>> i think you make a mistake.
>>> 
>>> Instead IMHO you should rethink what improvment the patch provides
>>> and then objectively test if it really does. If it does, then fix
>>> the issues and repost the patch so it gets included in ffmpeg and
>>> all users benefit.
>>> OTOH if it doesnt provide a real user vissible improvment, lets just
>>> lay it rest and work on something that does.
>>> 
>> 
>> Ok given up trying to convince you that you are wrong.
>
>For what it's worth I can't see where it would be wrong.
>By what I understand of it I can see only one way this can be used to definitely improve the code: actually check if the data that should be the same actually is, and if not use a heuristic to decide which to use.
>This would not change anything for concatenated streams if done correctly, while being more resilient to bitstream errors.
>Concerning the interpretation of the spec: I see it only making assurances, which means that e.g. as an optimization a demuxer can skip some processing with risking issues. That does however not >say anything about what a demuxer should do when those assurances are actually violated.


So explain why they put a version numbering system in, a bit silly adding something in for nothing would you not think?


All i can say in many many years of working on satellite receivers for numerous companies on numerous chips they all have this little bit in the filter for ignoring sections that have the same version number, you then build the table based on the current version number, when the table is valid (when all sections have the same version number), you turn the filter on and you get no data until the version number changes. At this point you invalidate the table and loop around again until the table is valid, and set the flag again, and so on.


Current code limitations

1. Tables with multiple sections
2. Sections are processed no matter what version number is sent, as pointed out on even a small .ts file, saving processing 99.9% of sections will amount to a speedup, but i was not looking for this.


So as it stands this code will FAIL if you get a multisection PAT, PMT, SDT or even EIT.


Also seeking will not be broken, as a VALID ts file when backward/forward seeking the table number will either be the same, no changes, or different which in the new code will process the section as soon as it appears.

joolz



More information about the ffmpeg-devel mailing list