[PATCH] "dvdnav_jump_to_sector" as an alternative to "dvdnav_time_search" (REV 6: add mode argument)
This revision features one minor change. It adds a new "mode" parameter to "dvdnav_jump_to_sector". As per the included comment: /* NOTE: Mode is currently unimplemented. Only 0 should be passed. */ /* 1 and -1 are for future implementation */ /* 0: Default. Jump to a time which may be either <> time_in_pts_ticks */ /* 1: After. Always jump to a time that is > time_in_pts_ticks */ /* -1: Before. Always jump to a time that is < time_in_pts_ticks */ dvdnav_status_t dvdnav_jump_to_sector_by_time(dvdnav_t *this, uint64_t time_in_pts_ticks, int32_t mode) ... The corresponding .h interface was also updated. My apologies for not implementing the new parameter, but as I detailed in another email, the change looks quite significant. As far as I know, there are no other known pending changes/requests. I am hoping that this patch is at a final state. Please let me know what else should be done to get this patch accepted. Thanks.
On Wed, Apr 25, 2012 at 9:48 PM, gnosygnu <gnosygnu@gmail.com> wrote:
This revision features one minor change. It adds a new "mode" parameter to "dvdnav_jump_to_sector".
As per the included comment:
/* NOTE: Mode is currently unimplemented. Only 0 should be passed. */ /* 1 and -1 are for future implementation */ /* 0: Default. Jump to a time which may be either <> time_in_pts_ticks */ /* 1: After. Always jump to a time that is > time_in_pts_ticks */ /* -1: Before. Always jump to a time that is < time_in_pts_ticks */ dvdnav_status_t dvdnav_jump_to_sector_by_time(dvdnav_t *this, uint64_t time_in_pts_ticks, int32_t mode)
Looks like the right interface to me. As a small nit, maybe return DVDNAV_STATUS_ERR instead of NULL on failure. and use int32_t result = DVDNAV_STATUS_OK; instead of int32_t result = 0; Also maybe fail if they pass in anything besides 0 for the mode parameter of dvdnav_jump_to_sector_by_time. Also would it be possible to ask for a write up some where (maybe on a wiki somewhere?) that describes the overall general process used? Like "you do this, then that". It's so poorly documented on the web, it would be nice to get a brain dump. Thanks! -roger-
Hi. Sorry for the late reply. I've been checking email very sporadically.
As a small nit, maybe return DVDNAV_STATUS_ERR instead of NULL on failure.
and use
int32_t result = DVDNAV_STATUS_OK;
instead of int32_t result = 0;
Thanks. You are completely correct. I should be using constants instead of numbers I changed them accordingly in my version int32_t result = DVDNAV_STATUS_ERR; result = DVDNAV_STATUS_OK;
Also maybe fail if they pass in anything besides 0 for the mode parameter of dvdnav_jump_to_sector_by_time.
Sure. I can add a "if (mode != 0) return DVDNAV_STATUS_ERR;" I'll change the "!=0" to a constant later.
Also would it be possible to ask for a write up some where (maybe on a wiki somewhere?) that describes the overall general process used? Like "you do this, then that". It's so poorly documented on the web, it would be nice to get a brain dump.
I tried to describe as much in my initial email a year ago. See the "Details" section in the following link: http://lists.mplayerhq.hu/pipermail/dvdnav-discuss/2011-November/001551.html Feel free to let me know if anything needs specific clarification. If you want it posted to a wiki, let me know where, and I'll document accordingly. For anyone else: I know this topic is a year old, and doesn't seem to generate much interest outside of Roger (and Joakim). However, I would like to get this patch accepted. It is working and addresses a current limitation (cannot jump accurately to a specific time). Please let me know what else I can do. Thanks.
For anyone else: I know this topic is a year old, and doesn't seem to generate much interest outside of Roger (and Joakim). However, I would like to get this patch accepted. It is working and addresses a current limitation (cannot jump accurately to a specific time). Please let me know what else I can do.
+1 from me. I might even start looking at using upstream libdvdnav instead of builtin in xbmc after this goes in.
Hello gnosygnu, thanks for your persistence. On Monday, 29 October 2012 at 01:22, gnosygnu wrote: [...]
For anyone else: I know this topic is a year old, and doesn't seem to generate much interest outside of Roger (and Joakim). However, I would like to get this patch accepted. It is working and addresses a current limitation (cannot jump accurately to a specific time). Please let me know what else I can do.
Please post the final version of the patch which addresses all comments so far and I'll apply it. Regards, Dominik -- Fedora http://fedoraproject.org/wiki/User:Rathann RPMFusion http://rpmfusion.org | MPlayer http://mplayerhq.hu "Faith manages." -- Delenn to Lennier in Babylon 5:"Confessions and Lamentations"
On Sun, Dec 9, 2012 at 4:52 PM, Dominik 'Rathann' Mierzejewski <dominik@greysector.net> wrote:
[...] Please post the final version of the patch which addresses all comments so far and I'll apply it.
Thanks for the offer! Sorry for being so late with my response. I'm enclosing patch.7. It is a rollup of all requested changes so far. The last change was requested by Roger. He pointed out that the absence of a constant, and recommended a guard against invalid mode. Here is a link to a related post: http://lists.mplayerhq.hu/pipermail/dvdnav-discuss/2012-October/001789.html As far as I know, all other comments are fulfilled. Anyone else: please feel free to comment/suggest, and I'll do my best to address. Thanks again!
I'm enclosing patch.7. It is a rollup of all requested changes so far.
Sorry all. I forgot that I should be sending the uncompressed patch. Here it is again. Thanks.
participants (4)
-
Dominik 'Rathann' Mierzejewski -
gnosygnu -
Joakim Plate -
Roger Pack