Skip to content

tvheadend: sync with PR #2117 at its current head - #7407

Draft
th0ma7 wants to merge 16 commits into
SynoCommunity:masterfrom
th0ma7:tvheadend-vainfo
Draft

tvheadend: sync with PR #2117 at its current head#7407
th0ma7 wants to merge 16 commits into
SynoCommunity:masterfrom
th0ma7:tvheadend-vainfo

Conversation

@th0ma7

@th0ma7 th0ma7 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

What

Carries the tvheadend PR #2117 transcoding rework as a single patch, rebased on its current head.

base       45cbe4adb (master, Jul 31st 2026)
100        diff(45cbe4adb..f9c549a95)
101-108    dropped, taken upstream

Chain is now 000-004, 100, 110, 111.

Why

This PR opened with two fixes to the VAAPI capability probe and grew, over the course of testing on a DS918+, into a series of eight: a hardware decode fallback, three audio-path fixes from a code review, a double free that took tvheadend down whenever transcoder_create() failed, a dropped first frame, and the audio/video start-up misalignment.

uknunknown has now pulled all eight into dev_transcoding_2026, with his own adjustments to the audio.c restructure and a fix of his own in context.c that fills a missing pts from the packet fed to av_parser_parse2() — attacking at its source the same first-frame timestamp loss that 106 handled downstream. So none of them need carrying here any more.

The commit history is kept intact rather than squashed: each patch, its measurements and the two cases where a first attempt was wrong are on the record, since that reasoning is what makes the upstream discussion followable.

How

100-transcoding-2117.patch regenerated as git diff --no-prefix 45cbe4adb..f9c549a95. PKG_GIT_HASH is unchanged — 45cbe4adb is still the parent of his head.

Worth recording, because it nearly went the other way: patch --dry-run reported 106 as still applicable against his tree even though the change was already there. Applying it succeeded with fuzz 1 and duplicated the block — the surrounding context had drifted enough to defeat the reverse-apply check that decides "already applied". Only comparing file contents gives the right answer, and that is now noted in the HOWTO.

Testing

Patch 100 reproduces his tree exactly: applied to a pristine 45cbe4adb tarball, diff -rq against git archive f9c549a95 reports no differences.

Full chain applies with zero rejects. Patched tree configures and builds clean, no errors, no warnings.

The eight dropped patches were exercised on the reporting hardware before going upstream — the double free reproduced from a core dump and confirmed fixed, the first-frame timestamp verified to the exact tick on three encoder paths, and the start alignment measured across fifteen recordings. That work is in the commit history and in the upstream PR discussion.

🤖 Generated with Claude Code

https://claude.ai/code/session_01QEH1b4ASNYSrZFQnEeroj8

@th0ma7 th0ma7 changed the title tvheadend: fix two defects in the VAAPI capability probe of PR #2117 tvheadend: sync with the rewritten PR #2117 branch Sep 3, 2026
@th0ma7 th0ma7 self-assigned this Sep 3, 2026
@th0ma7
th0ma7 marked this pull request as draft September 3, 2026 19:55
th0ma7 and others added 15 commits September 9, 2026 00:30
…mmunity#2117

Two patches on top of the tvheadend PR SynoCommunity#2117 transcoding series, plus a
correction to an existing comment.

112-vainfo-no-terminate-on-failed-init.patch
    init() in vainfo.c unwinds through a fall-through ladder and the
    vaInitialize() failure path does "goto error_Initialize", which calls
    vaTerminate() on a display that was never initialised. Reachable when
    /dev/dri opens but VA init fails: driver/ABI mismatch, missing i965/iHD
    backend, driver not mounted into a container. Jump past vaTerminate()
    instead. The now-unreferenced label goes with it, or -Werror=unused-label
    fails the build.

113-config-vainfo-own-group.patch
    "Enable vainfo detection" was declared with .group = 7, which is
    "Ports settings" -- a VAAPI codec-detection switch among the port
    numbers. Add a "Transcoding" group (9 was free) and move it there.

102-transcode-apply-stream-params.patch
    The explanatory comment claimed the unset geometry left frames without
    a usable duration, tripped the pts-monotonicity guard and caused the
    software HEVC artifacts. It does not: those are input loss, addressed
    by 106. Reworded to state only what is checkable in the tree. The patch
    code is unchanged.

