There is some signedness 'bugs' in ogm rip, I'll attatch the diffs when this bug is opened. (please forward the patch or tell me to) Reproducible: Always Steps to Reproduce: 1. 2. 3.
Created attachment 66516 [details, diff] Fixes for the subrip sources. + int pts100; /* FIXME: Should perhaps be time_t */ I don't know how large that timestamp can be, ir if it's a delta or not..
Created attachment 66517 [details, diff] Signedness fixes for libogmrip.
- unsigned int pts100; + int pts100; /* FIXME: Should perhaps be time_t */ if that comment is true then it should be fixed ... time_t is not always the same size as int depending on the host architecture, so passing references/pointers could cause issues
(In reply to comment #3) > - unsigned int pts100; > + int pts100; /* FIXME: Should perhaps be time_t */ > > if that comment is true then it should be fixed ... time_t is not always the > same size as int depending on the host architecture, so passing > references/pointers could cause issues well as i stated in the patch comment, if its just a delta or so then it's ok i assume... but...(In reply to comment #3)
The patch to fix the signedness in libogmrip makes ogmrip compile cleanly with gcc4 but this is not the right way to fix this issue. In fact, there is a bug in ogmrip_codec_get_chapters and ogmrip_codec_set_chapters definition because the last chapter to encode might be negative if it should be the last chapter of the title. That is, the 'end' parameter should be gint, not guint. I'll provide a patch as soon as I'm back from holydays.
Regarding the subrip patch, I think it should be better to change int vobsub_get_next_packet (void *vobhandle, void **data, int *timestamp) to int vobsub_get_next_packet (void *vobhandle, void **data, unsigned int *timestamp) in both vobsub.h and vobsub.c instead of removing the unsigned qualifier to pts100 in spudec.c.
Created attachment 66595 [details, diff] Something more along the lines of this? I think i cought all of the comments, I wonder about set_chapters though, should that also use signed ints?
that looks acceptable to me at a glance perhaps try contacting the upstream maintainers too while you're at it ? :)
Created attachment 66778 [details, diff] gcc4 patch This patch improves Ian's patch: it defines the get_chapters and set_chapters functions correctly. Thanks Ian for your work.
(In reply to comment #9) > This patch improves Ian's patch: it defines the get_chapters and set_chapters > functions correctly. Thanks Ian for your work. NP =), Glad i could help =)
Patch commited in CVS. Special thanks all for your work.
