[PATCH] "dvdnav_jump_to_sector" as an alternative to "dvdnav_time_search" (REV 5: combined args to structs)
This revision includes the following changes: 1) combined multiple arguments into structures (See dvdnav_pos_data_t, dvdnav_cell_data_t, dvdnav_jump_args_t) 2) changed multi-line statements to remove leading commas 3) removed tabs/trailing whitespace 4) replaced "discontinuity tmap indexes" approach with simpler "use entry sector of current cell" to handle DVDs with initial dummy cells. Thanks to all who have read through the patch and/or offered comments. As far as I know, there are no other known pending changes/requests. I am hoping that this patch is at a near-final state. Please let me know what else should be done to get this patch accepted. Thanks.
4) replaced "discontinuity tmap indexes" approach with simpler "use entry sector of current cell" to handle DVDs with initial dummy cells.
I was having trouble with "barely off" seeking for the first cell for one DVD, but it now works thanks! (With other DVD's works well, too). -roger-
I noticed this today with a DVD [IFO's available on request]: When attempting to jump to 642729000 cell not found; find=7141433 And it ended up jumping to near the beginning of the film. I guess that brings up the question...what *should* a seek operation do if instructed to seek past the end? My guess is it should seek to the last block, any thoughts? (FWIW, the current seek operations also "loop back around"). -r
On Fri, Jan 20, 2012 at 8:56 AM, Roger Pack <rogerdpack2@gmail.com> wrote:
I noticed this today with a DVD [IFO's available on request]: When attempting to jump to 642729000 cell not found; find=7141433
Sorry to hear about that. The 7141433 represents the requested time in milliseconds (642729000 / 90). This corresponds to 1:59:01 seconds. Does such a time exist in the title? - If it does, then the IFO's might be corrupt. The proc can't find a cell with a start time / end time that would enclose the 1:59:01. - If it doesn't, then it's a "normal" error. The proc should exit immediately for invalid times.
And it ended up jumping to near the beginning of the film.
That is odd. If it can't find a cell, it should exit immediately. I looked at the proc now, and I can't see it doing anything else. If you jump to 1:59:01 again, does it also take you near the beginning? If it does, does it take you to 00:00:00 exactly or some other time?
I guess that brings up the question...what *should* a seek operation do if instructed to seek past the end? My guess is it should seek to the last block, any thoughts?
I think that it should do nothing, rather than seek to the last block. Of course, it's in my best interests to say that. :) However, I downloaded vanilla VLC 1.1.11 and it also does nothing when I try to jump to an invalid time (03:00:00).
(FWIW, the current seek operations also "loop back around").
Sorry. Not sure what this means.
-r _______________________________________________ DVDnav-discuss mailing list DVDnav-discuss@mplayerhq.hu https://lists.mplayerhq.hu/mailman/listinfo/dvdnav-discuss
I noticed this today with a DVD [IFO's available on request]: When attempting to jump to 642729000 cell not found; find=7141433
Sorry to hear about that.
The 7141433 represents the requested time in milliseconds (642729000 / 90). This corresponds to 1:59:01 seconds.
Does such a time exist in the title?
No.
- If it does, then the IFO's might be corrupt. The proc can't find a cell with a start time / end time that would enclose the 1:59:01. - If it doesn't, then it's a "normal" error. The proc should exit immediately for invalid times.
And it ended up jumping to near the beginning of the film.
That is odd. If it can't find a cell, it should exit immediately. I looked at the proc now, and I can't see it doing anything else.
If you jump to 1:59:01 again, does it also take you near the beginning? If it does, does it take you to 00:00:00 exactly or some other time?
If I seek directly there it just starts at the beginning. So I think this may be a "relative seek" mplayer bug, not the fault of dvdnav. Thanks! -roger-
The 7141433 represents the requested time in milliseconds (642729000 / 90). This corresponds to 1:59:01 seconds.
Does such a time exist in the title?
No.
Okay. Then I'd say this is normal. I fprintf all "failed" attempts to find a cell for a time. The fprintfs may be noisy, but I do so in case of corrupted IFOs.
If you jump to 1:59:01 again, does it also take you near the beginning? If it does, does it take you to 00:00:00 exactly or some other time?
If I seek directly there it just starts at the beginning. So I think this may be a "relative seek" mplayer bug, not the fault of dvdnav.
I agree. I used VLC just now and tried to seek to a 3:00:00 point in a 2:07:18 film. I got a "cell not found; find=10800000" but didn't go anywhere. Hope this helps
On Tue, Nov 29, 2011 at 10:31 PM, gnosygnu <gnosygnu@gmail.com> wrote:
This revision includes the following changes:
1) combined multiple arguments into structures (See dvdnav_pos_data_t, dvdnav_cell_data_t, dvdnav_jump_args_t) 2) changed multi-line statements to remove leading commas 3) removed tabs/trailing whitespace 4) replaced "discontinuity tmap indexes" approach with simpler "use entry sector of current cell" to handle DVDs with initial dummy cells.
Thanks to all who have read through the patch and/or offered comments.
As far as I know, there are no other known pending changes/requests. I am hoping that this patch is at a near-final state.
Let me know if there's anything else I could do to help get this patch accepted. It works for me as advertised. The only difficulty I've found is that it jumps to a time that isn't precisely relative to the time given by dvdnav_get_current_time. This can cause the following to occur: if you want to use mplayer with an EDL that says "skip from second 10 to second 15" it will play to second 10, then instruct it to jump to 15. It may jump to second 14 instead. When the player detects that it is now at second 14, it instructs it to jump to 15, which actually causes it to jump to 14 (infinite loop). This is a problem that can probably be worked out later, though, and I'm ok with the patch as is--it seems to be the only way to send "human time" jump instructions to libdvdnav without using external code that some libraries have had to create locally to get around it. Thanks! -roger-
Let me know if there's anything else I could do to help get this patch accepted. It works for me as advertised.
Same here. I'd also like to get the patch accepted. I'm open to suggestions from anyone. I believe I've fulfilled all earlier requests (let me know if I missed any).
The only difficulty I've found is that it jumps to a time that isn't precisely relative to the time given by dvdnav_get_current_time. This can cause the following to occur: if you want to use mplayer with an EDL that says "skip from second 10 to second 15" it will play to second 10, then instruct it to jump to 15. It may jump to second 14 instead. When the player detects that it is now at second 14, it instructs it to jump to 15, which actually causes it to jump to 14 (infinite loop).
If you want to forward me the IFOs, I can take a look at it. Unfortunately, the code is not entirely precise, and it may be off by 1/2 a second. Some of this has to do with the VOBUs which are generally in increments of .4 seconds. It could be that 1/2 a second is causing the jump to slip from 15 to 14 seconds. If I look at the IFOs, I can see whether or not it is an issue with the IFO or the patch itself.
I would like to see a API change before this goes in. A parameter saying if you want closest time before or after the passed time. Reason for this is that if you are skipping back, you don't want the situation where you end up after the current location even if you ask for a skip back by one millisecond. Similar but opposite occurs in the forward case. It also applies if you try to do a poor mans reverse playback. By doing seeks and decoding one frame. You need to get to previous keyframe. A keyframe after current location is no good. /Joakim
I would like to see a API change before this goes in. A parameter saying if you want closest time before or after the passed time.
Yeah it would be nice to have more control and be able to say "jump to the nearest NAV before this time" or "after this time" -r
I would like to see a API change before this goes in. A parameter saying if you want closest time before or after the passed time. Reason for this is that if you are skipping back, you don't want the situation where you end up after the current location even if you ask for a skip back by one millisecond. Similar but opposite occurs in the forward case.
It also applies if you try to do a poor mans reverse playback. By doing seeks and decoding one frame. You need to get to previous keyframe. A keyframe after current location is no good.
Thanks for the reply. The request is reasonable, however my method doesn't know *exactly* what time gets chosen. I know this sounds strange, but from what I know (and what I know may very well be wrong), there is no way to get an exact time with the IFOs only. For example, let's say you want to jump to the 15 second mark. In simplified form, my patch does the following. - find out where 15 seconds is on the time map. If there is a time map with a 4 second interval, it is between time map entries 3 (12 seconds) and 4 (16 seconds) - find out the corresponding VOBUs for the time map entries Let's say that entry 3 is at VOBU 21, and entry 4 is at VOBU 29. - interpolate the VOBU based on the time The 15 second mark is .75 between 12 seconds and 16 seconds .75 of the distance between VOBU 21 and VOBU 29 is VOBU 27 - jump to the sector for VOBU 27 Unfortunately, VOBU 27 has no specifically defined time. Generally it is around 15.0 seconds, but since all VOBUs are not the same duration it could be 15.0 or 14.9 or 15.1. Without knowing whether or not VOBU 27 is 14.9 (before) or 15.1 (after), there's no way to adjust for the extra parameter. What we really need is a VOBU to time map. Unfortunately, the IFOs don't provide this data. They only provide a VOBU to sector map and a TimeInterval to sector map (with TimeInterval usually being defined in large 4 second blocks). Note that the TimeInterval map is not precise either, so that when it says that time 4 seconds is at sector 876, it doesn't mean that sector 876 is exactly at time 4.00, but somewhere near time 4.00. In short, I only see two ways of implementing the extra parameter. Neither are really good. 1) Open up the VOB to find out the exact timestamp for VOBU 27. Unfortunately this would require even more code on top of what I wrote. I looked at this last year, and I don't think there is code in dvdread to easily extract the timestamp for a VOBU from a VOB. Also, I would think that there is a performance hit in doing a disc-read of VOBU 27 to get its timestamp. An earlier version of my patch was incorrectly reading the IFO on each jump, and this extra "read" was noticeable to Roger. 2) Create another method called "dvdnav_jump_to_sector_by_time_precise". This would try to use the parameter by doing the following: - call "dvdnav_jump_to_sector_by_time" for the given time. For example, this will jump to VOBU 27 - call "dvdnav_get_position" to get the time for VOBU 27. For example, let's say it finds out the time is 14.90 - call "dvdnav_jump_to_sector_by_time" again b/c 14.90 is < 15.00 and the parameter specified "get me a > time". The problem with this approach is the double-jump. This could look potentially "jerky". Also, I'm not sure how reliable the get_time will be (will the VM still be at 14.90 after the jump or could it have advanced to 15.00). Let me know your thoughts on the above. Thanks.
1) Open up the VOB to find out the exact timestamp for VOBU 27. Unfortunately this would require even more code on top of what I wrote. I looked at this last year, and I don't think there is code in dvdread to easily extract the timestamp for a VOBU from a VOB. Also, I would think that there is a performance hit in doing a disc-read of VOBU 27 to get its timestamp. An earlier version of my patch was incorrectly reading the IFO on each jump, and this extra "read" was noticeable to Roger.
The first data at a vob unit is a NAV packet. This contains time information for requested vob unit, AS WELL as back time skip information both back and forward in time, contained in the vobu_sri_t structure. Doing a read of this single sector won't hurt much I think (you are going to read it the moment you request playback anyway). I think a tmap search to a vobunit close to the requested time, read NAV packet, use the info in vobu_sri_t to figure out how to get a more exact location jump there is what is the best approach. But i'd like to add that i think it's better we add the parameter to the API even if it's currently won't be implemented. That way the API won't need to be changed to add support.
On Thu, Apr 19, 2012 at 3:50 AM, Joakim Plate <elupus@ecce.se> wrote:
The first data at a vob unit is a NAV packet. This contains time information for requested vob unit, AS WELL as back time skip information both back and forward in time, contained in the vobu_sri_t structure.
Ok. Thanks. This helps.
Doing a read of this single sector won't hurt much I think (you are going to read it the moment you request playback anyway).
One other note. The time map is not precise, and my methodolgy of interpolating VOBUs may be less so. In theory, the time I get should be within 1 sector of the actual time. However, it could be 2+ sectors away. My concern is that I may end up having to read multiple sectors and this would be noticeable from a performance perspective.
I think a tmap search to a vobunit close to the requested time, read NAV packet, use the info in vobu_sri_t to figure out how to get a more exact location jump there is what is the best approach.
I agree. This is the best approach. I'll look at the vobu_sri_t later this week, and see what can be done.
But i'd like to add that i think it's better we add the parameter to the API even if it's currently won't be implemented. That way the API won't need to be changed to add support.
I have no objections. How about the following?: An int32_t parameter called "mode" which can be any of the following 0: normal. same as current behavior 1: always jump to a time > than requested time. For example, if time "15.00" is requested, and the nearest vobus are "14.90" and "15.40", pick "15.40" (even though "14.90" is closer) -1: always jump to a time < than requested time. Similar to above, but pick "14.90"
But i'd like to add that i think it's better we add the parameter to the API even if it's currently won't be implemented. That way the API won't need to be changed to add support.
I have no objections. How about the following?:
An int32_t parameter called "mode" which can be any of the following
0: normal. same as current behavior 1: always jump to a time > than requested time. For example, if time "15.00" is requested, and the nearest vobus are "14.90" and "15.40", pick "15.40" (even though "14.90" is closer) -1: always jump to a time < than requested time. Similar to above, but pick "14.90"
That API would work for me. -roger-
I have no objections. How about the following?:
An int32_t parameter called "mode" which can be any of the following
0: normal. same as current behavior 1: always jump to a time > than requested time. For example, if time "15.00" is requested, and the nearest vobus are "14.90" and "15.40", pick "15.40" (even though "14.90" is closer) -1: always jump to a time < than requested time. Similar to above, but pick "14.90"
Seems good to me.
I spent some time looking at this today, and I think the effort is going to be quite significant (at least by my abilities). The dsi information seems to be read by dvdnav_decode_packet which is deep in dvdnav_get_next_cache_block. I think I would need a separate dvdnav_t struct to "peek" at the jumped packet. If I use the existing dvdnav_get_next_cache_block (*this), I would be overwriting the existing state. Creating a 2nd dvdnav_t doesn't seem better. In either event, I think the code would be involved. On a high-level basis, I think the following would be done: 1) For the "jumped" VOBU, read its dsi info (as mentioned above, there doesn't seem to be a way to peek at it without using the existing dvdnav_t struct) 2) With the dsi, compare the requested time with the c_eltm. 3) Depending on the difference, get the VOBU from vobu_sri_t. For example, if c_eltm is .5 seconds behind the requested time, look at vobu_sri_t.fwda and sri_fwda1 to get the VOBU span for the VOBU .5 seconds ahead. (BTW: I'm taking my info from here: http://dvd.sourceforge.net/dvdinfo/dsi_pkt.html) 4) With the VOBU from (3) look at the ADMAP and get the sector (1) and (3) seem like extensive operations, especially since I have little familiarity with the underlying procs/data structures. Unless there's something simpler, I'm going to have to defer this for later. I will still send a REVISION 6 with the new interface, but the actual implementation will be a TODO for later. Let me know if I'm missing anything. Thanks.
On Mon, Apr 23, 2012 at 6:13 AM, gnosygnu <gnosygnu@gmail.com> wrote:
I spent some time looking at this today, and I think the effort is going to be quite significant (at least by my abilities).
The dsi information seems to be read by dvdnav_decode_packet which is deep in dvdnav_get_next_cache_block. I think I would need a separate dvdnav_t struct to "peek" at the jumped packet. If I use the existing dvdnav_get_next_cache_block (*this), I would be overwriting the existing state. Creating a 2nd dvdnav_t doesn't seem better.
I think it should just setup a time value which get's handled on next call to dvdnav_get_next_cache_block(). Imho it doesn't need to setup state anywhere closer than the closest tmap before requested time, then on next read after NAV packet have been parsed, do the more exact block jump. But as i said, can be looked at later as long as API doesn't need to be changed.
Whatever happened to revision 6? Somebody just opened a pull request to incorporate this in xbmc: https://github.com/xbmc/xbmc/pull/2957 On Mon, Apr 23, 2012 at 9:29 PM, Joakim Plate <elupus@ecce.se> wrote:
On Mon, Apr 23, 2012 at 6:13 AM, gnosygnu <gnosygnu@gmail.com> wrote:
I spent some time looking at this today, and I think the effort is going to be quite significant (at least by my abilities).
The dsi information seems to be read by dvdnav_decode_packet which is deep in dvdnav_get_next_cache_block. I think I would need a separate dvdnav_t struct to "peek" at the jumped packet. If I use the existing dvdnav_get_next_cache_block (*this), I would be overwriting the existing state. Creating a 2nd dvdnav_t doesn't seem better.
I think it should just setup a time value which get's handled on next call to dvdnav_get_next_cache_block(). Imho it doesn't need to setup state anywhere closer than the closest tmap before requested time, then on next read after NAV packet have been parsed, do the more exact block jump.
But as i said, can be looked at later as long as API doesn't need to be changed.
Thanks for the update! I'm happy to see that the patch might finally get some usage somewhere! Version 6 was here: http://lists.mplayerhq.hu/pipermail/dvdnav-discuss/2012-April/001711.html However, the latest version is version 7. I sent it here as a zip: http://lists.mplayerhq.hu/pipermail/dvdnav-discuss/2012-December/001837.html I sent it again as plain text here: http://lists.mplayerhq.hu/pipermail/dvdnav-discuss/2012-December/001836.html Let me know if there is anything else. Thanks! On Thu, Jul 11, 2013 at 11:10 AM, Joakim Plate <elupus@ecce.se> wrote:
Whatever happened to revision 6?
Somebody just opened a pull request to incorporate this in xbmc: https://github.com/xbmc/xbmc/pull/2957
Yes basically everyone should use either the gnosygnu patch, or sector based seeking like VLC does, or they will get *inconsistent* seeking behavior (sometimes even going backward). VLC ideally could use the gnosygnu patch as well, to get more accurate timestamps [1]. -roger [1] http://forum.videolan.org/viewtopic.php?f=2&t=81879&start=0 On 7/11/13, gnosygnu <gnosygnu@gmail.com> wrote:
Thanks for the update! I'm happy to see that the patch might finally get some usage somewhere!
Version 6 was here: http://lists.mplayerhq.hu/pipermail/dvdnav-discuss/2012-April/001711.html
However, the latest version is version 7. I sent it here as a zip: http://lists.mplayerhq.hu/pipermail/dvdnav-discuss/2012-December/001837.html I sent it again as plain text here: http://lists.mplayerhq.hu/pipermail/dvdnav-discuss/2012-December/001836.html
Let me know if there is anything else.
Thanks!
On Thu, Jul 11, 2013 at 11:10 AM, Joakim Plate <elupus@ecce.se> wrote:
Whatever happened to revision 6?
Somebody just opened a pull request to incorporate this in xbmc: https://github.com/xbmc/xbmc/pull/2957
_______________________________________________ DVDnav-discuss mailing list DVDnav-discuss@mplayerhq.hu https://lists.mplayerhq.hu/mailman/listinfo/dvdnav-discuss
participants (3)
-
gnosygnu -
Joakim Plate -
Roger Pack