gmane.comp.video.dvdnav.general

Re: Please add dvdnav_dup() and dvdnav_free_dup() functions from the handbrake project

Erik Hovland — 2012-05-24T21:46:04

Now that sounds like a good reason. I stuck the patch into my repo and
added some words to the comment above the conditional to mention that
opening a VOB if one isn't already open is a good idea.

Thanks again!

E

Re: Please add dvdnav_dup() and dvdnav_free_dup() functions from the handbrake project

John Stebbins — 2012-05-24T21:15:22

The problem isn't with what happens when this->file is null *inside* the 
conditional.  It's a problem with what happens if the conditional *is 
not* run and this->file is null.  The file does not get opened and 
DVDReadBlocks references a null pointer.

Re: Please add dvdnav_dup() and dvdnav_free_dup() functions from the handbrake project

Erik Hovland — 2012-05-24T20:25:38

Very good! I am glad I didn't screw anything up.


My only issue w/ the patch is that it isn't clear why we would need to
change the VOB if the this->file is null. Especially confusing is the snippet
of code inside the conditional block:
if(this->file) {
  DVDCloseFile(this->file);
  this->file = NULL;
}

It implies that this->file being null is not a big deal. I am going to
leave this
patch out for now.

E

Re: Please add dvdnav_dup() and dvdnav_free_dup() functions from the handbrake project

John Stebbins — 2012-05-24T19:16:16

I tested your git.  Works great. I also compared your git to my 
patchset.  You have merged them all with the exception of part of one 
patch.  Unfortunately, I now can not remember the circumstances under 
which this small patch was useful.  I have a vague recollection that it 
was a rare corner case that I stumbled over, but it's been so long I 
really don't know.  I'm attaching the patch if you have any interest in 
it. It was originally part of the patch to fix vm_reset problem.

_______________________________________________
DVDnav-discuss mailing list
DVDnav-discuss-4+PuYLO5yA7jufZW15ZwLg< at >public.gmane.org
https://lists.mplayerhq.hu/mailman/listinfo/dvdnav-discuss

Re: Please add dvdnav_dup() and dvdnav_free_dup() functions from the handbrake project

John Stebbins — 2012-05-22T08:04:55

Hi,

I still lurk on the list.  I'll pull your git and see if I can make it 
work cleanly in HandBrake.  I'll also reformat any patches that haven't 
made it into your tree yet for you.  This just hasn't been high on my 
priority list lately and I've been traveling *a lot*.

On 05/22/2012 01:52 AM, Erik Hovland wrote:

Re: Please add dvdnav_dup() and dvdnav_free_dup() functions from the handbrake project

Erik Hovland — 2012-05-21T23:52:36

After breaking this patch into 4 parts, I have add this patch to
my personal tree. As long as some people test this, I will have no
problem pushing this feature to svn before the next release.

You can find my personal tree here:
https://github.com/microe/libdvdnav

Thanks for bringing this patch to our attention again.

John - if you are out there, I have split this patch into
4 different pieces b/c the vm_stop/vm_close stuff is
separate from the dvdnav_dup change. If I stuffed anything
up, please let me know.

Thanks to John for originally writing this patch.

E

Please add dvdnav_dup() and dvdnav_free_dup() functions from the handbrake project

Fabian Greffrath — 2012-05-21T21:54:55

Dear libdvdnav devs,

please consider applying the following patch originating from the
handbrake project:
<https://github.com/HandBrake/HandBrake/blob/master/contrib/libdvdnav/A08-dvdnav-dup.patch>

It adds two new functions, dvdnav_dup() and dvdnav_free_dup(), that
duplicate a dvdnav_t object and remove a duplicate from memory,
respectively - well, at least roughly.

We (i.e. the pkg-multimedia-packages team in Debian) are currently trying
to get handbrake built cleanly against Debian's system libraries and this
patch is currently the only one that's missing to reach this goal. The
handbrake build system currently downloads the libdvdnav source code
(among others), patches it and builds a private library from it. Adding
the functions directly into the handbrake source code (instead of the
downloaded library) is impossible, because they need to know
sizeof(dvdnav_t), which is not part of libdvdnav's public API - which is
in turn reasonable.

So, please, having these two functions in the next libdvdnav release would
make it possible to build this huge OSS project that is handbrake using
only the distribution's libraries (apart from libfaac, of course, which is
prepared but still not in Debian, because it is considered non-free by our
FTP-Masters).

BTW, there are also some other interesting patches in handbrake's repos. ;)

Best regards,
Fabian

[PATCH] Filenames in Unicode decoding issue

Honza Horak — 2012-05-14T15:34:45

Hi all,

there's an issue in libdvdread, triggered by DVDs with Unicode filenames 
(example of such DVD is a movie Thor).

Original bug report is available here:
https://bugzilla.redhat.com/show_bug.cgi?id=813977

There is also an enhanced Unicodedecode function on Ubuntu forum, which 
I consider better, than the current implementation is (see the patch 
attached):
http://ubuntuforums.org/showthread.php?p=11254706

Unfortunately, I'm unable to reproduce this issue, since I don't have 
such DVD, so I cannot verify if the patch fixes the issue.

Cheers,

Honza
_______________________________________________
DVDnav-discuss mailing list
DVDnav-discuss-4+PuYLO5yA7jufZW15ZwLg< at >public.gmane.org
https://lists.mplayerhq.hu/mailman/listinfo/dvdnav-discuss

