tourette s wrote:
> On Mon, Mar 28, 2011 at 11:01 PM, tourettes <[email protected]> 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 :)
diff --git a/src/libbluray/bluray.c b/src/libbluray/bluray.c
index bf5612d..66760ae 100644
--- a/src/libbluray/bluray.c
+++ b/src/libbluray/bluray.c
@@ -259,9 +259,16 @@ static void _update_stream_psr_by_lang(BD_REGISTERS *regs,
static void _update_clip_psrs(BLURAY *bd, NAV_CLIP *clip)
{
+ uint32_t prev_clip_id = bd_psr_read(bd->regs, PSR_PLAYITEM);
+
bd_psr_write(bd->regs, PSR_PLAYITEM, clip->ref);
bd_psr_write(bd->regs, PSR_TIME, clip->in_time);
+ if (clip->ref == prev_clip_id) {
+ /* event was not triggered by PSR write */
+ _queue_event(bd, (BD_EVENT){BD_EVENT_PLAYITEM, prev_clip_id});
+ }
+
/* Update selected audio and subtitle stream PSRs when not using menus.
* Selection is based on language setting PSRs and clip STN.
*/
@@ -277,6 +284,27 @@ static void _update_clip_psrs(BLURAY *bd, NAV_CLIP *clip)
}
}
+static void _update_playlist_psrs(BLURAY *bd)
+{
+ uint32_t prev_playlist_id = bd_psr_read(bd->regs, PSR_PLAYLIST);
+ uint32_t prev_angle_id = bd_psr_read(bd->regs, PSR_ANGLE);
+ uint32_t prev_chapter_id = bd_psr_read(bd->regs, PSR_CHAPTER);
+
+ bd_psr_write(bd->regs, PSR_PLAYLIST, atoi(bd->title->name));
+ if (prev_playlist_id == bd_psr_read(bd->regs, PSR_PLAYLIST)) {
+ _queue_event(bd, (BD_EVENT){BD_EVENT_PLAYLIST, prev_playlist_id});
+ }
+
+ bd_psr_write(bd->regs, PSR_ANGLE_NUMBER, bd->title->angle + 1);
+ if (prev_angle_id == bd->title->angle + 1) {
+ _queue_event(bd, (BD_EVENT){BD_EVENT_ANGLE, prev_angle_id});
+ }
+
+ bd_psr_write(bd->regs, PSR_CHAPTER, 1);
+ if (prev_chapter_id == 1) {
+ _queue_event(bd, (BD_EVENT){BD_EVENT_CHAPTER, prev_chapter_id});
+ }
+}
static void _update_chapter_psr(BLURAY *bd)
{
@@ -1315,9 +1343,7 @@ static int _open_playlist(BLURAY *bd, const char *f_name)
bd->next_chapter_start = bd_chapter_pos(bd, 1);
- bd_psr_write(bd->regs, PSR_PLAYLIST, atoi(bd->title->name));
- bd_psr_write(bd->regs, PSR_ANGLE_NUMBER, bd->title->angle + 1);
- bd_psr_write(bd->regs, PSR_CHAPTER, 1);
+ _update_playlist_psrs(bd);
// Get the initial clip of the playlist
bd->st0.clip = nav_next_clip(bd->title, NULL);
_______________________________________________
libbluray-devel mailing list
[email protected]
http://mailman.videolan.org/listinfo/libbluray-devel
