Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mpegts-mythtv harmonization #15

Open
wants to merge 21 commits into
base: release/5.1
Choose a base branch
from

Conversation

ulmus-scott
Copy link

The only STREAM_TYPE_* define used is STREAM_TYPE_PRIVATE_DATA.

STREAM_TYPE_AUDIO_DTS is already defined by FFmpeg to a different value.  0x8a
is used in MISC_types[].

STREAM_TYPE_AUDIO_HDMV_* defines are from:
MythTV/mythtv@d15e482
They are now unused and FFmpeg has added the entries to HDMV_types[].
STREAM_TYPE_SUBTITLE_DVB is from:
MythTV/mythtv@aaa9372#diff-ae852538fd61b663231c8bdda1fdbade40c97e8305401fcc6f1ad9f395efdf18
STREAM_TYPE_VBI_DVB is from:
MythTV/mythtv@6da1854

However, the corresponding uses in MISC_types[] can never be used.
in mpegts_set_stream_info()
    pes->stream_type = stream_type;
...
        mpegts_find_stream_type(st, pes->stream_type, MISC_types);

mpegts_set_stream_info() is called either with stream_type = 0 or
stream_type = get8(&p, p_end)

0x100 and 0x101 exceed UINT8_MAX and thus will never be found in the search.
Use the names given in ETSI 300 468 and list all of the values used,
even if the defines aren't.
I'm not sure that the new name is correct since I don't have a copy of
ISO/IEC 13818-6.  However, it should be close.
Appears to have been from an FFmpeg merge in 2009:
MythTV/mythtv@6d62e66
…ptor

This has no functional change since MythTV only uses the MythTV addition
AVStream::carousel_id if AVStream::codecpar::codec_id == AV_CODEC_ID_DSMCC_B.
originally from:
MythTV/mythtv@a1d4d11
referencing:
https://code.mythtv.org/trac/ticket/1887

ISO/IEC 13818-1:2021 specifies a valid range of [0x0010, 0x1FFE] in
§ 2.4.4.6 Semantic definition of fields in program association section
and Table 2-3 – PID table
…ng, and pmt_scan_state

req_sid is from an FFmpeg merge:
MythTV/mythtv@2a56a37#diff-2fb8a33ba832f242d27f19c9f25755b57af9807343e99dac77263c1c5ebc383a
removed from FFmpeg in
FFmpeg@90d13e3

scanning and pmt_scan_state are from
MythTV/mythtv@084e3f8

MpegTSContext::req_sid is only set by mpegts_read_header().

First, in pmt_cb() always create PMT filters.
Now we don't want to stop_parse early since that would prevent creating all the
PMT filters, so remove those changes to pat_cb() and pmt_cb().

MpegTSContext::pmt_scan_state is only tested in mpegts_read_header() and its use
is equivalent to `!ts->prg[i].pmt_found`.

Now MpegTSContext::scanning and MpegTSContext::pmt_scan_state are unused, so
remove them.

MpegTSContext::req_sid is now set but otherwise unused, so remove it as well.

Additionally, in mpegts_read_header() sid would cause an invalid index of -1 on
the first iteration.

The fallback code in mpegts_read_header() has been broken since
MythTV/mythtv@0b0068c

sid != pmt_pid, so it was looking for the wrong filter in MpegTSContext::pids.
Disabling CRC is from:
MythTV/mythtv@084e3f8
referencing
https://code.mythtv.org/trac/ticket/328

However, the sample does not trigger that fallback.

Deal with incomplete PMT streams in BBC iPlayer IPTV
MythTV/mythtv@c11ee69
references
https://code.mythtv.org/trac/ticket/9926

The sample is unavailable and the issue was not in FFmpeg's unmodified demuxer,
so, following subsequent harmonization, I assume it is no longer an issue.  If
it still is, MythTV's other modifications should be looked at first.
There is one PMT per program, so add the stream_type to the corresponding
Program.  This means we no longer need to look at an MpegTSFilter.  The
searching logic in is_pmt_equal() is equivalent to that in pmt_equal_streams().
However, we now look at the entire PMT not just is_desired_stream()s.

Nothing now uses MpegTSContext::pid_cnt or MpegTSContext::pmt_pids, so remove
them and a lot of other now unused code.

See ISO/IEC 13818-1:2021 § 2.4.4.9 Program map table
Perhaps we could use TS_program_map_section()'s version_number and possibly
current_next_indicator instead of comparing the PMT ourselves?  Maybe by
modifying update_av_program_info()?

export_pmt() would probably need
if (old_version == -1 || old_version != version)
also its buffer should probably be moved to AVProgram since each program has a
different PMT.

Similarly streams_changed() should have a program_number parameter, since we
probably only care about one program at a time.
This is a MythTV addition that exports the PMT from an MPEG-TS.  However, each
program has its own PMT so having a single pmt_section in AVFormatContext is not
correct when there are multiple programs in the stream.  Previously, each PMT
overwrote the last one.

Adapt AvFormatDecoder to look in AVProgram.

In my limited testing, the PMT did not seem necessary for ATSC captions.

MythAVBufferRef: Satisfy the rule of five; however, since nullptr is now
allowed, the newly defined constructors and operators are used.  The compiler
generated ones were causing use after free segmentation faults.
This is an int to match type with AVProgram.  In mpegts.c, SectionHeader::id is
a uint16_t as ISO/IEC 13818-1:2021 specifies for program_number.

This allows us to ignore stream changes in programs that are not being watched
when there is more than one program.

Regarding the changes to mpeg.c:
Originally from:
dvb/ac3 patches 1, 2, and 3 from Mark Anderson
MythTV/mythtv@87795f6

changed in:
Refs #8134. internal dvd player. resolve problem where pcm_s16be audi…
MythTV/mythtv@e64371a

change reverted in:
Refs #8134. revert most of [24239]. Anduin withers reported it makes …
MythTV/mythtv@a3d6e14

referencing: #8134 (internal dvd player: pcm_s16be improperly detected as pcm_dvd audio codec.) – MythTV
https://code.mythtv.org/trac/ticket/8134

An MPEG Program Stream can only have one program, so FFmpeg does not create an
AVProgram for it.  Thus av_find_program_from_stream() (and therefore
get_current_AVProgram()) will always return nullptr, so avprogram_id will be
ignored.

The calling conventions of functions with "C" and "C++" language linkage are not
necessarily the same; however, in practice they are, so
AvFormatDecoder::streams_changed() could be used directly.  Doing it correctly
still eliminates the use of a friend function and its double declaration at only
the cost of an extra function call, which doesn't matter since streams_changed()
is only called very rarely.
by using the PMT's version_number (and current_next_indicator) to determine when
to call streams_changed().  (FFmpeg already checks the current_next_indicator
in pmt_cb() before the call to update_av_program_info().)

The only change is the first seen PMT for each program will now never call
streams_changed().  is_pmt_equal() would return 0 for the first seen PMT for
each program when opening the file, but streams_changed is null at that point.

This may prevent a call to streams_changed() when a new PAT version adds a
program.  However, MythTV does not fully support multiple programs in a single
transport stream, so this is acceptable.  The new PAT may also remove the
currently playing program, which the PMT code cannot detect.
It does nothing since pmt_cb() already closes the already existing filter before
calling and the call in handle_packet() will only occur when
`ts->pids[pid] == NULL`, i.e. there is no filter to close.

It was originally from:
PMT tracking in ffmpeg patch from danielk.
MythTV/mythtv@236872a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant