We're not checking if enc_context is correctly created #155

New Issue

claunia · 2026-01-29T16:36:32Z

claunia commented

2026-01-29 16:36:32 +00:00

Originally created by @cfsmp3 on GitHub (Jun 3, 2016).

Originally assigned to: @anshul1912 on GitHub.

'/home/captions/ccextractorgit/ccextractor/linux/ccextractor', '-nobom', '-quiet', '-txt', '-sem', '-outinterval', '10', '-s', '-o', '/home/captions/capture/work/DIS/.txt', '-udp', '127.0.0.1:7303'

will, if the directory not exist, loop forever like this:
Error: Failed /home/captions/capture/work/DIS/_000001.txt
Error: Failed /home/captions/capture/work/DIS/_000001.txt
Error: Failed /home/captions/capture/work/DIS/_000001.txt
Error: Failed /home/captions/capture/work/DIS/_000001.txt

The fundamental reason is that after this line, in general_loop()

enc_ctx = update_encoder_list_cinfo(ctx, cinfo);

We aren't checking for enc_ctx==NULL.

That call goes deep into a few functions. The one that actually tries to open the file

check_ret(cfg->output_filename);

where check_ret is a define

define check_ret(filename) if (ret != EXIT_OK) \

            {                                   \
                print_error(cfg->gui_mode_reports,"Failed %s\n", filename); \
                return ret;                         \
            }

so well, NULL is returned up and up until it reaches that updater_encoder_list. We don't do anything there, we're happy to continue with no encode context.

So question - is there any case in which not having a context can produce any kind of desirable results? (serious question, not being passive aggressive).

I'm assigning this to Anshul so he can give feedback.

Originally created by @cfsmp3 on GitHub (Jun 3, 2016). Originally assigned to: @anshul1912 on GitHub. '/home/captions/ccextractorgit/ccextractor/linux/ccextractor', '-nobom', '-quiet', '-txt', '-sem', '-outinterval', '10', '-s', '-o', '/home/captions/capture/work/DIS/.txt', '-udp', '127.0.0.1:7303' will, if the directory not exist, loop forever like this: Error: Failed /home/captions/capture/work/DIS/_000001.txt Error: Failed /home/captions/capture/work/DIS/_000001.txt Error: Failed /home/captions/capture/work/DIS/_000001.txt Error: Failed /home/captions/capture/work/DIS/_000001.txt The fundamental reason is that after this line, in general_loop() enc_ctx = update_encoder_list_cinfo(ctx, cinfo); We aren't checking for enc_ctx==NULL. That call goes deep into a few functions. The one that actually tries to open the file check_ret(cfg->output_filename); where check_ret is a define # define check_ret(filename) if (ret != EXIT_OK) \ ``` { \ print_error(cfg->gui_mode_reports,"Failed %s\n", filename); \ return ret; \ } ``` so well, NULL is returned up and up until it reaches that updater_encoder_list. We don't do anything there, we're happy to continue with no encode context. So question - is there _any_ case in which not having a context can produce any kind of desirable results? (serious question, not being passive aggressive). I'm assigning this to Anshul so he can give feedback.

claunia closed this issue

2026-01-29 16:36:32 +00:00

claunia commented

2026-01-29 16:36:32 +00:00

@cfsmp3 commented on GitHub (Jan 20, 2017):

GSoC qualification: This issues gives 3 points.

@cfsmp3 commented on GitHub (Jan 20, 2017): GSoC qualification: This issues gives 3 points.

claunia commented

2026-01-29 16:36:33 +00:00

@saurabhshri commented on GitHub (Jan 24, 2017):

@cfsmp3 Alright, I took a dig in. I supplied invalid directory/ non permissible directories and this is 100% replicable.

I tried debugging.

VBI/teletext stream ID 41 (0x29) for SID 785 (0x311)
This is GENERAL LOOP , count = 1
update_encoder_list_cinfo
init_encoder
Creating kdjksd/sdksl
Error: Failed kdjksd/sdksl, count is :1

