Skip to content
Permalink
Browse files

Disable DVD and BD menu support (to be removed)

DVD/BD menu support never worked right, and are a pain to maintain. In
particular, DVD menus never actually worked correctly, because
highlights were not rendered correctly. Fixing this requires major
effort, which I'm not interested to spend.

Most importantly, the requirement to switch streams without losing the
DVD/BD state caused major weirdness in the playback core. It was
implemented by somehow syncing the playback state to the DVD/BD
implementation (in stream_dvdnav.c etc.), and then reloading the demuxer
without destroying and recreating the stream. This caused a bunch of
special-cases which I'm looking forward to remove.

For now, don't just remove everything related to menu support and just
disable it. If someone volunteers, it can be restored (i.e. rewritten)
in a reasonable way. If nobody volunteers soon, it goes.
  • Loading branch information...
wm4
wm4 committed Jun 26, 2015
1 parent d9b1939 commit 3b3170aedb4c8a9bfa32238f3b580feea22071f8
Showing with 9 additions and 7 deletions.
  1. +2 −6 DOCS/man/mpv.rst
  2. +1 −1 DOCS/mplayer-changes.rst
  3. +2 −0 player/discnav.c
  4. +2 −0 stream/stream_bluray.c
  5. +2 −0 stream/stream_dvdnav.c
@@ -567,13 +567,9 @@ PROTOCOLS
you must mount the ISO file as filesystem, and point ``--bluray-device``
to the mounted directory directly.

``bdnav://[title][/device]``
Play a Blu-Ray disc, with navigation features enabled. This feature is
permanently experimental.

``dvd://[title|[starttitle]-endtitle][/device]`` ``--dvd-device=PATH``
Play a DVD. If you want dvdnav menus, use ``dvd://menu``. If no title
is given, the longest title is auto-selected.
Play a DVD. DVD menus are not supported. If no title is given, the longest
title is auto-selected.

``dvdnav://`` is an old alias for ``dvd://`` and does exactly the same
thing.
@@ -279,7 +279,7 @@ Command Line Switches
``-xineramascreen`` ``--screen`` (different values)
``-xy W`` ``--autofit=W``
``-zoom`` Inverse available as ``--video-unscaled``
``dvdnav://`` ``dvdnav://menu``
``dvdnav://`` Removed.
``dvd://1`` ``dvd://0`` (0-based offset)
=========================== ========================================

@@ -135,6 +135,7 @@ void mp_nav_init(struct MPContext *mpctx)
if (mpctx->encode_lavc_ctx)
return;

#if 0
struct mp_nav_cmd inp = {MP_NAV_CMD_ENABLE};
if (run_stream_control(mpctx, STREAM_CTRL_NAV_CMD, &inp) < 1)
return;
@@ -150,6 +151,7 @@ void mp_nav_init(struct MPContext *mpctx)

update_state(mpctx);
update_mouse_on_button(mpctx);
#endif
}

void mp_nav_reset(struct MPContext *mpctx)
@@ -744,6 +744,8 @@ static int bluray_stream_open(stream_t *s)

int title_guess = BLURAY_DEFAULT_TITLE;
if (b->use_nav) {
MP_FATAL(s, "BluRay menu support has been removed.\n");
return STREAM_ERROR;
const BLURAY_DISC_INFO *disc_info = bd_get_disc_info(b->bd);
b->num_titles = disc_info->num_hdmv_titles + disc_info->num_bdj_titles;
++b->num_titles; // for BLURAY_TITLE_TOP_MENU
@@ -781,6 +781,8 @@ static int open_s(stream_t *stream)
return STREAM_UNSUPPORTED;
}
} else {
MP_FATAL(stream, "DVD menu support has been removed.\n");
return STREAM_ERROR;
if (dvdnav_menu_call(priv->dvdnav, DVD_MENU_Root) != DVDNAV_STATUS_OK)
dvdnav_menu_call(priv->dvdnav, DVD_MENU_Title);
}

11 comments on commit 3b3170a

@rrooij

This comment has been minimized.

Copy link
Contributor

rrooij replied Jun 26, 2015

To be honest, I think not correctly implementing DVD/BD menus is a reason to not use libmpv for some players. It would be great if someone could volunteer for this.

I'll try to look into it, but I can't promise anything since I'm not familiar with the dvd playback code.

@z0z0z

This comment has been minimized.

Copy link

z0z0z replied Jun 27, 2015

DVD menus never actually worked correctly, because highlights were not rendered correctly.

DVD menus work perfectly fine for me. I have no trouble at all using them, and they work as expected. Please don't remove this.

@lachs0r

This comment has been minimized.

Copy link
Member

lachs0r replied Jun 27, 2015

@z0z0z

This comment has been minimized.

Copy link

z0z0z replied Jun 27, 2015

Well, they’re broken with all the DVDs I have. Almost unusable.

Well, they work with all the DVDs I have. Completely usable.

contribute working code

The code is working, it might not be aesthetically pleasing, and causes the need of some workarounds in unrelated parts of the codebase. This seems to be the problem from what I'm understanding.

To be honest, I think DVD/BD menus have no reason to exist in the first place.

There's a lot of shitty formats and things when it comes to digital media that have no reason to exist.

If you need it, use something else

Everything else sucks even more, even if you consider it to be terrible with mpv.

Note that complaining until someone gives in is not one of them.

If a request not to remove a feature is complaining then so be it. I'm not trying to be hostile and I don't think there's any reason for you to be. wm4 can do whatever he wants with mpv, and probably will regardless of what I say. This is just my input as a user of mpv, and I think my thoughts on the issue are the same as a lot of other users, or potential users. Why do I even bother trying.

@lachs0r

This comment has been minimized.

Copy link
Member

lachs0r replied Jun 27, 2015

@rrooij

This comment has been minimized.

Copy link
Contributor

rrooij replied Jun 27, 2015

That's a very good idea, lachs0r.

@wm4

This comment has been minimized.

Copy link
Contributor

wm4 replied Jun 27, 2015

@rrooij: contact me on IRC and we can discuss this.

@wm4

This comment has been minimized.

Copy link
Contributor

wm4 replied Jun 27, 2015

The code is working, it might not be aesthetically pleasing, and causes the need of some workarounds in unrelated parts of the codebase.

The code is very glitchy and doesn't even implement highlights correctly. It has a relatively deep impact on how the playback logic works. It is not something I can just ignore and leave rotting in some separate file. If it was, I'd just keep it.

@tobiasjakobi

This comment has been minimized.

Copy link
Contributor

tobiasjakobi replied Jun 29, 2015

I also would like to voice my interest in keeping the menu support alive (again it works fine for my purpose).

@wm4

This comment has been minimized.

Copy link
Contributor

wm4 replied Jun 29, 2015

Acknowledged, but I pretty much don't want to waste precious life time on DVD code, or working around DVD code. Life is too short for that. So unless someone rewrites it in a way that doesn't get in my way, it'll most likely go.

@kadogo

This comment has been minimized.

Copy link

kadogo replied Oct 11, 2015

@wm4 What do you think of lachs0r idea ?
In my case, I use the menu only to select the track I will.
If we can easily select the different tracks, I think it's a good alternative.

Please sign in to comment.
You can’t perform that action at this time.