Opened 4 years ago

Closed 4 years ago

#13488 closed Patch - Bug Fix (Fixed)

IPTV fix for deadlock, handle redirects

Reported by: ijc Owned by: paul-h
Priority: minor Milestone: 31.0
Component: MythTV - HTTP Streaming Version: Master Head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

I posted various IPTV fixes at https://github.com/MythTV/mythtv/pull/184. There's a long description over there which I won't repeat unless requested. Briefly, fixes:

  • A deadlock in HLSReader cancel path
  • Not handling m3u8 which has a 302 redirect
  • A segfault when combining overlapping segments which have gone backwards
  • Avoid hitting the servers for every single channel, we don't need precise protocol information at that time.
  • A few other random typos, log improvements, setting a User-Agent header.

Attachments (1)

iptv-20190928.tar (50.0 KB) - added by ijc 4 years ago.
patches corresponding to https://github.com/MythTV/mythtv/pull/184/commits/01099ce4decba9c624121212a49bee72cd26e283

Download all attachments as: .zip

Change History (16)

comment:1 Changed 4 years ago by paul-h

Milestone: needs_triage31.0
Owner: changed from cpinkham to paul-h
Status: newaccepted

I've been sitting on a related patch to save the protocol in the database so we don't have to keep doing test downloads to determine if the IPTV channel is a playlist file or TS stream. This is particularly a problem that affects the VBOX. I'll try to combine both sets of patches.

comment:2 Changed 4 years ago by ijc

Please see https://code.mythtv.org/trac/ticket/13489 for an incremental fix to the unit tests.

comment:3 Changed 4 years ago by Ian Campbell <ijc@…>

In 2cf309cd0/mythtv:

MythSingleDownload?: Set a User-Agent

Refs #13488
Signed-off-by: Paul Harrison <pharrison@…>
(cherry picked from commit 017440bde745e8ac97131ed6c01f8a1d17057c51)

comment:4 Changed 4 years ago by Ian Campbell <ijc@…>

In 2d9065e8c/mythtv:

HLSStreamHandler : add debug log to the constructor mirroring the destructor's

Refs #13488
Signed-off-by: Paul Harrison <pharrison@…>
(cherry picked from commit 88b3020b303fb46a419de8185e2000c789782940)

comment:5 Changed 4 years ago by Ian Campbell <ijc@…>

In 3d48c63a7/mythtv:

HLSStreamHandler : add missing arg to Return() debug log

Refs #13488
Signed-off-by: Paul Harrison <pharrison@…>
(cherry picked from commit 1be10a9c6227e115d9df4bf62ad28287e3cc2f14)

comment:6 Changed 4 years ago by Ian Campbell <ijc@…>

In 163e1a4a4f/mythtv:

MythSingleDownload?: fix typo in log message in DownloadURL()

Refs #13488
Signed-off-by: Paul Harrison <pharrison@…>
(cherry picked from commit 78b8e7bfbe0018c840a8d424e7b3809b24b31447)

comment:7 Changed 4 years ago by Ian Campbell <ijc@…>

In 783e0dfd6/mythtv:

HLSReader: fix deadlock in HLSReader cancellation.

The thread running HLSReader::Cancel calls HLSPlaylistWorker::Cancel with the
stream lock held, which blocks waiting for the thread to exit.

The HLSPlaylistWorker::run thread however can be at the point between the
m_cancel check and the call to HLSReader::PlaylistRetrying?. Since
PlaylistRetrying? also takes the stream lock we are therefore deadlocked.

To address this introduce a separate worker lock covering the m_playlistworker
and m_streamworker fields. This means that HLSReader::Cancel only needs to take
this new lock and therefore cannot deadlock against the use of the stream lock
in HLSReader::PlaylistRetrying?.

When they need to be held together the new lock is consistently nested within
the old. Refs #13488