Full chain (000-004, 100-113) applies to a pristine ddb984f4 tarball with
zero rejects and the patched tree builds clean natively. vainfo.c needs
libva, unavailable on the build host, so it was compiled standalone against
stub va/*.h headers under -Wall -Wextra -Werror: clean. Neither 112 nor 113
has been exercised on VAAPI hardware.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QEH1b4ASNYSrZFQnEeroj8
uknunknown rewrote dev_transcoding_2026 on 2026-09-01, dropping two merge
commits and a duplicated commit, and took most of this series upstream.

Base moves from ddb984f4c (master, Aug 7th) to 45cbe4adb (master, Aug 22nd),
which is the parent of his new head a97e57917. Keeping the old base would
have folded four unrelated webui/cloudsmith commits into the transcoding
patch, so PKG_GIT_HASH and digests move with it.

100-transcoding-2117.patch regenerated as diff(45cbe4adb..a97e57917).
Verified to reproduce his tree exactly: applying it to a pristine tarball
gives a directory identical to `git archive a97e57917` (0 differences).

Dropped, now carried by his commit:
  102 apply-stream-params (with his own V4L2 adjustment), 103 rename to mkv,
  104 restore mpegts profile, 105 fix webui tv profiles, 106 hevc software
  preset, 107 muxer report avcC/hvcC failure, 108 muxer fix false
  reconfigure, 109 MUX_BUF_SIZE 32768, 112 vainfo no vaTerminate() on failed
  init, 113 config vainfo own group.

Kept:
  101 hwaccel decode fallback. Still absent upstream, and his new head adds a
  second early-return software fallback that also returns a software pixel
  format without clearing iavhwdevtype, so the audio-loss path is reachable
  by one more route than before.

101-109-transcode-HOWTO.txt renamed to 101-transcode-HOWTO.txt and rewritten
for the single remaining patch.

Chain (000-004, 100, 101, 110, 111) applies to the new tarball with zero
rejects; patched tree configures and builds clean natively, no warnings.
Hardware paths (vaapi/qsv/nvenc/v4l2m2m) are not covered by that build --
the host has none of them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QEH1b4ASNYSrZFQnEeroj8
45cbe4adb has author date Jul 31st 2026 and commit date Aug 22nd. The
previous pin (ddb984f4c) was commented "git hash from Aug 7th 2026", which
is its author date, so follow the same convention rather than switching to
the commit date mid-file.

No functional change: the hash itself is unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QEH1b4ASNYSrZFQnEeroj8
SPK_GIT_HASH/SPK_GIT_DATE follow cross/tvheadend to 45cbe4ad (Jul 31st 2026);
SPK_REV 50 -> 51 since the shipped tree changes.

Changelog gains the three user-visible items that came with the refreshed
PR SynoCommunity#2117 series: the hwaccel rework, the stream profile renaming plus the
restored MPEG-TS profile, and the web UI player / MP4 muxing fixes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QEH1b4ASNYSrZFQnEeroj8
…ationale in-patch

Review of uknunknown's a97e57917. Each patch now carries its own reasoning as
a header block; 101-transcode-HOWTO.txt is dropped as redundant
(100-transcoding-2117-HOWTO.txt stays -- it documents patch regeneration).

102 audio: _audio_ch_layout_differs() compares AVChannelLayout.u.mask, which
    is the channel bitmask only for NATIVE/AMBISONIC order; under CUSTOM the
    same union member is the AVChannelCustom *map pointer. Use
    av_channel_layout_compare(), which dispatches on .order.

103 audio: tvh_audio_context_open_filters() builds source_args, layout,
    filters and resample unconditionally, but only the libavcodec <= 59 arm
    reads them; ch_layout is read by neither. On the ffmpeg 8 target that is
    two str_snprintf() and one av_channel_layout_describe() per setup for
    nothing -- and those calls can return -1, aborting audio setup over a
    string nobody consumes. Moved into the #else arm; ch_layout deleted.

104 audio: doc comment claims the aresample segment's condition "is written
    as never taken". It is taken whenever input and output sample rates
    differ. Documentation only.

101 gains a header recording why the fix stays at the tail of
hwaccels_decode_get_format() and must not be extended to the early return
added by this PR: that one is a deferral until the sequence header gives the
dimensions, get_format() is called again, and setup_context() is written to
be re-entered ("clean up required if you call decode twice"). Clearing
iavhwdevtype there would permanently disable hardware decoding for MPEG2.

Chain (000-004, 100-104, 110, 111) applies with zero rejects; patched tree
configures and builds clean, no warnings. audio.c additionally syntax-checked
against real ffmpeg 8 headers under -Wall -Wextra.

Not patched: the #ifdef ver2 blocks in vaapi.c/qsv.c. Both files do
"#define ver2" at line 29, so the #ifdef arm is live and the #else arm is the
dead one -- and neither file can be compiled on a host without libva/libmfx,
so a 96-line deletion there would ship unverified for no functional gain.
Left for upstream.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QEH1b4ASNYSrZFQnEeroj8
…itten

45cbe4adb has author date Jul 31st 2026 and commit date Aug 22nd 2026, and
"git log" shows the author date -- which is why the earlier comment said
July. tvheadend rebases pull requests before merging, so author dates are not
monotonic on master: the tip reads Jul 31st while the four commits beneath it
read Aug 6th to Aug 20th.

For a package pin the useful date is when the snapshot became master, so use
the commit date. That breaks with the previous convention here (ddb984f4c was
commented "Aug 7th 2026", its author date, though it landed Aug 17th) -- the
convention was giving the wrong date.

SPK_GIT_DATE follows, so SPK_VERS becomes 4.3.20260822. SPK_REV stays at 50:
the last published revision is 49.

Dates only; no change to the pinned hash, the patches, or the built tree.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QEH1b4ASNYSrZFQnEeroj8
Reproduced by @th0ma7 streaming a VP9/VAAPI profile on a DS918+ whose VAAPI
advertises no VP9 encode entrypoint: SIGSEGV in libc's allocator, faulting
address 0, garbage pointer in rdi. Core analysed against the debug build.

    #2  profile_chain_close ()    at src/profile.c:2720
    #3  profile_transcode_work () at src/profile.c:2732
    #4  profile_chain_close ()    at src/profile.c:1160
    #5  profile_transcode_open () at src/profile.c:2805

transcoder_create() returns NULL, profile_transcode_work() takes "goto fail"
and closes the chain, then profile_transcode_open() sees the error and closes
it a second time. That is meant to be harmless -- profile_chain_close() NULLs
every resource it releases -- except prch_sharer, which profile_sharer_destroy()
clears only inside "if (prsh->prsh_queue_run)". prsh_queue_run is set by
profile_sharer_postinit(), which this failure path never reaches, so the first
close free()s prsh with prch_sharer still pointing at it and the second close
runs LIST_REMOVE() and free() over freed memory.

Clear prch_sharer/prch_post_share before free(prsh).

Not introduced by PR SynoCommunity#2117: master (45cbe4adb) has the same double close and
the same profile_sharer_destroy(); the PR only makes the bug reachable by
shipping hardware profiles that can fail at transcoder_create(). Worth sending
upstream separately.

Chain applies with zero rejects; patched tree builds clean, no warnings.
Runtime verification needs the VP9/VAAPI repro on the NAS.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QEH1b4ASNYSrZFQnEeroj8
The 105 header said runtime verification was still pending. @th0ma7 has now
rebuilt with the patch and replayed the exact repro on the DS918+: the
VP9/VAAPI request that produced the core dump now returns 503 and the chain
tears down cleanly -- subscription unsubscribed, PIDs closed, mux stopped, no
crash.

Also records that the failure is total rather than a degradation to audio:
the codec profile is missing, so transcoder_create() fails before any
elementary stream exists. That is upstream's existing policy for a missing
codec profile and is untouched here.

Header text only; the patched hunk is unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QEH1b4ASNYSrZFQnEeroj8
Every software and VAAPI subscription discards exactly one frame at start-up.
Across a full test run all 14 firings of the monotonicity guard in
tvh_video_context_encode() were untimed frames; none was a real ordering
violation.

Trace-level logs pin the cause on av_parser_parse2(), which emits the first
frame of a stream with no timestamps even though the packet fed to it carried
both:

    before  pkt->pkt_dts 68709      pkt->pkt_pts 77718
    after   out_pkt->pkt_dts <unset>  out_pkt->pkt_pts <unset>

The decoder then hands over a frame with pts = AV_NOPTS_VALUE (INT64_MIN) but a
usable dts, and the guard reads INT64_MIN as older than everything. Filling the
missing pts from the dts is exact rather than approximate: 77718 + 3003 = 80721,
the next frame's pts, 3003 being one frame at 30000/1001 in 90 kHz units.

Caveat recorded in the patch header: on the QSV path pts and dts sit in
different time bases (vpp_qsv outputs 1001/30000, dts stays 1/90000), so the
substitution would be wrong there. It is unreachable today -- the mpeg2_qsv
hardware decoder absorbs the untimed frame and the guard never fires on that
path -- but it is a latent hazard worth knowing about. This wants runtime
testing before being trusted.

Deliberately not fixed here: self->pts is seeded to 0, and 0 is a valid
timestamp. Seeding with AV_NOPTS_VALUE is unsafe because
tvh_audio_context_decode() shares the field and would overflow computing
"drift = new_pts - prev_pts" from INT64_MIN. It needs a video-only flag.

Not introduced by PR SynoCommunity#2117: the guard is identical in master (45cbe4adb).

Chain applies with zero rejects. Built with libav enabled so video.c is
actually compiled: video.o clean, no warnings.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QEH1b4ASNYSrZFQnEeroj8
Tested on the reporting hardware across software, VAAPI and QSV. The
substitution fires where an untimed frame reaches the encoder and the value it
supplies is exact each time -- hevc-vaapi recovered 106264 with the next frame
at 109267, software recovered 33720 with the next at 36723, both exactly 3003
ticks apart (one frame at 30000/1001 in 90 kHz). No "Invalid pts" remained.

QSV never triggered the substitution on any attempt, so the pts/dts time-base
mismatch documented in the header stayed unreachable.

Also records that the defect is intermittent rather than per-path: a VAAPI run
that joined the stream on a P frame produced no untimed frame at all, the
decoder having discarded everything until the next key frame.

Header text only; the patched hunk is unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QEH1b4ASNYSrZFQnEeroj8
Recording three profiles with curl and reading the Matroska blocks shows audio
beginning well ahead of the first picture: software +534 ms, VAAPI +1391 ms,
QSV +1368 ms. Everything after that is clean -- zero gaps above 50 ms, frame
spacing a steady 33-34 ms -- so the only defect is the start.

Timing the logs places the delay before the transcoder, not inside it: only
23 to 87 ms separate the filter graph being configured from the first frame
leaving it. The rest is the MPEG-2 decoder waiting on a sequence header from
the source for its width and height. The audio encoder meanwhile runs from the
first moment, so globalheaders releases its hold queue with several hundred
milliseconds of audio in front of any picture.

The patch drops that leading audio so both tracks start together. Nothing is
delayed, no video is touched, and an audio-only subscription is released
unchanged.

Marked a demonstration deliberately. Dropping audio trades away up to ~1.4 s of
sound the source did provide; holding it instead would lose nothing but add the
same latency to start-up. Which is right is a transcoding design decision, not
a packaging one, so the patch is offered to show the mechanism and let it be
argued. A third route -- not building the chain until elementary_info_t carries
the geometry -- addresses the cause rather than the symptom.

Not introduced by PR SynoCommunity#2117: globalheaders.c is untouched by it.

Chain applies with zero rejects; globalheaders.o builds clean, no warnings.
Not yet exercised at runtime.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QEH1b4ASNYSrZFQnEeroj8
…steps

107 gains the measurements it was missing: four recordings per profile put the
median offset at 130 ms (software, from 534), 356 ms (VAAPI, from 1391) and
85 ms (QSV, from 1368). QSV is consistent and software much improved, but one
VAAPI run returned 1052 ms -- near the pre-patch figure.

That residue is structural, not noise. 107 filters the hold queue, and
gh_passthru is armed as soon as that queue drains; the audio encoder is still
running ahead of the video one, so audio arriving between the release and the
first picture is forwarded unfiltered and re-opens the gap. How much depends on
how those moments line up, hence the spread being widest on VAAPI, the slowest
path to a first frame.

108 adds the missing condition: keep discarding leading audio until a picture
has actually been delivered, in the hold queue or after it, with the flag reset
on SMT_START. It applies on top of 107 so the two windows can be measured
separately -- 107 alone, then 107 + 108.

Also records why the "hold the audio instead" alternative does not work:
holding does not alter timestamps, so a later release still stamps the audio
ahead of the first picture and the player still shows a blank frame. Making it
work would require shifting audio timestamps forward, which desynchronises
sound from picture.

Chain applies with zero rejects; globalheaders.o builds clean, no warnings.
108 is untested at runtime.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QEH1b4ASNYSrZFQnEeroj8
… flag

The first version of 108 gated on "have we delivered a picture yet". Measured,
it removed the VAAPI worst case (1052 -> 385 ms) but did nothing for software,
and the block order in the recordings shows why.

Arrival order is not presentation order. The audio encoder lags the video one
on the delivery clock while still stamping its output with the original stream
times, so audio arrives after the first picture carrying a pts before it. One
software run, in file write order: VIDEO at 572 ms first, then six audio
packets stamped 277 to 384 ms. A flag is set by that picture and lets all of
them through, leaving a third of a second of sound with nothing to show.

So the test has to stay on the timestamps, which is what 107 already does. This
version remembers the pts of the first picture at release time and drops later
audio stamped before it, whatever moment it arrives; audio resumes on its own
once its pts passes that mark.

gh_first_video_pts is seeded to PTS_UNSET at creation, since calloc would leave
it at 0 which is a legitimate timestamp, and is reset in gh_flush() -- the one
place every stop and restart path goes through.

Chain applies with zero rejects from a pristine tarball; tree builds clean, no
warnings. Untested at runtime.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QEH1b4ASNYSrZFQnEeroj8
…demo

Five runs per encoder with the rewritten 108 close the problem outright.
Offset between the first audio packet and the first picture, negative meaning
the picture comes first:

    step                  SOFTWARE        VAAPI          QSV
                          med / worst     med / worst    med / worst
    none                   534 / 534      1391 / 1391    1368 / 1368
    107 alone              130 / 226       356 / 1052       85 / 110
    107 + 108 (flag)       280 / 295       304 /  385       53 /  93
    107 + 108 (pts)        -11 /   -5       -9 /    0      -18 /   -2

Fifteen measurements in the last row, every one at or below zero: no run left
with audio ahead of the picture. The DEMONSTRATION label comes off 107 -- the
remedy is settled and the numbers back each step of it.

Adds 106-108-av-start-HOWTO.txt, recording how the three patches divide the
problem and why none substitutes for another: 106 acts on a frame entering the
encoder, 107 and 108 on packets leaving for the client. Without 106 the first
picture lands one frame later and the other two discard 3003 ticks more audio
than they need to.

The flag-based row is kept deliberately. That version of 108 removed the VAAPI
worst case and did nothing for software, because a flag set by the first
picture lets through audio arriving behind it stamped earlier -- one software
recording shows VIDEO at pts 572 ms written first, then six audio packets
stamped 277 to 384 ms. The timestamp comparison is the point, and 107 already
had it right.

Chain applies with zero rejects from a pristine tarball.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QEH1b4ASNYSrZFQnEeroj8
…unity#2117

uknunknown pulled every one of them into dev_transcoding_2026, with his own
adjustments to the audio.c restructure (equivalent) and an addition of his own
in context.c. Verified by content, not by dry-run: 101 hwaccel decode fallback,
102 ch_layout compare, 103 dead legacy args, 104 stale comment, 105 sharer
double free, 106 first-frame pts, 107 and 108 the audio/video start alignment
are all present in f9c549a95.

That distinction matters here. `patch --dry-run` reported 106 as still
applicable against his tree; applying it succeeded with fuzz 1 and duplicated
the block, because the surrounding context had drifted enough to defeat the
reverse-apply check. Only comparing file contents gives the right answer.

100-transcoding-2117.patch regenerated as diff(45cbe4adb..f9c549a95). It
reproduces his tree exactly: applied to a pristine tarball, `diff -rq` against
`git archive f9c549a95` reports no differences. PKG_GIT_HASH is unchanged --
45cbe4adb is still the parent of his head.

106-108-av-start-HOWTO.txt goes with the patches it documented;
100-transcoding-2117-HOWTO.txt records what was taken upstream, uknunknown's
context.c fix, and the dry-run trap above.

Chain is now 000-004, 100, 110, 111. Applies with zero rejects; patched tree
configures and builds clean, no warnings.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QEH1b4ASNYSrZFQnEeroj8
@th0ma7 th0ma7 changed the title tvheadend: sync with the rewritten PR #2117 branch tvheadend: sync with PR #2117 at its current head Sep 9, 2026
Base goes from 45cbe4adb to fd698529f, thirteen commits later, and the date tag
follows the commit date -- when it landed on master, Sep 8th -- rather than the
author date of Sep 3rd. Same convention as the previous bump.

One of those thirteen is worth naming: 922a8b753 "ffmpeg: fetch x264 from the
GitHub mirror". Every CI failure on PR SynoCommunity#2117 was code.videolan.org either
timing out or serving a ~7 kB error page in place of the 841 kB tarball, so
that mirror should quiet them.

100-transcoding-2117.patch regenerated as diff(fd698529f..914870f28), where
914870f28 is uknunknown's head f9c549a95 rebased onto the new master. That
rebase is done locally: his branch still sits on 45cbe4adb, and it applied with
no conflicts. The HOWTO records it, and that the hash will change if he rebases
upstream himself.

Verified to reproduce that tree exactly -- applied to a pristine tarball,
diff -rq against git archive 914870f28 reports no differences.

SPK_REV stays at 50: the last published revision is 49.

Chain applies with zero rejects; patched tree configures and builds clean, no
warnings.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QEH1b4ASNYSrZFQnEeroj8
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