fps switching in HISTORY channel #188

New Issue

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

claunia commented

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

Originally created by @cfsmp3 on GitHub (Oct 4, 2016).

Originally assigned to: @cfsmp3, @anshul1912 on GitHub.

Processing a HISTORY (writing it in uppercase because it seems to be a big deal to them) TS we get this:

67% | 00:16Changed fps using NAL to: 29.970030
72% | 00:18Changed fps using NAL to: 29.970030
repeated lots of times

A sample file:
https://drive.google.com/file/d/0BxRlGXOr_NhUNmh0NXg3S2dHeFE/view?usp=sharing

The FPS changes to that value here:

void seq_parameter_set_rbsp (struct avc_ctx *ctx, unsigned char *seqbuf, unsigned char *seqend)
{
[...]
if (current_fps != (double)time_scale / (2 * num_units_in_tick))
{
current_fps = (double)time_scale / (2 * num_units_in_tick); // Based on formula D-2, p. 359 of the ISO/IEC 14496-10:2012(E) spec.
mprint("Changed fps using NAL to: %f\n", current_fps);
}
}

However it's going back to double that FPS (however it's not logged) here:

void slice_header (struct lib_cc_decode *ctx, unsigned char *heabuf, unsigned char *heaend, int nal_unit_type, struct cc_subtitle *sub)
{
[...]
// TODO - Do this right.
// When bottom_field_flag is set the video is interlaced,
// override current_fps.
current_fps = framerates_values[7];

Where that TODO comment really seems to apply.

Originally created by @cfsmp3 on GitHub (Oct 4, 2016). Originally assigned to: @cfsmp3, @anshul1912 on GitHub. Processing a HISTORY (writing it in uppercase because it seems to be a big deal to them) TS we get this: 67% | 00:16Changed fps using NAL to: 29.970030 72% | 00:18Changed fps using NAL to: 29.970030 repeated lots of times A sample file: https://drive.google.com/file/d/0BxRlGXOr_NhUNmh0NXg3S2dHeFE/view?usp=sharing The FPS changes to that value here: void seq_parameter_set_rbsp (struct avc_ctx *ctx, unsigned char *seqbuf, unsigned char *seqend) { [...] if (current_fps != (double)time_scale / (2 \* num_units_in_tick)) { current_fps = (double)time_scale / (2 \* num_units_in_tick); // Based on formula D-2, p. 359 of the ISO/IEC 14496-10:2012(E) spec. mprint("Changed fps using NAL to: %f\n", current_fps); } } However it's going back to double that FPS (however it's not logged) here: void slice_header (struct lib_cc_decode *ctx, unsigned char *heabuf, unsigned char *heaend, int nal_unit_type, struct cc_subtitle *sub) { [...] // TODO - Do this right. // When bottom_field_flag is set the video is interlaced, // override current_fps. current_fps = framerates_values[7]; Where that TODO comment really seems to apply.

claunia closed this issue

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

claunia commented

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

@mackworth commented on GitHub (Oct 10, 2016):

I'm seeing this on multiple channels on Comcast.

@mackworth commented on GitHub (Oct 10, 2016): I'm seeing this on multiple channels on Comcast.

claunia commented

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

@cfsmp3 commented on GitHub (Oct 13, 2016):

I've been looking into this a bit more. The issue is probably more serious than it seems.

The variable current_fps is global, not part of the context. This alone is probably very wrong since we may be processing both interlaced and non-interlaced streams at the same time if they are multiplexed. Probably even at different FPS, even though that's likely just a theoretical case.
The line current_fps = framerates_values[7]; which is the same as current_fps=29.97*2 is definitely wrong as it pretty much means our timing is always going to be wrong when processing European AVC streams (well, unless it's working by pure luck due to the code that actually sets the fps to the right value as mentioned in the initial post).

I'm assigning this to Anshul (since he did the context refactoring - Anshul, do you remember why current_fps was left as a global?) and myself.

@cfsmp3 commented on GitHub (Oct 13, 2016): I've been looking into this a bit more. The issue is probably more serious than it seems. - The variable current_fps is global, not part of the context. This alone is probably very wrong since we may be processing both interlaced and non-interlaced streams at the same time if they are multiplexed. Probably even at different FPS, even though that's likely just a theoretical case. - The line current_fps = framerates_values[7]; which is the same as current_fps=29.97*2 is definitely wrong as it pretty much means our timing is always going to be wrong when processing European AVC streams (well, unless it's working by pure luck due to the code that actually sets the fps to the right value as mentioned in the initial post). I'm assigning this to Anshul (since he did the context refactoring - Anshul, do you remember why current_fps was left as a global?) and myself.

claunia commented

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

@cfsmp3 commented on GitHub (Nov 9, 2016):

Code-in task created.

@cfsmp3 commented on GitHub (Nov 9, 2016): Code-in task created.

claunia commented

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

@ghost commented on GitHub (Nov 29, 2016):

Error is still here as of 0.82. Number of times it appears is equal to the number of jump-in frames.

@ghost commented on GitHub (Nov 29, 2016): Error is still here as of 0.82. Number of times it appears is equal to the number of jump-in frames.

claunia commented

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

@Izaron commented on GitHub (Dec 10, 2016):

Why we use this line? https://github.com/CCExtractor/ccextractor/blob/master/src/lib_ccx/avc_functions.c#L925
Why we can't use current_fps = framerates_values[ctx->current_frame_rate];? As in this value write https://github.com/CCExtractor/ccextractor/blob/master/src/lib_ccx/es_functions.c#L435
If we use ctx->current_frame_rate, it does not seem to switch the fps, but probably it is just hiding the problem.

The variable current_fps is global, not part of the context

Probably, this variable can be inserted into some data structure, but there is still a lot of global variables

@Izaron commented on GitHub (Dec 10, 2016): Why we use this line? [https://github.com/CCExtractor/ccextractor/blob/master/src/lib_ccx/avc_functions.c#L925](https://github.com/CCExtractor/ccextractor/blob/master/src/lib_ccx/avc_functions.c#L925) Why we can't use `current_fps = framerates_values[ctx->current_frame_rate];`? As in this value write [https://github.com/CCExtractor/ccextractor/blob/master/src/lib_ccx/es_functions.c#L435](https://github.com/CCExtractor/ccextractor/blob/master/src/lib_ccx/es_functions.c#L435) If we use `ctx->current_frame_rate`, it does not seem to switch the fps, but probably it is just hiding the problem. > The variable current_fps is global, not part of the context Probably, this variable can be inserted into some data structure, but there is still a lot of global variables

claunia commented

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

@cfsmp3 commented on GitHub (Dec 13, 2016):

Confirmed fixed for HISTORY2.TS.

@cfsmp3 commented on GitHub (Dec 13, 2016): Confirmed fixed for HISTORY2.TS.

claunia referenced this issue

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

[PR #188] [MERGED] remove compiler warnings #1071

Sign in to join this conversation.