Signed-off-by: Paul Harrison <pharrison@…>
(cherry picked from commit 946efc5c303fd651276437294c6afaa10bb95e72)

comment:8 Changed 4 years ago by Ian Campbell <ijc@…>

In 13976eae2/mythtv:

MythSingleDownload?: add option to return final (post-redirect) URL

Refs #13488
Signed-off-by: Paul Harrison <pharrison@…>
(cherry picked from commit 753fdd84c3d891198045f4375925e2e3bcd546f0)

comment:9 Changed 4 years ago by Ian Campbell <ijc@…>

In 7d1c0d1eb1/mythtv:

MythDownloadManager?: add option to return final (post-redirect) URL

Refs #13488
Signed-off-by: Paul Harrison <pharrison@…>
(cherry picked from commit bdf652e8f5a771577b3550283fd79d7e026e2c6e)

comment:10 Changed 4 years ago by Ian Campbell <ijc@…>

In 2b6381407/mythtv:

IPTVChannelInfo: explictly set IPTVTuningData protocol to invalid

... and avoid the protocol probe. The uses in the channel fetching code do not
require it and this avoids hitting every single URL during a channel scan
(which some providers do not appreciate).

The channel fetcher code doesn't need the protocol, since it calls none of the
relevant methods.

Since the IPTVTuningData contructor argument list has changed we know that
there are no other users of that particular form.

While there constify IsHLSPlaylist

Refs #13488
Signed-off-by: Paul Harrison <pharrison@…>
(cherry picked from commit 896f0c816c175125291897b48a2850963c388175)

comment:11 Changed 4 years ago by Ian Campbell <ijc@…>

In 10691b473/mythtv:

HLSStream/Reader: rename m_url field to m_m3u8_url throughout

This makes space for subsequently recording the final base URL to use for the
individual streams, in the case where the m3u8 is behind a redirect.

Refs #13488
Signed-off-by: Paul Harrison <pharrison@…>
(cherry picked from commit ed0cb56d3e3f4d7eada82422d488998e7eaa954d)

comment:12 Changed 4 years ago by Ian Campbell <ijc@…>

In 3e61fef82/mythtv:

HLS: deal with m3u8 behind a 302 redirect

By recording the final m3u8 URL and using that as the basis for calculating
the stream URLs.

Refs #13488
Signed-off-by: Paul Harrison <pharrison@…>
(cherry picked from commit 1c846b682e21ab0c31a7b380b410f0e631dd182f)

comment:13 Changed 4 years ago by Ian Campbell <ijc@…>

In a2ac6aa62/mythtv:

HLSReader: Only try to combine overlapping playlists if the diff is >= 0

Otherwise Inew += diff results in going off the front of the array and a crash.

Refs #13488
Signed-off-by: Paul Harrison <pharrison@…>
(cherry picked from commit a59a4f0749edaed960d8b9f5fd35dff2c67d4c02)

comment:14 Changed 4 years ago by Ian Campbell <ijc@…>

In 36d96c956/mythtv:

Fix TestIPTVRecorder after change to IPTVChannelInfo

In a60e7429a09a ("IPTVChannelInfo: explictly set IPTVTuningData protocol to
invalid") I removed IPTVChannelInfo::IsValid() thinking it was unused but
missed the uses in these test cases. Reintroduce the method, but make it
protected and a friend of the TestIPTVRecorder class.

The above change also changed the behaviour of IPTVChannelInfo's constructors
such that the protocol is not always probed, and therefore IsValid() is not
necessarily true. This is the case when constructed via
IPTVChannelFetcher::ParsePlayList. Update TestIPTVRecorder::ParseChanInfo to
reflect this by now asserting that the tuning data is not valid and that the
protocol is inValid as expected.

Refs #13488

Signed-off-by: Paul Harrison <pharrison@…>

comment:15 Changed 4 years ago by paul-h

Resolution: Fixed
Status: acceptedclosed
Note: See TracTickets for help on using tickets.