Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

return a non-zero return code if no subtitles were found #553

Merged
merged 2 commits into from Jan 8, 2017
Merged

return a non-zero return code if no subtitles were found #553

merged 2 commits into from Jan 8, 2017

Conversation

ghost
Copy link

@ghost ghost commented Dec 18, 2016

Previous PR: #535

@cfsmp3
Copy link
Contributor

cfsmp3 commented Dec 22, 2016

Could work. @canihavesomecoffee what does the CI say?
@anukul I can't see a pending code-in task, did you claim and submit?

@ghost
Copy link
Author

ghost commented Dec 22, 2016

Yes, you had approved it tentatively. It is awaiting parental consent approval for completion.

@canihavesomecoffee
Copy link
Member

canihavesomecoffee commented Dec 22, 2016

The PR is almost fine.

@anukul:

Test 28-30 is incorrectly returning status code 10, while the output is non-empty. They're all 3 .mov files.
Test 139 should be returning 10, but seemingly returns 0.

@ghost
Copy link
Author

ghost commented Dec 24, 2016

Test 139 looks fine to me.

➜  fix ./ccextractor inp.mpg
CCExtractor 0.83, Carlos Fernandez Sanz, Volker Quetschke.
Teletext portions taken from Petr Kutalek's telxcc
--------------------------------------------------------------------------
Input: inp.mpg
[Extract: 1] [Stream mode: Autodetect]
[Program : Auto ] [Hauppage mode: No] [Use MythTV code: Auto]
[Timing mode: Auto] [Debug: No] [Buffer input: No]
[Use pic_order_cnt_lsb for H.264: No] [Print CC decoder traces: No]
[Target format: .srt] [Encoding: UTF-8] [Delay: 0] [Trim lines: No]
[Add font color data: Yes] [Add font typesetting: Yes]
[Convert case: No] [Video-edit join: No]
[Extraction start time: not set (from start)]
[Extraction end time: not set (to end)]
[Live stream: No] [Clock frequency: 90000]
[Teletext page: Autodetect]
[Start credits text: None]

-----------------------------------------------------------------
Opening file: inp.mpg
File seems to be a transport stream, enabling TS mode
Analyzing data in general mode
Creating inp.srt


New video information found
[1920 * 1080] [AR: 03 - 16:9] [FR: 04 - 29.97] [progressive: no]

100%  |  00:19
Number of NAL_type_7: 0
Number of VCL_HRD: 0
Number of NAL HRD: 0
Number of jump-in-frames: 0
Number of num_unexpected_sei_length: 0

Total frames time:	  00:00:20:020  (600 frames at 29.97fps)

Min PTS:				00:00:00:400
Max PTS:				00:00:20:453
Length:				 00:00:20:053

Initial GOP time:	   00:00:00:000
Final GOP time:		 00:00:19:500+15F
Diff. GOP length:	   00:00:19:500+15F	(00:00:20:000)


Total user data fields: 600
HDTV type user data fields: 600
Done, processing time = 1 seconds
This is beta software. Report issues to carlos at ccextractor org...
➜  fix wc -l inp.srt 
15 inp.srt
➜  fix 

As for tests 28-30, I don't think I'll be able to download those files with my current internet connection.
image
I'll change to another ISP by 28th December and try to fix those.

@ghost
Copy link
Author

ghost commented Dec 28, 2016

Okay, I checked 28 and my workaround for mp4 is incorrect. I had assumed that ctx->freport.mp4_cc_track_cnt would store a non zero value in the case when captions are present. But I guess it's not so.

@cfsmp3
Copy link
Contributor

cfsmp3 commented Dec 28, 2016 via email

@ghost
Copy link
Author

ghost commented Dec 29, 2016

The last commit fixes Tests 28, 29, 30.
I'm not completely convinced as to why Test 139 should be returning 10, it does generate a non-empty subtitle file.

@ghost
Copy link
Author

ghost commented Dec 31, 2016

break;
case CCX_SM_MP4:
mprint ("\rAnalyzing data with GPAC (MP4 library)\n");
close_input_file(ctx); // No need to have it open. GPAC will do it for us
processmp4(ctx, &ctx->mp4_cfg, ctx->inputfile[ctx->current_file]);
if (ccx_options.print_file_reports)
print_file_report(ctx);
tmp = ctx->freport.mp4_cc_track_cnt;
if (!ret) ret = tmp;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why tmp variable needed, why cant you just capture the return value in ret variable

Copy link
Author

@ghost ghost Jan 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because of this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants