[mythtv] [PATCH] DVB fixes

[Augusto Cardoso]

Hi!



*********** REPLY SEPARATOR  ***********

On 12/21/2003 at 4:44 AM Kenneth Aafløy wrote:

>Hi!
>
>> On 12/21/2003 at 3:32 AM Kenneth Aafløy wrote:
>>>
>>>But none of those had a cam attached, right?
>>>
>>>The problem is that the cam is sent a program map table,
>>>which I have to wait for in dvbcam. This will then skip
>>>some content in the stream, because the cam won't let
>>>data through untill it knows what to decode, and has
>>>verified the card ws stream content.
>>>
>>>I'm also pretty sure that all info in the PS is contained
>>>in the TS packets, and not generated by libdvbdev. Because
>>>TS is merely a wrapper for the PS, which just needs unpacking.
>> 
>> You're right, I'm not running with a CAM. What you do is to 
>> fix the dvbrecorder.cpp and enable the wait_for_seqstart. 
>> Without CAM, the you don't need the wait_for_seqstart.
>>
>> Please change the default for wait_for_seqstart to true and 
>> please commit these changes because some of them do fix a 
>> couple of crashes.
>
>Yea, this makes sense, and for a later todo item:
>
>Maybe something more close to what is in ctools.c:seek_mpg_start
>should be implemented in dvbrecorder.cpp, instead of the simple
>stuff that is in there right now.
>

Unfortunately, I don't have a CAM here right now, otherwise I would change and debug the code, but if you can send me the raw TS that comes from you card, I'll glad to look at it and figure what we can do to fix it.
Based on what you're saying, when we get video packets from the CAM, they should be fine. I'm very curious to look at the data stream coming from the CAM. By the way what CAM are you using?

The db change is very important, otherwise it may not work correct.

>>>Nothing audio/video related is stored in 1ba (pack_start), 
>>>it's stored in a program map with startcode 1bc (look at 
>>>libavformat/mpeg.c). It's merely an indicator of the
>>>start of a group of packets, iirc.
>> 
>> The muxrate is necessary and it's calculated from the video 
>> and audio data. This has to do more with avsync and the 
>> timing model of MPEG. Please note that some players and some 
>> implementation don't use this.
>> 
>> The 00 00 01 BA and 00 00 01 BB aren't present in the TS 
>> stream. They are currently generated in the libdvbdev. Please 
>> look at the code if you don't believe me. This is in ctools.c 
>> "cwrite_ps". There is a whole sequence of events that starts 
>> in transform.c and will eventually get to "cwrite_ps".
>
>I'm still pretty sure that for each service id there is 1ba contained
>in the stream, but the information in this is for the complete
>stream, but the library is designed to extract some/all pids
>of a serviceid, which requires the headers to be regenerated.
>I belive it should be contained in the packet with 'pktbuf[1] & 0x40',
>or have gotten filtered away by packet filtering.
>
>I also might be totally wrong about the whole thing, as I'm not
>any kind of mpeg expert :)
>

Unfortunately, I believe you're wrong about this. You can wait all day and you will never seen the 00 00 01 BA or 00 00 01 BB in a TS.
Please look at iso13818-1 "Figure 0-1 -- Simplified overview of ITU-T Rec. H.222.0 | ISO/IEC 13818-1 scope"
and
>From iso13818-1 page 54:
"In a Program Stream, PES packets are organized in packs. A pack commences with a pack header and is
followed by zero or more PES packets. The pack header begins with a 32-bit start-code. The pack header is
used to store timing and bitrate information.
The Program Stream begins with a system header that optionally may be repeated. The system header carries a
summary of the system parameters defined in the stream."

There is a lot of confusion between PS, PES, ES and TS. You can have a PES only file. It's fine to have that type of file, but a lot of applications won't support it. A PS is a supported by most applications.
The PS must have the header. The current code in dvbrecorder.cpp will strip the header at the begging, which is the most important.

The 00 00 01 BA and 00 00 01 BB are part of the PS and not of the TS. That is a common mistake.
TS is designed for a imperfect medium. PS is for a lossless medium.

Please let me know if you have questions about the specification. Note that the specification is a recommendation and a lot of companies/people will "interpret" the information different ways. What is really important is how things are done in the real world.

>Also are the static fixes really necessary, wouldn't it be enough
>to serialize the send_ipack calls with locks?
>

They are necessary because you can change a channel and keep the thread going. If there is a mechanism to clean the statics when you change a channel, it should be fine. I personally think that static variable inside a function in a library is a bad idea. I understand why was done that way, but I would have implemented a different way.
If not a problem today, it will become a headache later.

>And put comments of what you have done at the top, and not in the
>middle of the code, please.
>

I keep the comments in the code for two reasons, it serves as documentations of the changes and because this changes are not committed to the CVS, it's easier to merger this way.
You can delete the comments. In the future I will remove the comments before sending a patch.

>Thanks alot for the fixes!
>

No problem. Thank you for doing all the work for getting DVB support and Mythtv. This is a great software. I've been working with DVB for 6 years and MPEG for 8. This is by far one of the best things that I have seen in terms of set top box software. It beats a lot of commercial software.

>Kenneth
>
>
>_______________________________________________
>mythtv-dev mailing list
>mythtv-dev at mythtv.org
>http://mythtv.org/cgi-bin/mailman/listinfo/mythtv-dev