[libbluray-devel] first patch-set

[hpi]

>>HDMV and BD-J applications use the registers too. Initial values are
>>fixed and all players use the same initial values. Changing those may
>>cause problems in HDMV / BD-J applications, so filtering should be
>>based on some flag instead of changing initial values.
>
> I did that initially, but it got over complicated (since I also needed to
> restore flags, and backup them, ..)
>
>
>>Following applies only to navigation mode started with bd_play() -
>>that's also the only scenario where restore is currently used:
>
>>When title is 0xffff other values (playlist, clip, chapter, angle,
>>current time, ...) have no meaning and should not be used at all. But
>>application should still cache the values for later use: when title
>>playback starts, only changed values are signaled to application.
>
> Thats what the patch does: the values that are changed/restored are stored,
> but not send to the player when they are the initialize values from startup
> (so that only when we have real valid values are available, they are send).
> Note that the change I made doesnt change anything internally on flow. It
> just doesnt send back to the user.
>
> Question: does the spec state that playlist and playlist item should be
> initailized to 0?

I think so. I can verify it in couple of days.

>>Maybe we could drop all status events in _process_psr_event() when
>>title is 0xffff ? Well, then, some care should be taken to ensure
>>events are always sent before playback starts.
>
> Might be a possibility too. Do you mean check PSR4 for 0xffff and dont send
> if thats the case?

Yes, and send values of blocked PSRs later (before playback starts,
somewhere after playlist has been selected).

>>One possible solution is to block all status events while HDMV VM is
>>running and generate events when HDMV VM enters suspended state
>> (playlist playback starts and register values are valid).
>>bd_read_ext() won't return any data before that as playback is
>>suspended while HDMV VM is running,
>>Here it might be good idea to run the whole HDMV application once
>>without returning to application for every event. HDMV execution is
>>supposed to be instant i.e. take no time. Not returning from
>>bd_read_ext() should prevent application from doing anything stupid
>>like seeking or changing title :)
>
> Which was my problem with playlist value 0 ;)
>
>>> 2) I saw some issues where on playback on object gets restored (ege play
>>> title, then go back if I remember correctly). The restore object thus
>>> resends the init values of the registers again, and then whatever the
>>> statemachine is giving. Ill try to repro the restore scenario if you
> want.
>
>>No need, I can see it too and agree it may cause problems and is
>>misleading (BD_EVENT_PLAYLIST 0 when playlist 0 is not playing -->
>>application does something, maybe tries to fetch title info for
>>possibly unexisting playlist 0).
>
> Yep.
>
>>> Ill check with the BR I have that shows the problem. If you want, and let
> me
>>> know how I can check for the muxing I can probably give you a patch (I
> know
>>> and have experience on mpegts muxing)
>
>>Demuxing is already implemented, you just need to hook it to bd_read()
>>when there is active/selected IG stream in the main path. I don't have
>>such samples so I didn't add it yet. _find_ig_stream() should always
>>find current IG stream. If it is muxed to main path, sub_path_idx is
>>not set. In that case we need some flag to tell when embedded IG
>>stream should be demuxed, and call gc_decode_ts(,...,ig_pid,...) for
>>every block. Maybe just add IG stream pid field to BD_STREAM and call
>>gc_decode_ts() in _read_block if PID is set ?
>
> Need to make sure the pts values are set correctly in the controller also.
> Apps that are cached reading ahead of the stream will be out of sync
> otherwise. I'll look into it.

--> [1]

>>_find_ig_stream() needs to be called at least when opening playlist
>>and when when PSR for active IG stream changes. Currently it is not
>>called if there are no sub-paths. Current code also doesn't support
>>changing IG stream in middle of playlist, so that could be added too
>>:).
>
>>Maybe we need to reset graphics controller after some operations (seek ?).
>
> Is seek allowed when an IG stream is running?
>

That depends on title/playlist/clip properties (allowed user
operations are in uo_mask). Typically seeking is not allowed in
traditional menus (and does not make any sense with those), but it is
allowed while (movie) playlist is playing - even if there's active
pop-up menu.

(Checking for allowed user operations is not implemented)

>>Synchronous menus can be valid only during certain perioid of clip, so
>>some logic should be implemented in graphics controller too. Maybe we
>>need to run it periodically so that possibly visible menu is
>>invalidated when it is not anymore valid (based on current player SCR
>>and composition_timeout_pts). Also selection_timeout_pts handling is
>>not implemented yet...
>
> I think in general some callback/hook is needed that will run the VM machine
> for HDMV without doing an actuall read for movie data (as in; update the
> animation/page).

--> [2]

>> Further remarks ;)
>>
>>> 1) Theres a subtle bug if you use the FILTER values (ege
>>> TITLES_FILTER_DUP_TITLE/TITLES_FILTER_DUP_CLIP). If that is used then
> some
>>> valid mpls's will be filtered that are valid when the BR is played in
>>> navigation mode. The list that navigation mode uses should be the full
> list
>>> available on the disc, while the filter values should return a filter
> lists
>>> based on that. I am also not really sure why that filtering is there
> anyway?
>
>>Yes, bd_get_titles() and bd_select_title() are quite useless in
>>navigation mode (using bd_select_title() may even cause some problems
>>...). Maybe bd_play() should always populate complete title list and
>>even disable further calls to bd_get_titles().
>>Filtering is intended for direct playlist playback when menus are not
>>used at all. The idea is to filter menu backgrounds etc. from title
>>list.
>
> That, or always read the complete list, and return a duplicate list (that
> the user needs to free explicitly) when he/she does a filtered retrieve.