enc_ctx init_encoder = 5
enc_ctx in update_encoder_list_cinfo = 0
enc_ctx in general loop is  = (null)
This is GENERAL LOOP , count = 2
update_encoder_list_cinfo
init_encoder
Creating kdjksd/sdksl
Error: Failed kdjksd/sdksl, count is :2

enc_ctx init_encoder = 5
enc_ctx in update_encoder_list_cinfo = 0
enc_ctx in general loop is  = (null)
Segmentation fault

is there any case in which not having a context can produce any kind of desirable results

I tried a lot of samples but I am really not sure about this. But I think maybe even if enc_ctx is NULL in an iteration of general_loop, there's a possibility of it getting created properly in next ones (maybe by switch of stream / proper stdin or something), but I am not really sure.

While you were getting infinite loop, I got segfault for both stdin and normal file input.

For now I think we should just check if the path supplied in -o is feasible or not. Because there is really no need to process a file if no output is to be generated. Also that should prevent this or now, and as we get clear about this, we can solve it further.

Moreover, I also found out that when using stdin, if there is no -o argument, the file created is named .srt . I think we should change it to have a better name, such as maybe, stdin.srt because just .srt is basically a hidden file.

@saurabhshri commented on GitHub (Jan 24, 2017): @cfsmp3 Alright, I took a dig in. I supplied invalid directory/ non permissible directories and this is 100% replicable. I tried debugging. ``` VBI/teletext stream ID 41 (0x29) for SID 785 (0x311) This is GENERAL LOOP , count = 1 update_encoder_list_cinfo init_encoder Creating kdjksd/sdksl Error: Failed kdjksd/sdksl, count is :1 enc_ctx init_encoder = 5 enc_ctx in update_encoder_list_cinfo = 0 enc_ctx in general loop is = (null) This is GENERAL LOOP , count = 2 update_encoder_list_cinfo init_encoder Creating kdjksd/sdksl Error: Failed kdjksd/sdksl, count is :2 enc_ctx init_encoder = 5 enc_ctx in update_encoder_list_cinfo = 0 enc_ctx in general loop is = (null) Segmentation fault ``` >is there any case in which not having a context can produce any kind of desirable results I tried a lot of samples but I am really not sure about this. But I think maybe even if `enc_ctx` is NULL in an iteration of `general_loop`, there's a possibility of it getting created properly in next ones (maybe by switch of stream / proper stdin or something), but I am not really sure. While you were getting infinite loop, I got segfault for both stdin and normal file input. For now I think we should just check if the path supplied in `-o` is feasible or not. Because there is really no need to process a file if no output is to be generated. Also that should prevent this or now, and as we get clear about this, we can solve it further. Moreover, I also found out that when using stdin, if there is no `-o` argument, the file created is named `.srt` . I think we should change it to have a better name, such as maybe, `stdin.srt` because just `.srt` is basically a hidden file.

claunia commented

2026-01-29 16:36:33 +00:00

@cfsmp3 commented on GitHub (Jan 30, 2017):

OK, there's a case in which it's OK not to have a encoder context - when the output is NULL, which is OK if you just want the EPG data.

Maybe if we cannot open the output file we should just fatal() ?

@cfsmp3 commented on GitHub (Jan 30, 2017): OK, there's a case in which it's OK not to have a encoder context - when the output is NULL, which is OK if you just want the EPG data. Maybe if we cannot open the output file we should just fatal() ?

claunia commented

2026-01-29 16:36:33 +00:00

@saurabhshri commented on GitHub (Jan 31, 2017):

@cfsmp3 I think it will be a good option for now. I'll make the changes.

@saurabhshri commented on GitHub (Jan 31, 2017): @cfsmp3 I think it will be a good option for now. I'll make the changes.

claunia referenced this issue

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

[PR #155] [MERGED] handling 7-byte PES packets for dvb-subs stream #1042

claunia referenced this issue

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

[PR #155] handling 7-byte PES packets for dvb-subs stream #1045

Sign in to join this conversation.