Refactor/move demuxer options to struct#2102
Refactor/move demuxer options to struct#2102AhmedAlian7 wants to merge 3 commits intoCCExtractor:masterfrom
Conversation
…_demuxer struct Resolves the TODO in file_functions.c:311 by encapsulating demuxer-related options within the ccx_demuxer struct instead of accessing the global ccx_options. Changes: - Added live_stream, buffer_input, input_source, binary_concat to struct ccx_demuxer - Updated init_demuxer() to initialize these from global options - Updated file_functions.c and ccx_demuxer.c to use context fields instead of globals - Updated changelog
8b9acf1 to
42e513d
Compare
|
Code passes all tests locally, but failed on GitHub CI cuz github server issues |
cfsmp3
left a comment
There was a problem hiding this comment.
This PR doesn't pass the build CI. Please fix the build failures before requesting review.
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
@cfsmp3 building failure is fixed, sorry for not noticing thatThe issue due to:
Changes1.
|
|
@cfsmp3, plz review this one. |
CCExtractor CI platform finished running the test files on linux. Below is a summary of the test results, when compared to test for commit 6f7ce27...:
Your PR breaks these cases:
Congratulations: Merging this PR would fix the following tests:
It seems that not all tests were passed completely. This is an indication that the output of some files is not as expected (but might be according to you). Check the result page for more info. |
CCExtractor CI platform finished running the test files on windows. Below is a summary of the test results, when compared to test for commit 6f7ce27...:
Your PR breaks these cases:
Congratulations: Merging this PR would fix the following tests:
It seems that not all tests were passed completely. This is an indication that the output of some files is not as expected (but might be according to you). Check the result page for more info. |
cfsmp3
left a comment
There was a problem hiding this comment.
This refactoring has a regression: the options are copied from ccx_options into the demuxer struct once at init time (init_demuxer in ccx_demuxer.c:418), but some code mutates these options at runtime and expects the read logic to see the changes.
Specifically:
-
MythTV buffering:
myth_loopinmyth.c:813togglesccx_options.buffer_inputat runtime. After this PR, the read logic infile_functions.cusesctx->buffer_input(the init-time copy), so it never sees the MythTV override. This breaks MythTV buffering behavior. -
Live stream timeout:
sleepandchecktimeoutsetsccx_options.live_stream = 0when the timeout expires or stdin is detected. Your PR correctly updatesctx->live_streaminstead, but any other code that reads the globalccx_options.live_streamafter that point would still see the old value, creating an inconsistency.
The fundamental issue is that you can't half-migrate mutable global state — if any code path still mutates the global expecting readers to see it, copying once at init breaks that contract. To do this properly you'd need to also update all runtime mutation sites (myth.c, etc.) to write to the demuxer-local copies instead of the global, and verify no other code reads the global after init.
[IMPROVEMENT]
In raising this pull request, I confirm the following (please check boxes):
My familiarity with the project is as follows (check one):
closes #2101
Summary
Resolves the TODO in
file_functions.c:311by encapsulating demuxer-related options within the ccx_demuxer struct instead of accessing the globalccx_options.Problem
Multiple functions directly accessed global
ccx_optionsfor 4 options that logically belong to the demuxer context:live_stream- Live stream mode and timeoutbuffer_input- Whether to buffer input datainput_source- Source type (file, stdin, network, tcp)binary_concat- Whether to concatenate binary filesThis caused issues with testability, encapsulation, thread safety, and library reusability.
Changes
src/lib_ccx/ccx_demuxer.h
Added 4 new fields to
struct ccx_demuxer:src/lib_ccx/ccx_demuxer.c
init_demuxer() - Initialize new fields from global options:
ccx_demuxer_close() - Updated to use
ctx->input_source.ccx_demuxer_open() - Updated all input source checks to use
ctx->input_source.src/lib_ccx/file_functions.c
sleepandchecktimeout() - Added
ctxparameter.buffered_read_opt() - Updated all 4 options to use
ctx->.get_total_file_size() - Updated to use
ctx->demux_ctx->live_stream.switch_to_next_file() - Updated all options to use
ctx->demux_ctx->.Testing
makein linux directory)ccextractor --version)Impact
ccx_optionsat demuxer creation