User can requests only number of titles and single title info at time,
so there should be no need for multiple lists. Using always full list
internally should be quite easy to implement.
Maybe nav_get_title_list() could populate the flags
(DUP_TITLE/DUP_CLIP) to title list items so that there's no need to
examine those multiple times.
Then we just need to make sure title_idx's are correctly mapped
(applications expect continous range 0...N-1). So we need to count
number of matching titles in bd_get_titles() and convert
title_list[title_idx]... references to simple for() loops to search
the title in bd_get_title_info() and bd_select_title(). title_idx is
not used internally, just in API.

Maybe it could be possible to drop title_idx usage from API and use
only playlist id's (bd_select_playlist()). But playlist range is not
continous so bd_get_titles() would need to return list of possible
playlists.
That would move the title_idx concept to application side, and
simplify API. I think using title_idx is confusing as it does not map
to title objects but playlists. Current API has three different ways
to start playback : title object index (bd_play_title()), title index
(bd_select_title()) and playlist ID (bd_select_playlist()).

>>I assume you're referring to animations in buttons (controlled by
>>start_object_id_ref, end_object_id_ref and repeat_flag) and in menu
>>pages (in/out effects) ?
>
>>HDMV VM can run only when playlist is not playing. It is suspended
>>until playback stops. Animations should be implemented in graphics
>>controller.
>
> Let me explain better, or try to ;). I have some BR's that have LPCM data in
> really small chunks embeded in the stream (960 bytes) every 20 milliseconds
> or so. A result of this is that you need to buffer a good chunk of audio
> (2-3 seconds) before you can start feeding the audio playback device.
> Playing these small lpcm chunks as they arrive leads the cracking sounds
> (the audio device runs out of audio to play to soon because of the small
> chunks). I thus need to buffer a couple of seconds of the total movie (this
> also helps greatly on slower machines that have no hardware or multicore
> decoding for video). This however means that when a playlist/playitem
> changes, I need to wait until that buffer runs clear. I thus can not call
> bd_read_ext() anymore, since that would continue to retrieve movie data for
> a playlist I am not at yet. However, I still need valid user feedback for
> menu's (the user moves the mouse over menu's, and they dynamically show
> different size, highlight,  etc). Basically, the VM needs to run for any new
> instructions set on user input IF its just an animation. The VM however only
> runs on bd_read_ext()....
> Right now I changed bd_read_ext(), passing the title_idx I am playing. When
> that title_idx matches the one that libbluray has, it runs the VM based on
> the program that was set. When there is no program, or it does not match, it
> returns.

[2]

Yes, button actions should still be executed. Currently calling
    bd_read_ext(bd, NULL, 0, &event);
returns events and runs HDMV VM. It does not return any data or resume
suspended HDMV object.

The problem is suspended object is resumed immediately at end of
playlist when bd_read() returns 0. It could work if bd_read_ext()
would return pending events before resuming HDMV object. That way
application could process END_OF_TITLE event before HDMV object is
resumed:

    int bytes = bd_read(bd, buf, len);

    if (bytes == 0) {

+        if (_get_event(bd, event)) {
+            return 0;
+        }

        // if no next clip (=end of title), resume HDMV VM
        if (!bd->st0.clip && bd->title_type == title_hdmv) {

Well, in this simplest form it will loop for ever returning
END_OF_TITLE event. Check for resume condition should be done before
bd_read() that generates END_OF_TITLE events.

Anyway we need to delay resuming title object as HDMV VM can't run IG
stream code while there is active / resumed title object. It loses
current title object + IP and sees incorrect PSR values. In fact it
should refuse to take the IG object in hdmv_vm_set_object() if there's
active title object...

We could also run HDMV VM directly from _run_gc(). Both HDMV_VM and
graphics controller need proper locking anyway ; it is quite common to
run stream input and user input in different threads. But we still
need to delay resuming the title object...

BTW, are the animations really implemented as HDMV command sequences ?

>>You can use bd_user_input(..., BD_VK_NONE) to trigger page rendering.
> Yes, but that only sets new vm instructions. Does do not run until a new
> read().
>
>>Maybe BD_VK_NONE should be mapped to GC_CTRL_NOP to always force page
>>rendering (currently it does nothing if menu is already visible). This
>>code path already supports setting PSR_TIME; it should be used for
>>animation timing. Adding button animations should be quite simple
>>using this method, I already have some code for that but it is far
>>from complete :)
> Except that the BR that I have doesnt seem to set/use the PSR time
> register/instruction. I saw that piece of code after I send my last email,
> and checked. It does set the tickcount variable in its bog button.

[1]

PSR_TIME should be updated by the application. It should contain
current playback time (based on SCR / currently showing video frame
PTS / audio sample PTS). We can't rely on timestamps seen in bd_read()
as that is not synced to output. That's why you need to pass current
time in bd_user_input().

> I'll look into the muxing part and see if I can figure it out. Btw; if you
> need any of the parts of the disks that show behavior, let me know.