CCExtractor / CCExtractor/ccextractor

BUG: SCC timecodes use hardcoded 29.97 FPS instead of actual stream frame rate

Đang mở
#2,145 10 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
C
Star
903
Fork
589
Merge trung bình
3 ngày 2 giờ
Pull request đã merge (30 ngày)
10

Mô tả

## Summary

`print_scc_time()` in `src/lib_ccx/ccx_common_timing.c` computes SCC timecode frame numbers using a hardcoded literal `29.97`, completely ignoring the `current_fps` global that exists in the same file and is dynamically updated from stream NAL data at runtime.

Any SCC output from a 24fps, 25fps, 30fps, or 50/60fps source will have **incorrect frame numbers in every timecode**, making the output non-standard and potentially rejected by downstream validators.

---

## Affected File

`src/lib_ccx/ccx_common_timing.c` — Line 125

---

## Reproduction
```bash
grep -n "29\.97" src/lib_ccx/ccx_common_timing.c
```
```
22: double current_fps = (double)30000.0 / 1001; /* 29.97 */ // TODO: Get from framerates_values[] instead
125: frame = ((double)(time.time_in_ms - 1000 * (time.ss + 60 * (time.mm + 60 * time.hh))) * 29.97 / 1000);
```

---

## Root Cause

`current_fps` is already updated dynamically in two places:

- `avc_functions.c:891` — from stream NAL timing data
- `avc_functions.c:991` and `es_functions.c:442` — from `framerates_values[current_frame_rate]`

`print_scc_time()` is simply the one function that was never updated to use it.

---

## Impact

- All non-NTSC sources (24fps film, 25fps PAL, 30fps progressive) produce SCC timecodes with wrong frame numbers
- 25fps PAL broadcasts — common across Europe — see a ~20% frame count error
- Downstream broadcast validators may reject the output or apply incorrect sync

---

## Suggested Fix

Single-token change on line 125:
```c
// BEFORE:
frame = ((double)(time.time_in_ms - 1000 * (time.ss + 60 * (time.mm + 60 * time.hh))) * 29.97 / 1000);

// AFTER:
frame = ((double)(time.time_in_ms - 1000 * (time.ss + 60 * (time.mm + 60 * time.hh))) * current_fps / 1000);
```

`current_fps` is a file-level global in the same translation unit — no header changes needed. Happy to submit a PR for this.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.