Re: [PATCHES] libdvdread and libdvdnav patches

Erik Hovland — 2012-04-30T19:11:07

I have incorporated the bigendian and incdir fix into my github repo. I will
push them to svn soon. Please double check that they both work
for you.

The --libdatadir stuff is interesting. But if it is just pkgconfig, then that
might be a better name for the argument (--pkgconfigdir or --pkg-config).
If you submit that patch again against the github repo w/ that change, I
am likely to accept it.


I have incorporated this patch into my github repo. I will push this
up to svn soon.


I have incorporated the endian detection into my github repo. Please
try it and double check I did it right.

Same comment as about about libdatadir.

Thanks again for some great patches and pointing out some bad bugs.

E

Re: [PATCHES] libdvdread and libdvdnav patches

Brad Smith — 2012-04-30T08:57:32

Thanks for the big endian patches. I ran into this breakage when updating
from libdvdread 0.9.7 to the MPlayer projects fork for the OpenBSD ports.
Could the relevant pieces in the 3 patches abovee for the big endian fixes
please be commited to the SVN repo for each respective project? It's pretty
bad this was broken by the MPlayer project.

[PATCH] "dvdnav_jump_to_sector" as an alternative to "dvdnav_time_search" (REV 6: add mode argument)

gnosygnu — 2012-04-26T03:48:41

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.
_______________________________________________
DVDnav-discuss mailing list
DVDnav-discuss-4+PuYLO5yA7jufZW15ZwLg< at >public.gmane.org
https://lists.mplayerhq.hu/mailman/listinfo/dvdnav-discuss

Re: [PATCH] "dvdnav_jump_to_sector" as an alternative to "dvdnav_time_search" (REV 5: combined args to structs)

Joakim Plate — 2012-04-23T19:29:10

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.

Re: [PATCH] "dvdnav_jump_to_sector" as an alternative to "dvdnav_time_search" (REV 5: combined args to structs)

gnosygnu — 2012-04-23T04:13:21

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.

Re: [PATCH] "dvdnav_jump_to_sector" as an alternative to "dvdnav_time_search" (REV 5: combined args to structs)

Joakim Plate — 2012-04-20T17:25:20

Seems good to me.

Re: [PATCH] "dvdnav_jump_to_sector" as an alternative to "dvdnav_time_search" (REV 5: combined args to structs)

Roger Pack — 2012-04-20T14:13:40

That API would work for me.
-roger-

Re: [PATCH] Fix mount point -> device name on OSX

Roger Pack — 2012-04-20T14:05:37

the src files still compile for me, at least, on OS X.   Oh wait you
meant to try it on real BSD never mind...
-roger-

Re: [PATCH] "dvdnav_jump_to_sector" as an alternative to "dvdnav_time_search" (REV 5: combined args to structs)

gnosygnu — 2012-04-20T05:03:05

Ok. Thanks. This helps.


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 agree. This is the best approach. I'll look at the vobu_sri_t later
this week, and see what can be done.


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"

Re: [PATCH] "dvdnav_jump_to_sector" as an alternative to "dvdnav_time_search" (REV 5: combined args to structs)

Roger Pack — 2012-04-20T01:15:22

Yeah it's National Treasure.  I might be able to get it for you from
amazon...if I'm lucky enough to find the exact DVD :)


It looks like in this (other) case:

"EDL rel seek secs 2.962971 12.037029 [12.000000,15.000000]
jumping to 1310667"

It's jumping "forward 2.962971 secs"

Actually now that you mention it, I guess mplayer's code for "relative
seek" is currently based off "relative to the most recent NAV packet
time" so it might not have been seeking forward with "more than NAV
packet precision"

I sent mplayer some patches to help them seek better, hopefully that
will get committed this eon.

After overcoming that, now I get this console output, when attempting
to skip from "10 to 15"

last NAV packet was 9.600000
...
using mpeg ts appears larger, 10.082091:

EDL rel seek secs +4.917909 from pts:10.082091

last NAV packet was 9.600000, mpeg at 10.082091 adding MPEG difference
since last nav of 0.258597
 # this basically means that, because of my funky math, I'm skipping
from "DVD time" 9.858597 4.917909s forward
jumping to 1329885
...
A:  11.1 V: 15.16 A-V: -4.098 ct: -0.100   3/  3 ??% ??% ??,?% 0 0
...   new NAV packet! 14.833333 ...

so that's actually really close ( 9.858597 -> 14.833333 for a 4.92s skip) .
If I try seeking to "15.2" then it seeks to the same 14.833 mark, as
described previous, but at least I've figured out some more mplayer
workaround code for that in the meantime :)

Thanks for your tips I think it's working now.
-roger-

Re: [PATCH] "dvdnav_jump_to_sector" as an alternative to "dvdnav_time_search" (REV 5: combined args to structs)

Joakim Plate — 2012-04-19T07:50:01

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.
_______________________________________________
DVDnav-discuss mailing list
DVDnav-discuss< at >mplayerhq.hu
https://lists.mplayerhq.hu/mailman/listinfo/dvdnav-discuss

Re: [PATCH] "dvdnav_jump_to_sector" as an alternative to "dvdnav_time_search" (REV 5: combined args to structs)

gnosygnu — 2012-04-19T04:37:40

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.

Re: [PATCH] "dvdnav_jump_to_sector" as an alternative to "dvdnav_time_search" (REV 5: combined args to structs)

Roger Pack — 2012-04-18T22:54:52

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