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

Handle page termination with same page number #11

Open
ileanadumitrescu95 opened this issue Sep 16, 2022 · 0 comments
Open

Handle page termination with same page number #11

ileanadumitrescu95 opened this issue Sep 16, 2022 · 0 comments

Comments

@ileanadumitrescu95
Copy link
Contributor

SourceForge bug #203 written by Stefan Pöschel on 2017-05-24

I recently had a look at two VLC issues that affect Teletext subtitles and libzvbi:
https://trac.videolan.org/vlc/ticket/13498
https://trac.videolan.org/vlc/ticket/18113

In both cases, the subtitle lines remain displayed until being replaced by a new version of the specific line. So the subtitles never disappear and often an old line is still displayed, after another, more recent line is displayed/updated.

At first I suspected that the Erase Page Flag (C4) in the Page Header was not set, but the flag was indeed set (at least once between two LOPs with subtitles). After further debugging it turned out that the Teletext service in the two issues consists of just a single Teletext page - which is the reason here:

The Teletext spec defines the Magazine Serial Flag (C11) in Table 2 (ETSI EN 300 706 v1.2.1) as (bold font by me):

When set to '1' the service is designated to be in Serial mode and the transmission of a page is terminated by the next page header with a different page number.
When set to '0' the service is designated to be in Parallel mode and the transmission of a page is terminated by the next page header with a different page number but the same magazine number.
The same setting shall be used for all page headers in the service.
(BTW it doesn't really matter, but in these two cases this flag is always 0)

In vbi_decode_teletext in /src/packet.c, storing the page is aborted, if the condition "page terminated" in the C11 definition is not met. But as the service here consists of only one page, this condition is never met!

On the one hand, I am a great fan of implementing strictly according to the specs. On the other hand I think that the case of broadcasting just one Teletext page have by mistake not been considered when creating the spec. Furthermore I could not think of a case where removing the check for this condition could harm the decoding (I'm not sure, but I don't know why encoders should do that....which is the reason why I post this into Issues and not into Patches). Hence I removed the check for page termination (for both serial/parallel case) that aborts storing the page. This solves the problem for me for the two files.

Please find the patch attached. I did not check if further modifications are necessary or if the change has other implications. I also did not add a comment to the code (however it could make sense) as I'm not sure how you like to word such special case explanations.

zvbi_handle_same_number_page_termination.patch.txt

Comments by Stefan Pöschel:

I just wanted to ask if there is anything new regarding this or possibly further information missing.

Comments by Michael H. Schimek:

Hi Stefan,

EN 300 706 Annex A.1 recommends to insert a filler/page termination header with page number 0xFF if only a single page is transmitted, to commit the page to memory as well as display subtitles immediately rather than at the reception of the next page header, which may not occur until the subtitles are changing again. The libzbvi decoder recognizes 0xFF headers, but as you realized, not their omission. The check was added as a work-around for some stations repeating headers as filler, and causing a number of undesirable effects such as unnecessary page update events and redrawing, visible blanking if a header with an erase flag repeats ahead of the page body, or duplicate lines in a subtitle transcript.

I would prefer to keep the check but accept repeated headers as terminators under certain conditions, such as a different subpage number, the presence of a page body, or a page erase and subtitle flag in the suspect header. Preliminary tests with my Teletext samples were promising. Further I propose a new auto-termination feature where the decoder delivers subtitle events on its own if no terminator is received within a reasonable time. I already wrote a regression test failing on missing and delayed subtitles and will try auto-termination shortly.

Comments by Stefan Pöschel:

Ah, interesting....I originally thought that there was just a termination with a page header of the same page and that this case was merely not considered by the spec (but it is indeed, as you said). So that this is a corner case where it would make sense to add a workaround to libzvbi.

But there is no immediate termination; there is only a page header some seconds later with Erase Page set that removes the displayed subtitles. So my attached patch doesn't make sense here. And the transmission clearly doesn't comply to the Teletext spec.

For me this is something that definitely has to be fixed on broadcaster side. Hence I withdraw my request to fix this issue in libzvbi (as I'm a strong fan of fixing the source of an issue, if possible).

But I don't know the overall intention of libzvbi: Whether to only accept conform transmissions or to also cope with buggy encoders. So if you want to, you can fix this of course ;-)

ileanadumitrescu95 pushed a commit that referenced this issue Sep 26, 2022
This fixes some array bounds checks that were off by one in the
xds_separator function in caption.c. If class == 4 or c2 == 24 then the
xds_sub_packet "sp" would be set from memory outside the cc->sub_packet
array.

Closes Github issue #16
ileanadumitrescu95 pushed a commit that referenced this issue Sep 27, 2022
This fixes some array bounds checks that were off by one in the
xds_separator function in caption.c. If class == 4 or c2 == 24 then the
xds_sub_packet "sp" would be set from memory outside the cc->sub_packet
array.

Closes Github issue #16
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

No branches or pull requests

1 participant