[FFmpeg-devel] [PATCH v2 04/11] avformat/dvdvideodec: remove "auto" value for -pg option, default to 1

[Marth64]

The default "auto" mode is effectively useless; the reasonable
default use case is to use the first PG (segment) of the
selected PGC for both menus and standard titles. Just
default the value to 1, since the option is irrelevant
unless -pgc is also set.

Note that this should not break users using this advanced option.
The "auto" mode errored and asked for a PG number regardless
for non-menus, and for menus the mode simply defaulted to 1.

Signed-off-by: Marth64 <marth64 at proxyid.net>
---
 doc/demuxers.texi         |  3 +--
 libavformat/dvdvideodec.c | 31 +++++++++----------------------
 2 files changed, 10 insertions(+), 24 deletions(-)

diff --git a/doc/demuxers.texi b/doc/demuxers.texi
index 04293c4813..74b68778bd 100644
--- a/doc/demuxers.texi
+++ b/doc/demuxers.texi
@@ -397,8 +397,7 @@ Default is 0, automatically resolve from value of @option{title}.
 The entry PG to start playback, in conjunction with @option{pgc}.
 Alternative to setting @option{title}.
 Chapter markers are not supported at this time.
-Default is 0, automatically resolve from value of @option{title}, or
-start from the beginning (PG 1) of the menu.
+Default is 1, the first PG of the PGC.
 
 @item preindex @var{bool}
 Enable this to have accurate chapter (PTT) markers and duration measurement,
diff --git a/libavformat/dvdvideodec.c b/libavformat/dvdvideodec.c
index 18c356c132..bed7c38ff6 100644
--- a/libavformat/dvdvideodec.c
+++ b/libavformat/dvdvideodec.c
@@ -537,7 +537,7 @@ static int dvdvideo_play_open(AVFormatContext *s, DVDVideoPlaybackState *state)
         goto end_dvdnav_error;
     }
 
-    if (c->opt_pgc > 0 && c->opt_pg > 0) {
+    if (c->opt_pgc > 0) {
         if (dvdnav_program_play(state->dvdnav, c->opt_title, c->opt_pgc, c->opt_pg) != DVDNAV_STATUS_OK) {
             av_log(s, AV_LOG_ERROR, "Unable to start playback at title %d, PGC %d, PG %d\n",
                                     c->opt_title, c->opt_pgc, c->opt_pg);
@@ -1541,13 +1541,6 @@ static int dvdvideo_read_header(AVFormatContext *s)
             c->opt_menu_lu = 1;
         }
 
-        if (!c->opt_pg) {
-            av_log(s, AV_LOG_INFO, "Defaulting to menu PG #1. "
-                                   "This is not always desirable, validation suggested.\n");
-
-            c->opt_pg = 1;
-        }
-
         if ((ret = dvdvideo_ifo_open(s)) < 0                    ||
             (ret = dvdvideo_menu_open(s, &c->play_state)) < 0   ||
             (ret = dvdvideo_subdemux_open(s)) < 0               ||
@@ -1558,7 +1551,13 @@ static int dvdvideo_read_header(AVFormatContext *s)
         return 0;
     }
 
-    if (c->opt_chapter_end != 0 && c->opt_chapter_start > c->opt_chapter_end) {
+    if (c->opt_pgc && (c->opt_chapter_start > 1 || c->opt_chapter_end > 0 || c->opt_preindex)) {
+        av_log(s, AV_LOG_ERROR, "PGC extraction not compatible with chapter or preindex options\n");
+
+        return AVERROR(EINVAL);
+    }
+
+    if (!c->opt_pgc && (c->opt_chapter_end != 0 && c->opt_chapter_start > c->opt_chapter_end)) {
         av_log(s, AV_LOG_ERROR, "Chapter (PTT) range [%d, %d] is invalid\n",
                                 c->opt_chapter_start, c->opt_chapter_end);
 
@@ -1572,18 +1571,6 @@ static int dvdvideo_read_header(AVFormatContext *s)
         c->opt_title = 1;
     }
 
-    if (c->opt_pgc) {
-        if (c->opt_pg == 0) {
-            av_log(s, AV_LOG_ERROR, "Invalid coordinates. If -pgc is set, -pg must be set too.\n");
-
-            return AVERROR(EINVAL);
-        } else if (c->opt_chapter_start > 1 || c->opt_chapter_end > 0 || c->opt_preindex) {
-            av_log(s, AV_LOG_ERROR, "-pgc is not compatible with the -preindex or "
-                                    "-chapter_start/-chapter_end options\n");
-            return AVERROR(EINVAL);
-        }
-    }
-
     if ((ret = dvdvideo_ifo_open(s)) < 0)
         return ret;
 
@@ -1761,7 +1748,7 @@ static const AVOption dvdvideo_options[] = {
     {"menu",            "demux menu domain",                                        OFFSET(opt_menu),           AV_OPT_TYPE_BOOL,   { .i64=0 },     0,          1,         AV_OPT_FLAG_DECODING_PARAM },
     {"menu_lu",         "menu language unit (0=auto)",                              OFFSET(opt_menu_lu),        AV_OPT_TYPE_INT,    { .i64=0 },     0,          99,        AV_OPT_FLAG_DECODING_PARAM },
     {"menu_vts",        "menu VTS (0=VMG main menu)",                               OFFSET(opt_menu_vts),       AV_OPT_TYPE_INT,    { .i64=0 },     0,          99,        AV_OPT_FLAG_DECODING_PARAM },
-    {"pg",              "entry PG number (0=auto)",                                 OFFSET(opt_pg),             AV_OPT_TYPE_INT,    { .i64=0 },     0,          255,       AV_OPT_FLAG_DECODING_PARAM },
+    {"pg",              "entry PG number (when paired with PGC number)",            OFFSET(opt_pg),             AV_OPT_TYPE_INT,    { .i64=1 },     1,          255,       AV_OPT_FLAG_DECODING_PARAM },
     {"pgc",             "entry PGC number (0=auto)",                                OFFSET(opt_pgc),            AV_OPT_TYPE_INT,    { .i64=0 },     0,          999,       AV_OPT_FLAG_DECODING_PARAM },
     {"preindex",        "enable for accurate chapter markers, slow (2-pass read)",  OFFSET(opt_preindex),       AV_OPT_TYPE_BOOL,   { .i64=0 },     0,          1,         AV_OPT_FLAG_DECODING_PARAM },
     {"region",          "playback region number (0=free)",                          OFFSET(opt_region),         AV_OPT_TYPE_INT,    { .i64=0 },     0,          8,         AV_OPT_FLAG_DECODING_PARAM },
-- 
2.39.5 (Apple Git-154)