[mythtv] Harmonizing mpegts-mythtv.(c|h) with FFmpeg's mpegts.(c|h)

[John Pilkington]

On 17/02/2022 14:29, Peter Bennett wrote:
> 
> On 2/15/22 22:23, Scott Theisen wrote:
>> Peter,
>>
>> My harmonization of mpegts-mythtv.* is almost complete.  (This is on 
>> top of my rebase/4.4m3 branch, but should have no issues being applied 
>> on top of mythtv/FFmpeg/master.)
>>
>> See https://github.com/ulmus-scott/FFmpeg/commits/harmonize for the 
>> original commit order and 
>> https://github.com/ulmus-scott/FFmpeg/commits/harmonize2 for reordered 
>> with problematic commits moved to the end.
>>
>> I have been testing with some of my local stations and the samples 
>> from https://code.mythtv.org/trac/ticket/13557 / 
>> https://github.com/MythTV/mythtv/issues/351 and everything is fine 
>> through mpegts-mythtv.c: harmonize ff_parse_mpeg2_descriptor() part 1 
>> <https://github.com/ulmus-scott/FFmpeg/commit/c22901ac83f9955f3846d33f79f4ddb8a07aeb8d>.
>>
>> I have been banging my head against the brick wall that is pat_cb() 
>> and pmt_cb().  (It started out fairly straightforward, but dealing 
>> with those two functions and the 17 year old changes buried under 
>> updates has been increasingly frustrating.)
>>
>> mpegts-mythtv.c: pmt_cb(): harmonize part 6 
>> <https://github.com/ulmus-scott/FFmpeg/commit/58cc290d552169f2388e83384e1a28a22a710ae7> 
>> appears to actually work fine; however, the logic for calling the 
>> streams_changed() callback is wrong, so it isn't called when it was 
>> previously. Correcting it 
>> <https://github.com/ulmus-scott/FFmpeg/commit/8a8dc3d5599bd61368c26ed410248cad20f7fa25>, 
>> sort of (it updates on every different PMT, instead of if the streams 
>> actually changed), causes a segmentation fault in 
>> https://github.com/MythTV/mythtv/blob/709d05c0d6928e927ea1b0cc2b2c46bc80d40055/mythtv/libs/libmythtv/decoders/avformatdecoder.cpp#L2652 
>> .
>>
>> I suspect that is because the PSIPTable does not create a copy of its 
>> input data, which is then freed while the PMT table is being accessed.
>>
>> mpegts-mythtv.c: pat_cb() harmonization part 3 
>> <https://github.com/ulmus-scott/FFmpeg/commit/6887b3127bcc2b8211e0fd7ab9828965ffe847f0> 
>> causes a different segmentation fault in FFmpeg (dump.c trying to read 
>> the st->metadata AVDictionary for the "language").  I wouldn't worry 
>> about this one right now since I feel pmt_cb() has a higher priority, 
>> so ff_parse_mpeg2_descriptor() can be minimally modified.
>>
>>
>> Let me know if you want me to open another set of pull requests with 
>> the "good" commits for easier testing.
>>
>> Any thoughts on how to complete the harmonization would also be 
>> appreciated.
>>
>> Regards,
>>
>> Scott
> 
> I don't think I can help much here. I have stayed away from the 
> internals of FFmpeg except where necessary to fix errors when merging 
> new versions. I do not have any understanding of the internals of 
> Transport Streams or why MythTV needs to make so many changes to FFmpeg. 
> The changes that were done are very old, as you have noted, and were 
> done by others who are no longer around. I don't know if any other 
> developers of MythTV have better insight into this.
> 
> Peter

I'm not a MythTV developer or a serious coder;  I've just been using and 
taking an interest in it for many years.  But IIRC the project started 
using its own 'fork' of ffmpeg because unexpected changes made by the 
ffmpeg team could immediately affect MythTV users, whose maintainers 
would then have to try to pick up the pieces.

Doesn't this still seem likely if 'harmonisation' goes too far ?

John P