CEA-708 DFx_DefineWindow redefinition #185

New Issue

claunia · 2026-01-29T16:37:21Z

claunia commented

2026-01-29 16:37:21 +00:00

Originally created by @cactusGit on GitHub (Sep 19, 2016).

Here's sample: http://stream.sunnysubs.com/s06e16_crop.ts
As meant in #425, bin segfaults ccextractor, so I won't attach it.
Command line:

./ccextractor --noscte20 -svc 1 s06e16_crop.ts
cat s06e16_crop.p1.svc01.srt

Extracted subs from this stream by other ripper: link
As we can see, some lines are dropped.

When two lines appear on screen, before second line window is being redefined, but this code clears window and removes the first line:
https://github.com/CCExtractor/ccextractor/blob/master/src/lib_ccx/ccx_decoders_708.c#L931
Commenting this line solves problem.

Maybe we need an option to control this behavior?

Originally created by @cactusGit on GitHub (Sep 19, 2016). Here's sample: http://stream.sunnysubs.com/s06e16_crop.ts As meant in #425, `bin` segfaults ccextractor, so I won't attach it. Command line: ``` ./ccextractor --noscte20 -svc 1 s06e16_crop.ts cat s06e16_crop.p1.svc01.srt ``` Extracted subs from this stream by other ripper: [link](http://static.dhne.ws/wp-content/uploads/2016/08/My-Little-Pony-Friendship-is-Magic-617-The-Times-They-Are-a-Changeling.txt) As we can see, some lines are dropped. When two lines appear on screen, before second line window is being redefined, but this code clears window and removes the first line: https://github.com/CCExtractor/ccextractor/blob/master/src/lib_ccx/ccx_decoders_708.c#L931 Commenting this line solves problem. Maybe we need an option to control this behavior?

claunia closed this issue

2026-01-29 16:37:21 +00:00

claunia commented

2026-01-29 16:37:22 +00:00

@cfsmp3 commented on GitHub (Sep 21, 2016):

The subs from the other ripper are 608, so it's really oranges to apples.

I'm looking at the source code and DefineWindow will cause a clear if the definition is not identical to the existing one. If the windows exists and the parameters received to create it (pen style, etc) are the same ones (that what's the memcmp() does) then the window content is left alone.

This logic is probably just not correct. I don't think we need a option to control this - it's not like TVs let you choose, they just do the right thing. I don't have CEA 708 available right now but I'll reread when I do.

