mirror of
https://github.com/CCExtractor/ccextractor.git
synced 2026-02-15 21:23:10 +00:00
[QUESTION] get_video_min_pts function not called anywhere
#614
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @siv2r on GitHub (Mar 4, 2021).
CCExtractor version: master branch (4 Mar 2021)
Necessary information
No arguments used.Additional information
I noticed that the
get_video_min_ptsfunction is not used anywhere.cf84757e02/src/lib_ccx/ts_functions.c (L667-L668)Also the code which calls
get_video_min_ptsis commented.cf84757e02/src/lib_ccx/ts_functions.c (L746)Is there any specific reason for this?
@NilsIrl commented on GitHub (Mar 4, 2021):
may be relevant, the commit where this got commented out:
462f63a294 (diff-8ea29240ddcd2096742634831a5e214ec954ed35d04dc554742d1e65592a026cR734-R735)@cfsmp3 commented on GitHub (Mar 28, 2021):
Well, the commit says "fixes DVB multiprogram" :-) It's from 4 years ago. @alexbrt did this during GCI. I'm not sure he's still alive? :-)
Alex?
But the commit description itself is a good clue to what to test next.
https://drive.google.com/drive/u/5/folders/0B_61ywKPmI0TQTZEMm5Tajd5RVU
(or other DVB samples, looking specifically for ones with more than one stream)
@KW781 commented on GitHub (Apr 13, 2021):
I should preface by saying that I'm somewhat new to the CCExtractor codebase, especially compared to some of the major contributors to CCExtractor, but I decided to report my findings anyway.
I spent some time digging through all the files in lib_ccx, and that function wasn't called in any other C file either. So it's clear that the function was written to be used within ts_functions.c only. From looking at the structure definition of ccx_demuxer and at the definition of UINT64_MAX, it looks like UINT64_MAX (the maximum width of an int64 type), is the default value for the got_important_streams_min_pts[VIDEO] member for that structure. So basically, it's checking whether the minimum presentation time stamp for that video stream is the default, and if so, finds the minimum presentation time stamp for that video stream.
To try and figure out why it was commented out, I traced through all the function calls to main(). get_video_min_pts() is called in ts_readstream(), ts_readstream() is called in ts_get_more_data, ts_get_more_data() is called in general_loop(), general_loop() is called in api_start() and then api_start() is called in main(). What I noticed in all of these functions is that the member got_important_streams_min_pts[VIDEO] for *ctx is not referenced at all, except for in that code that is commented out. So my guess is that the code was commented out because that member wasn't being used at all, which is why it didn't need to be assigned a value from get_video_min_pts().
I assume that the function exists just to find the minimum presentation time stamp for any given video stream, if required. We could get rid of the comment on that code, since it might be good practice to populate that member anyway, and to avoid future confusion like this. But it's probably up to the more senior contributors to decide on whether to do this or not.
@alexbrt commented on GitHub (Apr 13, 2021):
@cfsmp3 I am not dead (yet), haha, just very busy with work and university.
@KW781 @siv2r I do have some arcane findings though which could explain the confusion. Fixing overall DVB support was done in 2 stages, the first one being for single-program streams followed by the multi-program commit (
462f63a) referenced in this issue.Quoting from my GCI presentation, the reason I commented out that function:
I can only conclude that
get_video_min_ptsshould have been completely deleted instead of being commented out, and is now just a leftover from the old code.To further clear things up, this is the presentation I'm talking about which briefly summarizes @Izaron's and my work:
https://docs.google.com/presentation/d/14hKczN-fbeTZLd_CeZQILZYz4Ylnhaj05PEEwEd_Q7c/edit?usp=sharing
@KW781 commented on GitHub (Apr 13, 2021):
@alexbrt Ah I see, so that's why that member (got_important_streams_min_pts[VIDEO])for *ctx wasn't being used at all?
@cfsmp3 commented on GitHub (May 2, 2021):
Function removed