[Video][Subtitles] Use ffmpeg A53 sidedata instead of custom demuxer

[enen92]

Description

While Kodi's closed caption decoder (although outdated when compared to upstream) is still far superior than that decoder contained within ffmpeg, the same can't be said about our custom demuxer implementation. There are a lot of reports/samples out there that simply cannot be played in Kodi but work just fine in mpv or ffplay. While investigating #22143 I realised the root cause of the problem was the fact our demuxed was extended from an initial support of AV_CODEC_ID_MPEG2VIDEO to AV_CODEC_ID_H264 reusing some assumptions that only make sense in the former codec case (e.g. the start code). This code is complex and not maintained, ffmpeg seem to always fill the A53 side data with the closed caption data block, so we might want to use that instead in our demuxer (taking advantage of the already existing decoding code).

User feedback is appreciated...
@matthuisman would be great to know if this finally fixes the CC captions not available in some of your addons.

@ksooo it'd be nice if you could have a look into the code if you find some time. Looking for a pure code review (stuff to improve), I am not expecting you to be familiar with the inner-workings of the thing.

Motivation and context

Improve subtitles and simplify our codebase at the same time. Get an alternative to the #22143 revert

How has this been tested?

Runtime tested with all my CC samples, some that did work and others that didn't work. Those that used to not have captions detected now show the subtitle stream.

What is the effect on users?

Hopefully better support for CC captions in Omega

Screenshots (if appropriate):

Before:

PR:

Types of change

• Bug fix (non-breaking change which fixes an issue)
• Clean up (non-breaking change which removes non-working, unmaintained functionality)
• Improvement (non-breaking change which improves existing functionality)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that will cause existing functionality to change)
• Cosmetic change (non-breaking change that doesn't touch code)
• None of the above (please explain below)

[enen92]

@CastagnaIT thanks for the tips, hopefully done right on the last push

[enen92]

@lrusak think I've managed to use a unique_ptr for the side data (added on a separate commit). Please take a look if you have a chance.

[enen92]

Added a new commit to fix another issue, according to the standard duplicate dual codes must be discarded. Also in ccextractor:
https://github.com/CCExtractor/ccextractor/blob/master/src/lib_ccx/ccx_decoders_608.c#L1193-L1202

The checking code was already there but was only being applied to the control chars, not regular dual char codes.

Before:

After:

[emveepee]

@enen92 is this for subtitles as shown in the title or closed captioning 608/708 (or both)?

[enen92]

Closed captions (eia-608/708) which are also subtitles.
This is still a bit WIP, I've found one sample that didn't work and I'm trying to figure out the issue.

If you have content and are able to give them a go it'd be great.

[emveepee]

Closed captions (eia-608/708) which are also subtitles. This is still a bit WIP, I've found one sample that didn't work and I'm trying to figure out the issue.

If you have content and are able to give them a go it'd be great.

Here in North America we generally use the term closed captioning for accessibility (it also has description of sounds, music playing notes etc) and subtitles for alternate languages which is why I wasn't sure.

I will add this after Nexus final and try with Live TV we have 608/708. I have a client for Android too and Exoplayer does a better job then Kodi so I will use that for comparing. The Exoplayer code is here
https://github.com/google/ExoPlayer/tree/release-v2/library/extractor/src/main/java/com/google/android/exoplayer2/text/cea It doesn't work with as many samples as ccextractor but it might help you too

[enen92]

I am interested in samples or accessing addon streams (temporarily) which the closed captions cannot be displayed/demuxed in Kodi (I mean no subtitles at all not exactly errors displaying, colouring or positioning the subtitle entries).

So far the only public samples that didn't work (also didn't work before) are those provided by the dash industry forum (e.g: https://livesim.dashif.org/dash/vod/testpic_2s/cea608.mpd) but unfortunately they are completely broken and can't be used as a reference for now, see:

Dash-Industry-Forum/dash-live-source-simulator#99 (comment)
Dash-Industry-Forum/dash-live-source-simulator#115

Maybe we can give them a go later on if those major issues are fixed

[enen92]

Added a few more commits and I consider this finished:

• Simplify the code of ccdemuxer as we don't need two buffers anymore
• Removed the codec restrictions we used to have for calling the cc demuxer. FFMpeg sets the side data whenever it is detected and other codecs might indeed have closed captions (e.g. HEVC not only mpeg && h264 as before) and the code path is the same.
• Decided to remove our old setting "Enable parse of closed captions" since the extraction of closed captions no longer puts any additional load on the CPU. It's done by ffmpeg when decoding the pictures anyway. Any load caused by decoding the captions is pretty similar to any other subtitle type. (reverted)

Also the DASH foundation has fixed their public streams.

I don't want to add more stuff into this (nor any other fixes out of the scope of CC demuxing) to avoid exploding the diff. Plan to get this merged by the end of the month unless someone objects. Getting a green and some testing would still be great.

Note CC decoding and general behaviour is (and should be) still pretty much the same. Any issues with colors, positions, some entries no showing are out of the scope of this PR. I've just changed the way we get the data.

[emveepee]

I've been trying this with live TV today and in general it seems like a definite improvement. Omega is getting less stable for general use though so I think my testing will be limited to your PR I can't use master at the moment..

You probably should look at this sample since the captioning just stops https://www.mediafire.com/file/gefs76f1ooppj3o/subtitles.ts/file but can be re-enabled on track 2. Once track 2 is saved it is ok. Other plays seem to autodetect this. Musical notes show as boxes too, don't know if that is in scope.

It is also hard to compare it to other players with positional support especially when several people are talking but it still seems like an improvement. I don't need cc's but I am interested in it for the community. It is really nice not to have to configure Kodi to parse them.

[CastagnaIT]

i have tested with my samples and all worked good job!

However, I think a solution should be found for the removed CC setting
was here because also for another reason, so due to track language problem,
currently we have no way to know CC language type and this lead to following situation:

for example:
you play a video that have CC Spanish,
the user want display only Italian subtitles (then set kodi sub language setting to Italian)
but since the CC track have unknown language, kodi player always display CC subtitles no matter what language you set in kodi language setting, so without a setting the user always have to manually disable subtitles each time that play a video

[enen92]

Thanks for the feedback. Yeap I am not a big fan of it either, they are always enabled by default and their definition is too generic. I have to take a look to into the standards to check if there's any way to flag some of the properties (language, type, etc) but I don't think so. Also not sure if inputsream add-ons can create those streams beforehand since they are usually flagged in the respective manifests...but then we also lack anyway to create multiple streams of the same type (e.g. 2 cea608 streams)...
I guess I'll just remove the setting removal commit for now to keep the previous behaviour (this is about the demuxer anyway). There's lot of room for future improvements.

[CastagnaIT]

generically after the test it for me its fine,
but i cannot evaluate the changes on VP

[emveepee]

@enen92 not having to enable parsing was one of the major benefits of this PR. I agree that they should not be forced on but couldn't there be a compromise be made to not display them by default like other subtitle types? In any case for closed captioning the language cannot be consider safe, here often the language identifier when it is used and not unknown is wrong.

[enen92]

Yes, later...
This is not really the focus of this PR. I agree with castagna we should have a proper mechanism in place if we decide to drop the setting.

[enen92]

I think I'll merge this tonight if no one objects. I have a few changes I want to make to closed captions and its easier to work on top of this. Plus, it's quite early in v21 release cycle anyway.
I'll track hypothetical regressions if they appear ofc.