@cfsmp3 commented on GitHub (Sep 21, 2016): The subs from the other ripper are 608, so it's really oranges to apples. I'm looking at the source code and DefineWindow will cause a clear if the definition is not identical to the existing one. If the windows exists and the parameters received to create it (pen style, etc) are the same ones (that what's the memcmp() does) then the window content is left alone. This logic is probably just not correct. I don't think we need a option to control this - it's not like TVs let you choose, they just do the right thing. I don't have CEA 708 available right now but I'll reread when I do.

claunia commented

2026-01-29 16:37:22 +00:00

@cactusGit commented on GitHub (Sep 21, 2016):

Yep, it's 608, but they're mostly identical, when this line commented.

I've missed that memcmp. But if we'll look in debug output, there are small differences between these definitions:

[CEA-708] dtvcc_handle_DFx_DefineWindow: W[1], attributes: 
[CEA-708] Visible: [No]
[CEA-708] Priority: [3]
[CEA-708] Row count: [1] # this
[CEA-708] Column count: [32]
[CEA-708] Anchor point: [0]
[CEA-708] Anchor vertical: [65]
[CEA-708] Anchor horizontal: [0]
[CEA-708] Relative pos: [No]
[CEA-708] Row lock: [Yes]
[CEA-708] Column lock: [Yes]
[CEA-708] Pen style: [4] # this
[CEA-708] Win style: [2]
...
[CEA-708] dtvcc_handle_DFx_DefineWindow: W[1], attributes: 
[CEA-708] Visible: [No]
[CEA-708] Priority: [3]
[CEA-708] Row count: [2] # this
[CEA-708] Column count: [32]
[CEA-708] Anchor point: [0]
[CEA-708] Anchor vertical: [65]
[CEA-708] Anchor horizontal: [0]
[CEA-708] Relative pos: [No]
[CEA-708] Row lock: [Yes]
[CEA-708] Column lock: [Yes]
[CEA-708] Pen style: [0] # this
[CEA-708] Win style: [2]

Also, from debug output it's clear that some lines lost.

Adding an option will at least fix this subs, why not?

@cactusGit commented on GitHub (Sep 21, 2016): Yep, it's 608, but they're mostly identical, when this line commented. I've missed that memcmp. But if we'll look in debug output, there are small differences between these definitions: ``` [CEA-708] dtvcc_handle_DFx_DefineWindow: W[1], attributes: [CEA-708] Visible: [No] [CEA-708] Priority: [3] [CEA-708] Row count: [1] # this [CEA-708] Column count: [32] [CEA-708] Anchor point: [0] [CEA-708] Anchor vertical: [65] [CEA-708] Anchor horizontal: [0] [CEA-708] Relative pos: [No] [CEA-708] Row lock: [Yes] [CEA-708] Column lock: [Yes] [CEA-708] Pen style: [4] # this [CEA-708] Win style: [2] ... [CEA-708] dtvcc_handle_DFx_DefineWindow: W[1], attributes: [CEA-708] Visible: [No] [CEA-708] Priority: [3] [CEA-708] Row count: [2] # this [CEA-708] Column count: [32] [CEA-708] Anchor point: [0] [CEA-708] Anchor vertical: [65] [CEA-708] Anchor horizontal: [0] [CEA-708] Relative pos: [No] [CEA-708] Row lock: [Yes] [CEA-708] Column lock: [Yes] [CEA-708] Pen style: [0] # this [CEA-708] Win style: [2] ``` Also, from debug output it's clear that some lines lost. Adding an option will at least fix this subs, why not?

claunia commented

2026-01-29 16:37:22 +00:00

@cfsmp3 commented on GitHub (Sep 21, 2016):

All efforts are focused on getting things right (when possible), not just make it work for a specific sample. It's clear that the assumption that the window definition having to be identical or you have to clear is wrong. But I don't want to just remove the clear - that breaks the sample for which it was added in the first place.
I don't want to add a new parameter to switch between different behavior either. People will rely on this parameter being available even when it's not needed.
The problem is clear, and you made the effort of looking at the differences (which is really useful, thanks) so let's fix it correctly. I'll reread the specs in a few hours.

My guess

Makes sense that an increase in the row count doesn't require a clear - probably just means that a new line of text is going to be added, and in fact they bother not to ask for a window larger than needed at each time.
Null pen is OK according to wikipedia.

But if the specs are specific on which fields means 'clear window' and which mean 'change definition, leave text alone' I'd rather follow them :-)

@cfsmp3 commented on GitHub (Sep 21, 2016): All efforts are focused on getting things right (when possible), not just make it work for a specific sample. It's clear that the assumption that the window definition having to be identical or you have to clear is wrong. But I don't want to just remove the clear - that breaks the sample for which it was added in the first place. I don't want to add a new parameter to switch between different behavior either. People will rely on this parameter being available even when it's not needed. The problem is clear, and you made the effort of looking at the differences (which is really useful, thanks) so let's fix it correctly. I'll reread the specs in a few hours. My guess - Makes sense that an increase in the row count doesn't require a clear - probably just means that a new line of text is going to be added, and in fact they bother not to ask for a window larger than needed at each time. - Null pen is OK according to wikipedia. But if the specs are specific on which fields means 'clear window' and which mean 'change definition, leave text alone' I'd rather follow them :-)

claunia commented

2026-01-29 16:37:23 +00:00

@cactusGit commented on GitHub (Sep 21, 2016):

Found in CEA-708-D specs:

@cactusGit commented on GitHub (Sep 21, 2016): Found in CEA-708-D specs: ![](https://cloud.githubusercontent.com/assets/10499493/18694786/aadd3764-7fb5-11e6-988a-2de48914317e.png)

claunia commented

2026-01-29 16:37:24 +00:00

@cfsmp3 commented on GitHub (Sep 21, 2016):

Yes, CEA-708 is quite unspecific with the definition of "being updated". I've changed the behavior so the window is only cleared if the window style changes, but I don't know but sure if that's a correct interpretation or not.
Anyway if you are willing to do the testing work on 708 I'll make the effort to improve the implementation.
I'm closing the ticket for now as it seems to be OK for your sample now. Feel free to reopen if needed.

@cfsmp3 commented on GitHub (Sep 21, 2016): Yes, CEA-708 is quite unspecific with the definition of "being updated". I've changed the behavior so the window is only cleared if the window style changes, but I don't know but sure if that's a correct interpretation or not. Anyway if you are willing to do the testing work on 708 I'll make the effort to improve the implementation. I'm closing the ticket for now as it seems to be OK for your sample now. Feel free to reopen if needed.

claunia referenced this issue

2026-01-29 16:58:55 +00:00

[PR #185] [MERGED] Corrected typos in help info #1068

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/ccextractor#185