ad_spdif: recreate spdif muxer on reset - #18241
Conversation
|
Everyone is throwing their tokens at truehd packing it looks like. See also: |
Thanks for pointing these out. Neither of the LAV issues affect the spdifenc changes at https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23542. Good to see kodi going to the output timing solution too. |
70656c0 to
f72080f
Compare
|
A few changes:
|
| { | ||
| return pkt->size >= 8 && pkt->data[4] == 0xf8 && | ||
| pkt->data[5] == 0x72 && pkt->data[6] == 0x6f && | ||
| (pkt->data[7] & 0xfe) == 0xba; |
There was a problem hiding this comment.
This should be ... && (AV_RB32(pkt->data + 4) & 0xfffffffe) == 0xf8726fba. I'm not sure though you need MLP too? Comment suggest you do, but it's used only for truehd, no? Otherwise you can drop the mask.
There was a problem hiding this comment.
I dropped the MLP check and switched to the avutil helper.
I had previously temporarily modified ad_spdif to support MLP to test 44100/88200 sample rates in a few MLP files I have. All my TrueHD samples are 48000. I can't think of a good reason to keep MLP support here now and it would be easy to add back when/if needed.
| @@ -175,6 +221,8 @@ static int init_filter(struct mp_filter *da) | |||
| int c_rate = 0; | |||
| determine_codec_params(da, pkt, &profile, &c_rate); | |||
There was a problem hiding this comment.
Now, we will re-probe on ever re-init (seek). If the probe fails after a seek where it succeeded initially (cut/partial first frame), you get a wrong result rather than the old one. And we end up fallbacking to default values. I think we could cache profile/c_rate after initial open and don't reprobe it each time.
Also truehd parser doesn't give profiles, so we spin whole decoder there, avoiding this too is good.
There was a problem hiding this comment.
Probing once should be fine, especially since that's how it currently works. determine_codec_params will now only be called once at start and the profile and rate are cached.
|
|
||
| close_lavf_context(spdif_ctx, false); | ||
| spdif_ctx->dropped_startup_packets = 0; | ||
| TA_FREEP(&spdif_ctx->fmt); |
There was a problem hiding this comment.
I think this should go into close_lavf_context, no? It can be leaked on init failures.
There was a problem hiding this comment.
Good catch. This was wrong.
| { | ||
| spdif_ctx->dropped_startup_packets++; | ||
| drop_packet = true; | ||
| goto done; |
There was a problem hiding this comment.
I would add here MP_WARN after every N packets, in case we lock ourselves in the infinite loop. It can be big number, so it only is printed when something bad happens.
There was a problem hiding this comment.
Done. The interval is 6000 packets, which would be 5 seconds if the stream contained the expected 1200 truehd AU/s.
| TA_FREEP(&spdif_ctx->fmt); | ||
| } | ||
|
|
||
| static bool truehd_has_major_sync(AVPacket *pkt) |
The spdif decoder uses avformat/spdifenc. Some spdifenc paths keep muxer state across packets, such as partial TrueHD MAT frames. After a seek, that state belongs to the old stream position and can cause the next packet to be muxed with stale timing or buffering state. On decoder reset, discard the current spdif muxer context. The next usable input packet will recreate the muxer through the existing initialization path. Cache codec parameters from the initial probe so recreating the muxer does not probe a partial packet after a seek or replace known values with fallbacks. A fresh TrueHD spdif muxer needs a TrueHD major sync before it can begin muxing the stream. Startup or post-seek playback may begin on a TrueHD access unit that does not have major sync, so drop TrueHD packets until a major sync access unit is seen. This also gives the downstream decoder a clean restart point after seeks. Warn periodically if no major sync has been found. Partially addresses: https://trac.ffmpeg.org/ticket/9569 Signed-off-by: Nathan Lucas <nlucasgit@gmail.com>
determine_codec_params() warns when it cannot determine a codec profile. Some passthrough formats use fixed output parameters and do not require the probed profile or sample rate making the warning misleading. Warn only when a value used to configure the selected passthrough format is missing: the profile for DTS-HD, or the sample rate for core DTS and AC-3. Signed-off-by: Nathan Lucas <nlucasgit@gmail.com>
|
I tested with the PR changes so far and haven't seen anything new or unusual. This included seeking around The Martian Extended Edition to trigger the behavior with no major sync after seek. Also tested movies with AC-3 ,E-AC-3, DTS core/MA/HRA. |
2 participants
This partially addresses https://trac.ffmpeg.org/ticket/9569 by resetting the spdif context on seek so stale avformat/spdifenc timing and buffer state will not be used after the seek. This prevents an incomplete TrueHD MAT frame from remaining muxed with access units before the seek, and prevents invalid MAT padding from being calculated relative to timing before the seek.
This also silences timing discontinuity warnings from avformat/spdifenc caused by seeking.
The second commit reduces unnecessary log spam from determine_codec_params by only logging failed profile and rate probes when those values were necessary for the selected format.
AI disclosure: OpenAI Codex assisted with the research and development of these changes. I have reviewed generated code and understand the changes that are being submitted.