[libbluray-devel] Issue With HDVM Suspend / Resume Handling

[hpi]

tourette s wrote: 
> On Mon, Mar 28, 2011 at 11:01 PM, tourettes <tourettesmp at gmail.com> wrote:
> > Hi,
> >
> > I think I have found an issue with the way how HDVM suspend and resume
> > are handled. 

Yes, suspend/resume functionality is still incomplete. It should not
unconditionally backup/restore registers. Also, bd_menu_call() should
(conditionally) suspend current playlist.

> > We will end up in lost events on the library user side at least in
> > the following use case:
> >
> > 1) BD is started in navigation mode
> > 2) Main movie is selected in the main menu
> > 3) User goes back into the main menu (with bd_menu_call())
> > 4) User starts again the main movie from main menu
> >
> > Here's what happens in the bluray.c & register.c
> >
> > * Menu is entered for the 2nd time (with bd_menu_call() from the main movie)
> >  - registers gets saved

Currently nothing is saved when bd_menu_call() is called -> current
playback location is lost. Maybe menu title itself has some code that
triggers saving registers ?

> >  - bd->title_idx will point to the menu related index
> >
> > * Start main movie 2nd time -> restore state -> register restoring sets
> > correct playlist ID
> >  - bd->title_idx will point to the menu related title
> >  - an event about playlist change is generated (at this point application
> > queries the
> >    bd->title_idx, which results wrong title index)

The purprose of restoring registers is to resume playlist playback where
it was interrupted, so restore events should be handled internally
instead of passing those to application. This means starting title /
playlist playback at suspended location. This would also fix the
situation where playlist register != currently playing playlist.

If you select "Resume Movie" (etc.) from menu, it should restore
registers and resume playlist playback at the location defined by
restored registers. This is not implemented, so what happens here is
undefined and depends on the title object.

If you just start movie playback, it should not trigger register
restore. Instead it can discard suspended object and backup registers
(or replace those with current ones).

> > * run_hdvm() triggers bd_select_playlist()
> >  - bd->title_idx will get correct title index (main movie)
> >  - an event about the playlist change won't get triggered since registers
> > are already up to date
> >
> > How to fix that?

You shouldn't be using title_idx with menus. If you have filtered title
list all playlists won't be in title list.

> >  - Restore bd->title_idx (and other content as well?) when registers are
> > restored
> >  - Remove  "if (p->psr[reg] == val)" check in bd_psr_setting_write() and
> > leave it up to libbrary
> >    user to filter out the possible duplicate events
> >  - Poll on client side for possible bd->title_idx changes
> >
> > None of the fixes seem good ones (at least the polling one is really bad).
> 
> 
> In addition to that there is quite similar issue with the
> BD_EVENT_PLAYITEM events.
> They are missing on event bigger scale. On menu / navigation based
> playback mode
> when a title or playlist is changing the BD_EVENT_PLAYITEM is missing since most
> of the playlist / title changes are changing the clip to the 1st clip
> in the playlist (which is
> quite logical) but the clip identifier / index is generated on the fly
> and stays as zero.

This depends on how event semantics are defined. Currently event means
"Clip ID changed". It assumes application caches current clip id if it
needs it later. Playlist change (or angle change) implies clip change
too, even if the clip ID does not change (application should query new
playlist info when playlist or angle changes).

But I see there's problem: application can't know if clip ID is going to
change after playlist change. Unless it does something like:

while(bd_get_event()) {
  switch(event.type) {
    case NONE:     break;
    case PLAYLIST: pl = event.param; pl_changed = 1; break;
    case CLIP:     cl = event.param; cl_changed = 1; break;
    case ANGLE:    angle = event.param; angle_changed = 1; break;
    ...
  }
}
if (pl_changed || angle_changed) {
  bd_get_playlist_info(pl);
  cl_changed = 1;
}
if (cl_changed) {
  // update something...
}

> Use case:
> 
> 1) playback is started - 1st playlist starts (contains only one clip)
> -> BD_EVENT_PLAYITEM
> is generated with clip index 0
> 
> 2) next playlist is started (could be the menu- 1st clip is started ->
> BD_EVENT_PLAYITEM
> is not generated since registers already contain clip zero
> 
> 3) user selects something from BD menu - new playlist is started - 1st
> clip is started
>  -> BD_EVENT_PLAYITEM is not generated since registers already contain clip zero
> 
> Effectively the player cannot see the actual m2ts file changes that
> happen underneat, this
> would be quite important for to be able to track the clip / timestamp
> boundaries. Currently
> it is just possible to remove the register event filtering for
> duplicate events but I don't think
> it is the best solution. Also the player side workarounds aren't that
> good to solve the issue
> (you could use other events to query if the clip has changed). It
> would be logical if every single
> m2ts clip change would trigger the BD_EVENT_PLAYITEM that is visible
> to the application.

Maybe BD_EVENT_TITLE, BD_EVENT_ANGLE, BD_EVENT_PLAYLIST,
BD_EVENT_PLAYITEM and BD_EVENT_CHAPTER should be merged to single
event ?

I've attached a patch to generate event every time playlist or playitem
changes (playback starts). 

I have also some stuff related to suspend/resume handling, if someone
volunteers to do some heavy testing :)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: events.diff
Type: text/x-patch
Size: 2224 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/libbluray-devel/attachments/20110419/e3552f92/attachment.bin>