* [FFmpeg-devel] [PATCH 1/7] avformat/dvdvideodec: Fix racy PTS calculation and frame drops
2024-07-28 7:34 [FFmpeg-devel] [PATCH 0/7] avformat/dvdvideodec: Bug fixes, seeking support, and menu chapters Marth64
@ 2024-07-28 7:34 ` Marth64
2024-07-28 15:34 ` Marth64
2024-07-28 7:34 ` [FFmpeg-devel] [PATCH 2/7] avformat/dvdvideodec: Implement seeking Marth64
` (5 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Marth64 @ 2024-07-28 7:34 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Marth64
DVDs naturally consist of segmented MPEG-PS blobs within a VOB
(i.e. VOBs are not linear). NAV packs set the segment boundaries.
When switching between segments, discontinuities occur and thus
the subdemuxer needs to be reset. The current approach to manage
this is by invoking ff_read_frame_flush() on the subdemuxer context,
via a callback function which is invoked during the menu or dvdnav
block functions. The same subdemuxer context is used throughout
the demux, with a stretched PTS wrap bits value (64) + disabled
overflow correction, and then flushed on each segment. Eventually,
a play_end context variable is set to declare EOF.
However, this approach is wrong and racy. The block read flushes the
demuxer before the frame read is complete, causing frames to drop
on discontinuity. The play_end signal likewise ends playback before
the frame read is complete, causing frames to drop at end of the title.
To compound the issue, the PTS wrap bits value of 64 is wrong;
the VOBU limit is actually 32 and the overflow correction should work.
Instead, EOF the MPEG-PS subdemuxer organically when each VOB segment
ends, and re-open it if needed with the offset after the full frame read
is complete. In doing so, correct the PTS wrap behavior to 32 bits,
remove the racy play_end/segment_started signals and the callback pattern.
The behavior is now more similar to the HLS/DASH demuxers.
This commit fixes five intertwined issues, yielding an accurate demux:
(1) Racy segment switching
(2) Racy EOF signaling
(3) Off-by-one leading to missed packets at start of menus
(4) Incorrect PTS wrap behavior
(5) Unnecessary frame discard workarounds removed
Signed-off-by: Marth64 <marth64@proxyid.net>
---
libavformat/dvdvideodec.c | 198 +++++++++++++++++++-------------------
1 file changed, 100 insertions(+), 98 deletions(-)
diff --git a/libavformat/dvdvideodec.c b/libavformat/dvdvideodec.c
index 7a859071c3..e745165e00 100644
--- a/libavformat/dvdvideodec.c
+++ b/libavformat/dvdvideodec.c
@@ -58,7 +58,7 @@
#define DVDVIDEO_MAX_PS_SEARCH_BLOCKS 128
#define DVDVIDEO_BLOCK_SIZE 2048
#define DVDVIDEO_TIME_BASE_Q (AVRational) { 1, 90000 }
-#define DVDVIDEO_PTS_WRAP_BITS 64 /* VOBUs use 32 (PES allows 33) */
+#define DVDVIDEO_PTS_WRAP_BITS 32 /* VOBUs use 32 (PES allows 33) */
#define DVDVIDEO_LIBDVDX_LOG_BUFFER_SIZE 1024
#define PCI_START_BYTE 45 /* complement dvdread's DSI_START_BYTE */
@@ -116,8 +116,9 @@ typedef struct DVDVideoPlaybackState {
int pgc_nb_pg_est; /* number of PGs as reported by IFOs */
int pgcn; /* ID of the PGC we are playing */
int pgn; /* ID of the PG we are in now */
+ int ptm_discont; /* signal that a PTM discontinuity occurred */
+ int64_t ptm_offset; /* PTM discontinuity offset (as NAV value) */
int ptt; /* ID of the chapter we are in now */
- int64_t ts_offset; /* PTS discontinuity offset (ex. VOB change) */
uint32_t vobu_duration; /* duration of the current VOBU */
uint32_t vobu_e_ptm; /* end PTM of the current VOBU */
int vtsn; /* ID of the active VTS (video title set) */
@@ -164,10 +165,11 @@ typedef struct DVDVideoDemuxContext {
/* playback control */
int64_t first_pts; /* the PTS of the first video keyframe */
- int play_end; /* signal EOF to the parent demuxer */
- DVDVideoPlaybackState play_state; /* the active playback state */
int play_started; /* signal that playback has started */
- int segment_started; /* signal that subdemuxer is on a segment */
+ DVDVideoPlaybackState play_state; /* the active playback state */
+ int64_t pts_offset; /* PTS discontinuity offset (ex. VOB change) */
+ int seek_warned; /* signal that we warned about seeking limits */
+ int subdemux_reset; /* signal that subdemuxer should be reset */
} DVDVideoDemuxContext;
static void dvdvideo_libdvdread_log(void *opaque, dvd_logger_level_t level,
@@ -344,7 +346,7 @@ static int dvdvideo_menu_open(AVFormatContext *s, DVDVideoPlaybackState *state)
}
/* make sure the PGC is valid */
- state->pgcn = c->opt_pgc - 1;
+ state->pgcn = c->opt_pgc;
state->pgc = pgci_ut->lu[c->opt_menu_lu - 1].pgcit->pgci_srp[c->opt_pgc - 1].pgc;
if (!state->pgc || !state->pgc->program_map || !state->pgc->cell_playback) {
av_log(s, AV_LOG_ERROR, "Invalid PGC structure for menu [LU %d, PGC %d]\n",
@@ -390,14 +392,16 @@ static int dvdvideo_menu_open(AVFormatContext *s, DVDVideoPlaybackState *state)
}
static int dvdvideo_menu_next_ps_block(AVFormatContext *s, DVDVideoPlaybackState *state,
- uint8_t *buf, int buf_size,
- void (*flush_cb)(AVFormatContext *s))
+ uint8_t *buf, int buf_size, int *p_is_nav_packet)
{
int64_t blocks_read = 0;
uint8_t read_buf[DVDVIDEO_BLOCK_SIZE] = {0};
pci_t pci = (pci_t) {0};
dsi_t dsi = (dsi_t) {0};
+ (*p_is_nav_packet) = 0;
+ state->ptm_discont = 0;
+
if (buf_size != DVDVIDEO_BLOCK_SIZE) {
av_log(s, AV_LOG_ERROR, "Invalid buffer size (expected=%d actual=%d)\n",
DVDVIDEO_BLOCK_SIZE, buf_size);
@@ -463,10 +467,8 @@ static int dvdvideo_menu_next_ps_block(AVFormatContext *s, DVDVideoPlaybackState
if (state->in_pgc) {
if (state->vobu_e_ptm != pci.pci_gi.vobu_s_ptm) {
- if (flush_cb)
- flush_cb(s);
-
- state->ts_offset += state->vobu_e_ptm - pci.pci_gi.vobu_s_ptm;
+ state->ptm_discont = 1;
+ state->ptm_offset += state->vobu_e_ptm - pci.pci_gi.vobu_s_ptm;
}
} else {
state->in_pgc = 1;
@@ -474,13 +476,17 @@ static int dvdvideo_menu_next_ps_block(AVFormatContext *s, DVDVideoPlaybackState
}
state->vobu_e_ptm = pci.pci_gi.vobu_e_ptm;
+ state->vobu_duration = pci.pci_gi.vobu_e_ptm - pci.pci_gi.vobu_s_ptm;
av_log(s, AV_LOG_DEBUG, "NAV packet: sector=%d "
- "vobu_s_ptm=%d vobu_e_ptm=%d ts_offset=%" PRId64 "\n",
+ "vobu_s_ptm=%d vobu_e_ptm=%d ptm_offset=%" PRId64 "\n",
dsi.dsi_gi.nv_pck_lbn,
- pci.pci_gi.vobu_s_ptm, pci.pci_gi.vobu_e_ptm, state->ts_offset);
+ pci.pci_gi.vobu_s_ptm, pci.pci_gi.vobu_e_ptm, state->ptm_offset);
- return FFERROR_REDO;
+
+ (*p_is_nav_packet) = 1;
+
+ return DVDVIDEO_BLOCK_SIZE;
}
/* we are in the middle of a VOBU, so pass on the PS packet */
@@ -610,9 +616,7 @@ end_dvdnav_error:
}
static int dvdvideo_play_next_ps_block(AVFormatContext *s, DVDVideoPlaybackState *state,
- uint8_t *buf, int buf_size,
- int *p_nav_event,
- void (*flush_cb)(AVFormatContext *s))
+ uint8_t *buf, int buf_size, int *p_is_nav_packet)
{
DVDVideoDemuxContext *c = s->priv_data;
@@ -626,6 +630,9 @@ static int dvdvideo_play_next_ps_block(AVFormatContext *s, DVDVideoPlaybackState
pci_t *e_pci;
dsi_t *e_dsi;
+ (*p_is_nav_packet) = 0;
+ state->ptm_discont = 0;
+
if (buf_size != DVDVIDEO_BLOCK_SIZE) {
av_log(s, AV_LOG_ERROR, "Invalid buffer size (expected=%d actual=%d)\n",
DVDVIDEO_BLOCK_SIZE, buf_size);
@@ -769,16 +776,14 @@ static int dvdvideo_play_next_ps_block(AVFormatContext *s, DVDVideoPlaybackState
state->in_ps = 1;
} else {
if (state->vobu_e_ptm != e_pci->pci_gi.vobu_s_ptm) {
- if (flush_cb)
- flush_cb(s);
-
- state->ts_offset += state->vobu_e_ptm - e_pci->pci_gi.vobu_s_ptm;
+ state->ptm_discont = 1;
+ state->ptm_offset += state->vobu_e_ptm - e_pci->pci_gi.vobu_s_ptm;
}
}
state->vobu_e_ptm = e_pci->pci_gi.vobu_e_ptm;
- (*p_nav_event) = nav_event;
+ (*p_is_nav_packet) = 1;
return nav_len;
case DVDNAV_BLOCK_OK:
@@ -809,8 +814,6 @@ static int dvdvideo_play_next_ps_block(AVFormatContext *s, DVDVideoPlaybackState
"this could be due to a missed NAV packet\n",
state->pgn, cur_pgn);
- (*p_nav_event) = nav_event;
-
return nav_len;
case DVDNAV_WAIT:
if (dvdnav_wait_skip(state->dvdnav) != DVDNAV_STATUS_OK) {
@@ -908,7 +911,7 @@ static int dvdvideo_chapters_setup_preindex(AVFormatContext *s)
DVDVideoPlaybackState state = {0};
uint8_t nav_buf[DVDVIDEO_BLOCK_SIZE];
- int nav_event;
+ int is_nav_packet;
if (c->opt_chapter_start == c->opt_chapter_end)
return ret;
@@ -924,11 +927,11 @@ static int dvdvideo_chapters_setup_preindex(AVFormatContext *s)
while (!(interrupt = ff_check_interrupt(&s->interrupt_callback))) {
ret = dvdvideo_play_next_ps_block(s, &state, nav_buf, DVDVIDEO_BLOCK_SIZE,
- &nav_event, NULL);
+ &is_nav_packet);
if (ret < 0 && ret != AVERROR_EOF)
goto end_close;
- if (nav_event != DVDNAV_NAV_PACKET && ret != AVERROR_EOF)
+ if (!is_nav_packet && ret != AVERROR_EOF)
continue;
if (state.ptt == last_ptt) {
@@ -1420,46 +1423,40 @@ static int dvdvideo_subp_stream_add_all(AVFormatContext *s)
return 0;
}
-static void dvdvideo_subdemux_flush(AVFormatContext *s)
-{
- DVDVideoDemuxContext *c = s->priv_data;
-
- if (!c->segment_started)
- return;
-
- av_log(s, AV_LOG_DEBUG, "flushing sub-demuxer\n");
- avio_flush(&c->mpeg_pb.pub);
- ff_read_frame_flush(c->mpeg_ctx);
- c->segment_started = 0;
-}
-
static int dvdvideo_subdemux_read_data(void *opaque, uint8_t *buf, int buf_size)
{
AVFormatContext *s = opaque;
DVDVideoDemuxContext *c = s->priv_data;
- int ret = 0;
- int nav_event;
-
- if (c->play_end)
- return AVERROR_EOF;
+ int ret;
+ int is_nav_packet;
if (c->opt_menu)
- ret = dvdvideo_menu_next_ps_block(s, &c->play_state, buf, buf_size,
- dvdvideo_subdemux_flush);
+ ret = dvdvideo_menu_next_ps_block(s, &c->play_state, buf, buf_size, &is_nav_packet);
else
- ret = dvdvideo_play_next_ps_block(opaque, &c->play_state, buf, buf_size,
- &nav_event, dvdvideo_subdemux_flush);
+ ret = dvdvideo_play_next_ps_block(s, &c->play_state, buf, buf_size, &is_nav_packet);
- if (ret == AVERROR_EOF) {
- c->mpeg_pb.pub.eof_reached = 1;
- c->play_end = 1;
+ if (ret < 0)
+ goto subdemux_eof;
- return AVERROR_EOF;
- }
+ if (is_nav_packet) {
+ if (c->play_state.ptm_discont) {
+ c->subdemux_reset = 1;
+
+ ret = AVERROR_EOF;
+ goto subdemux_eof;
+ }
- if (ret >= 0 && nav_event == DVDNAV_NAV_PACKET)
return FFERROR_REDO;
+ }
+
+ return ret;
+
+subdemux_eof:
+ c->mpeg_pb.pub.eof_reached = 1;
+ c->mpeg_pb.pub.error = ret;
+ c->mpeg_pb.pub.read_packet = NULL;
+ c->mpeg_pb.pub.buf_end = c->mpeg_pb.pub.buf_ptr = c->mpeg_pb.pub.buffer;
return ret;
}
@@ -1469,7 +1466,9 @@ static void dvdvideo_subdemux_close(AVFormatContext *s)
DVDVideoDemuxContext *c = s->priv_data;
av_freep(&c->mpeg_pb.pub.buffer);
+ memset(&c->mpeg_pb, 0x00, sizeof(c->mpeg_pb));
avformat_close_input(&c->mpeg_ctx);
+ c->mpeg_ctx = NULL;
}
static int dvdvideo_subdemux_open(AVFormatContext *s)
@@ -1501,12 +1500,23 @@ static int dvdvideo_subdemux_open(AVFormatContext *s)
c->mpeg_ctx->max_analyze_duration = 0;
c->mpeg_ctx->interrupt_callback = s->interrupt_callback;
c->mpeg_ctx->pb = &c->mpeg_pb.pub;
- c->mpeg_ctx->correct_ts_overflow = 0;
- c->mpeg_ctx->io_open = NULL;
return avformat_open_input(&c->mpeg_ctx, "", &ff_mpegps_demuxer.p, NULL);
}
+static int dvdvideo_subdemux_reset(AVFormatContext *s)
+{
+ int ret;
+
+ av_log(s, AV_LOG_DEBUG, "resetting sub-demuxer\n");
+
+ dvdvideo_subdemux_close(s);
+ if ((ret = dvdvideo_subdemux_open(s)) < 0)
+ return ret;
+
+ return 0;
+}
+
static int dvdvideo_read_header(AVFormatContext *s)
{
DVDVideoDemuxContext *c = s->priv_data;
@@ -1604,72 +1614,64 @@ static int dvdvideo_read_packet(AVFormatContext *s, AVPacket *pkt)
DVDVideoDemuxContext *c = s->priv_data;
int ret;
- enum AVMediaType st_type;
- int found_stream = 0;
-
- if (c->play_end)
- return AVERROR_EOF;
+ int st_matched = 0;
+ AVStream *st_subdemux;
ret = av_read_frame(c->mpeg_ctx, pkt);
+ if (ret < 0) {
+ if (c->subdemux_reset) {
+ c->subdemux_reset = 0;
+ c->pts_offset = c->play_state.ptm_offset;
- if (ret < 0)
- return ret;
+ if ((ret = dvdvideo_subdemux_reset(s)) < 0)
+ return ret;
+
+ return FFERROR_REDO;
+ }
- if (!c->segment_started)
- c->segment_started = 1;
+ return ret;
+ }
- st_type = c->mpeg_ctx->streams[pkt->stream_index]->codecpar->codec_type;
+ st_subdemux = c->mpeg_ctx->streams[pkt->stream_index];
/* map the subdemuxer stream to the parent demuxer's stream (by startcode) */
for (int i = 0; i < s->nb_streams; i++) {
- if (s->streams[i]->id == c->mpeg_ctx->streams[pkt->stream_index]->id) {
+ if (s->streams[i]->id == st_subdemux->id) {
pkt->stream_index = s->streams[i]->index;
- found_stream = 1;
+ st_matched = 1;
break;
}
}
- if (!found_stream) {
- av_log(s, AV_LOG_DEBUG, "discarding frame with stream that was not in IFO headers "
- "(stream id=%d)\n", c->mpeg_ctx->streams[pkt->stream_index]->id);
-
- return FFERROR_REDO;
- }
+ if (!st_matched)
+ goto discard;
if (pkt->pts != AV_NOPTS_VALUE && pkt->dts != AV_NOPTS_VALUE) {
if (!c->play_started) {
- /* try to start at the beginning of a GOP */
- if (st_type != AVMEDIA_TYPE_VIDEO || !(pkt->flags & AV_PKT_FLAG_KEY)) {
- av_log(s, AV_LOG_VERBOSE, "Discarding packet which is not a video keyframe or "
- "with unset PTS/DTS at start\n");
- return FFERROR_REDO;
- }
-
c->first_pts = pkt->pts;
c->play_started = 1;
}
- pkt->pts += c->play_state.ts_offset - c->first_pts;
- pkt->dts += c->play_state.ts_offset - c->first_pts;
-
- if (pkt->pts < 0) {
- av_log(s, AV_LOG_VERBOSE, "Discarding packet with negative PTS (st=%d pts=%" PRId64 "), "
- "this is OK at start of playback\n",
- pkt->stream_index, pkt->pts);
-
- return FFERROR_REDO;
- }
+ pkt->pts += c->pts_offset - c->first_pts;
+ pkt->dts += c->pts_offset - c->first_pts;
} else {
- av_log(s, AV_LOG_WARNING, "Unset PTS or DTS @ st=%d pts=%" PRId64 " dts=%" PRId64 "\n",
- pkt->stream_index, pkt->pts, pkt->dts);
+ av_log(s, AV_LOG_WARNING, "Received packet with unset PTS or DTS\n");
}
av_log(s, AV_LOG_TRACE, "st=%d pts=%" PRId64 " dts=%" PRId64 " "
- "ts_offset=%" PRId64 " first_pts=%" PRId64 "\n",
+ "pts_offset=%" PRId64 " first_pts=%" PRId64 "\n",
pkt->stream_index, pkt->pts, pkt->dts,
- c->play_state.ts_offset, c->first_pts);
+ c->pts_offset);
+
+ return 0;
+
+discard:
+ av_log(s, AV_LOG_VERBOSE, "Discarding packet @ st=%d pts=%" PRId64 " dts=%" PRId64 " "
+ "st_matched=%d\n",
+ st_matched ? pkt->stream_index : -1, pkt->pts, pkt->dts,
+ st_matched);
- return c->play_end ? AVERROR_EOF : 0;
+ return FFERROR_REDO;
}
static int dvdvideo_close(AVFormatContext *s)
--
2.34.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 11+ messages in thread* [FFmpeg-devel] [PATCH 2/7] avformat/dvdvideodec: Implement seeking
2024-07-28 7:34 [FFmpeg-devel] [PATCH 0/7] avformat/dvdvideodec: Bug fixes, seeking support, and menu chapters Marth64
2024-07-28 7:34 ` [FFmpeg-devel] [PATCH 1/7] avformat/dvdvideodec: Fix racy PTS calculation and frame drops Marth64
@ 2024-07-28 7:34 ` Marth64
2024-07-28 15:20 ` Sean McGovern
2024-07-28 7:34 ` [FFmpeg-devel] [PATCH 3/7] avformat/dvdvideodec: Combine libdvdread and libdvdnav log callbacks Marth64
` (4 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Marth64 @ 2024-07-28 7:34 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Marth64
Player applications can now enjoy seeking while playing back
a title. Accuracy is at the mercy of what libdvdnav offers,
which is currently dvdnav_jump_to_sector_by_time().
Signed-off-by: Marth64 <marth64@proxyid.net>
---
libavformat/dvdvideodec.c | 93 ++++++++++++++++++++++++++++++++++-----
1 file changed, 82 insertions(+), 11 deletions(-)
diff --git a/libavformat/dvdvideodec.c b/libavformat/dvdvideodec.c
index e745165e00..e8301b1173 100644
--- a/libavformat/dvdvideodec.c
+++ b/libavformat/dvdvideodec.c
@@ -110,6 +110,7 @@ typedef struct DVDVideoPlaybackState {
int in_pgc; /* if our navigator is in the PGC */
int in_ps; /* if our navigator is in the program stream */
int in_vts; /* if our navigator is in the VTS */
+ int is_seeking; /* relax navigation path while seeking */
int64_t nav_pts; /* PTS according to IFO, not frame-accurate */
uint64_t pgc_duration_est; /* estimated duration as reported by IFO */
uint64_t pgc_elapsed; /* the elapsed time of the PGC, cell-relative */
@@ -722,7 +723,8 @@ static int dvdvideo_play_next_ps_block(AVFormatContext *s, DVDVideoPlaybackState
state->in_pgc = 1;
}
- } else if (state->celln >= e_cell->cellN || state->pgn > cur_pgn) {
+ } else if (!state->is_seeking &&
+ (state->celln >= e_cell->cellN || state->pgn > cur_pgn)) {
return AVERROR_EOF;
}
@@ -735,7 +737,7 @@ static int dvdvideo_play_next_ps_block(AVFormatContext *s, DVDVideoPlaybackState
if (!state->in_pgc)
continue;
- if ((state->ptt > 0 && state->ptt > cur_ptt) ||
+ if ((!state->is_seeking && state->ptt > 0 && state->ptt > cur_ptt) ||
(c->opt_chapter_end > 0 && cur_ptt > c->opt_chapter_end)) {
return AVERROR_EOF;
}
@@ -807,13 +809,15 @@ static int dvdvideo_play_next_ps_block(AVFormatContext *s, DVDVideoPlaybackState
return AVERROR_INPUT_CHANGED;
}
- memcpy(buf, &nav_buf, nav_len);
-
if (state->pgn != cur_pgn)
av_log(s, AV_LOG_WARNING, "Unexpected PG change (expected=%d actual=%d); "
"this could be due to a missed NAV packet\n",
state->pgn, cur_pgn);
+ memcpy(buf, &nav_buf, nav_len);
+
+ state->is_seeking = 0;
+
return nav_len;
case DVDNAV_WAIT:
if (dvdnav_wait_skip(state->dvdnav) != DVDNAV_STATUS_OK) {
@@ -1659,17 +1663,17 @@ static int dvdvideo_read_packet(AVFormatContext *s, AVPacket *pkt)
}
av_log(s, AV_LOG_TRACE, "st=%d pts=%" PRId64 " dts=%" PRId64 " "
- "pts_offset=%" PRId64 " first_pts=%" PRId64 "\n",
+ "pts_offset=%" PRId64 " first_pts=%" PRId64 " is_seeking=%d\n",
pkt->stream_index, pkt->pts, pkt->dts,
- c->pts_offset);
+ c->pts_offset, c->first_pts, c->play_state.is_seeking);
return 0;
discard:
av_log(s, AV_LOG_VERBOSE, "Discarding packet @ st=%d pts=%" PRId64 " dts=%" PRId64 " "
- "st_matched=%d\n",
+ "st_matched=%d is_seeking=%d\n",
st_matched ? pkt->stream_index : -1, pkt->pts, pkt->dts,
- st_matched);
+ st_matched, c->play_state.is_seeking);
return FFERROR_REDO;
}
@@ -1690,6 +1694,72 @@ static int dvdvideo_close(AVFormatContext *s)
return 0;
}
+static int dvdvideo_read_seek(AVFormatContext *s, int stream_index, int64_t timestamp, int flags)
+{
+ DVDVideoDemuxContext *c = s->priv_data;
+ int ret;
+ int64_t new_nav_pts;
+ pci_t* new_nav_pci;
+ dsi_t* new_nav_dsi;
+
+ if (c->opt_menu || c->opt_chapter_start > 1) {
+ av_log(s, AV_LOG_ERROR, "Seeking is not compatible with menus or chapter extraction\n");
+
+ return AVERROR_PATCHWELCOME;
+ }
+
+ if ((flags & AVSEEK_FLAG_BYTE))
+ return AVERROR(ENOSYS);
+
+ if (timestamp < 0)
+ return AVERROR(EINVAL);
+
+ if (!c->seek_warned) {
+ av_log(s, AV_LOG_WARNING, "Seeking is inherently unreliable and will result "
+ "in imprecise timecodes from this point\n");
+ c->seek_warned = 1;
+ }
+
+ /* XXX(PATCHWELCOME): use dvdnav_jump_to_sector_by_time(c->play_state.dvdnav, timestamp, 0)
+ * when it is available in a released version of libdvdnav; it is more accurate */
+ if (dvdnav_time_search(c->play_state.dvdnav, timestamp) != DVDNAV_STATUS_OK) {
+ av_log(s, AV_LOG_ERROR, "libdvdnav: seeking to %" PRId64 " failed\n", timestamp);
+
+ return AVERROR_EXTERNAL;
+ }
+
+ new_nav_pts = dvdnav_get_current_time (c->play_state.dvdnav);
+ new_nav_pci = dvdnav_get_current_nav_pci(c->play_state.dvdnav);
+ new_nav_dsi = dvdnav_get_current_nav_dsi(c->play_state.dvdnav);
+
+ if (new_nav_pci == NULL || new_nav_dsi == NULL) {
+ av_log(s, AV_LOG_ERROR, "Invalid NAV packet after seeking\n");
+
+ return AVERROR_INVALIDDATA;
+ }
+
+ c->play_state.in_pgc = 1;
+ c->play_state.in_ps = 0;
+ c->play_state.is_seeking = 1;
+ c->play_state.nav_pts = timestamp;
+ c->play_state.ptm_offset = timestamp;
+ c->play_state.ptm_discont = 0;
+ c->play_state.vobu_e_ptm = new_nav_pci->pci_gi.vobu_s_ptm;
+
+ c->first_pts = 0;
+ c->play_started = 0;
+ c->pts_offset = timestamp;
+ c->subdemux_reset = 0;
+
+ if ((ret = dvdvideo_subdemux_reset(s)) < 0)
+ return ret;
+
+ av_log(s, AV_LOG_DEBUG, "seeking: requested_nav_pts=%" PRId64 " new_nav_pts=%" PRId64 "\n",
+ timestamp, new_nav_pts);
+
+ return 0;
+}
+
#define OFFSET(x) offsetof(DVDVideoDemuxContext, x)
static const AVOption dvdvideo_options[] = {
{"angle", "playback angle number", OFFSET(opt_angle), AV_OPT_TYPE_INT, { .i64=1 }, 1, 9, AV_OPT_FLAG_DECODING_PARAM },
@@ -1718,11 +1788,12 @@ const FFInputFormat ff_dvdvideo_demuxer = {
.p.name = "dvdvideo",
.p.long_name = NULL_IF_CONFIG_SMALL("DVD-Video"),
.p.priv_class = &dvdvideo_class,
- .p.flags = AVFMT_NOFILE | AVFMT_SHOW_IDS | AVFMT_TS_DISCONT |
- AVFMT_NO_BYTE_SEEK | AVFMT_NOGENSEARCH | AVFMT_NOBINSEARCH,
+ .p.flags = AVFMT_SHOW_IDS | AVFMT_TS_DISCONT | AVFMT_SEEK_TO_PTS |
+ AVFMT_NOFILE | AVFMT_NO_BYTE_SEEK | AVFMT_NOGENSEARCH | AVFMT_NOBINSEARCH,
.priv_data_size = sizeof(DVDVideoDemuxContext),
.flags_internal = FF_INFMT_FLAG_INIT_CLEANUP,
.read_close = dvdvideo_close,
.read_header = dvdvideo_read_header,
- .read_packet = dvdvideo_read_packet
+ .read_packet = dvdvideo_read_packet,
+ .read_seek = dvdvideo_read_seek
};
--
2.34.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [FFmpeg-devel] [PATCH 2/7] avformat/dvdvideodec: Implement seeking
2024-07-28 7:34 ` [FFmpeg-devel] [PATCH 2/7] avformat/dvdvideodec: Implement seeking Marth64
@ 2024-07-28 15:20 ` Sean McGovern
2024-07-28 15:30 ` Marth64
0 siblings, 1 reply; 11+ messages in thread
From: Sean McGovern @ 2024-07-28 15:20 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Marth64
Hi Marth64,
On Sun, Jul 28, 2024 at 3:35 AM Marth64 <marth64@proxyid.net> wrote:
>
> Player applications can now enjoy seeking while playing back
> a title. Accuracy is at the mercy of what libdvdnav offers,
> which is currently dvdnav_jump_to_sector_by_time().
>
> Signed-off-by: Marth64 <marth64@proxyid.net>
> ---
> libavformat/dvdvideodec.c | 93 ++++++++++++++++++++++++++++++++++-----
> 1 file changed, 82 insertions(+), 11 deletions(-)
>
> diff --git a/libavformat/dvdvideodec.c b/libavformat/dvdvideodec.c
> index e745165e00..e8301b1173 100644
> --- a/libavformat/dvdvideodec.c
> +++ b/libavformat/dvdvideodec.c
> @@ -110,6 +110,7 @@ typedef struct DVDVideoPlaybackState {
> int in_pgc; /* if our navigator is in the PGC */
> int in_ps; /* if our navigator is in the program stream */
> int in_vts; /* if our navigator is in the VTS */
> + int is_seeking; /* relax navigation path while seeking */
> int64_t nav_pts; /* PTS according to IFO, not frame-accurate */
> uint64_t pgc_duration_est; /* estimated duration as reported by IFO */
> uint64_t pgc_elapsed; /* the elapsed time of the PGC, cell-relative */
> @@ -722,7 +723,8 @@ static int dvdvideo_play_next_ps_block(AVFormatContext *s, DVDVideoPlaybackState
>
> state->in_pgc = 1;
> }
> - } else if (state->celln >= e_cell->cellN || state->pgn > cur_pgn) {
> + } else if (!state->is_seeking &&
> + (state->celln >= e_cell->cellN || state->pgn > cur_pgn)) {
> return AVERROR_EOF;
> }
>
> @@ -735,7 +737,7 @@ static int dvdvideo_play_next_ps_block(AVFormatContext *s, DVDVideoPlaybackState
> if (!state->in_pgc)
> continue;
>
> - if ((state->ptt > 0 && state->ptt > cur_ptt) ||
> + if ((!state->is_seeking && state->ptt > 0 && state->ptt > cur_ptt) ||
> (c->opt_chapter_end > 0 && cur_ptt > c->opt_chapter_end)) {
> return AVERROR_EOF;
> }
> @@ -807,13 +809,15 @@ static int dvdvideo_play_next_ps_block(AVFormatContext *s, DVDVideoPlaybackState
> return AVERROR_INPUT_CHANGED;
> }
>
> - memcpy(buf, &nav_buf, nav_len);
> -
> if (state->pgn != cur_pgn)
> av_log(s, AV_LOG_WARNING, "Unexpected PG change (expected=%d actual=%d); "
> "this could be due to a missed NAV packet\n",
> state->pgn, cur_pgn);
>
> + memcpy(buf, &nav_buf, nav_len);
> +
> + state->is_seeking = 0;
> +
> return nav_len;
> case DVDNAV_WAIT:
> if (dvdnav_wait_skip(state->dvdnav) != DVDNAV_STATUS_OK) {
> @@ -1659,17 +1663,17 @@ static int dvdvideo_read_packet(AVFormatContext *s, AVPacket *pkt)
> }
>
> av_log(s, AV_LOG_TRACE, "st=%d pts=%" PRId64 " dts=%" PRId64 " "
> - "pts_offset=%" PRId64 " first_pts=%" PRId64 "\n",
> + "pts_offset=%" PRId64 " first_pts=%" PRId64 " is_seeking=%d\n",
> pkt->stream_index, pkt->pts, pkt->dts,
> - c->pts_offset);
> + c->pts_offset, c->first_pts, c->play_state.is_seeking);
>
> return 0;
>
> discard:
> av_log(s, AV_LOG_VERBOSE, "Discarding packet @ st=%d pts=%" PRId64 " dts=%" PRId64 " "
> - "st_matched=%d\n",
> + "st_matched=%d is_seeking=%d\n",
> st_matched ? pkt->stream_index : -1, pkt->pts, pkt->dts,
> - st_matched);
> + st_matched, c->play_state.is_seeking);
>
> return FFERROR_REDO;
> }
> @@ -1690,6 +1694,72 @@ static int dvdvideo_close(AVFormatContext *s)
> return 0;
> }
>
> +static int dvdvideo_read_seek(AVFormatContext *s, int stream_index, int64_t timestamp, int flags)
> +{
> + DVDVideoDemuxContext *c = s->priv_data;
> + int ret;
> + int64_t new_nav_pts;
> + pci_t* new_nav_pci;
> + dsi_t* new_nav_dsi;
> +
> + if (c->opt_menu || c->opt_chapter_start > 1) {
> + av_log(s, AV_LOG_ERROR, "Seeking is not compatible with menus or chapter extraction\n");
> +
> + return AVERROR_PATCHWELCOME;
> + }
> +
> + if ((flags & AVSEEK_FLAG_BYTE))
> + return AVERROR(ENOSYS);
> +
> + if (timestamp < 0)
> + return AVERROR(EINVAL);
> +
> + if (!c->seek_warned) {
> + av_log(s, AV_LOG_WARNING, "Seeking is inherently unreliable and will result "
> + "in imprecise timecodes from this point\n");
> + c->seek_warned = 1;
> + }
> +
> + /* XXX(PATCHWELCOME): use dvdnav_jump_to_sector_by_time(c->play_state.dvdnav, timestamp, 0)
> + * when it is available in a released version of libdvdnav; it is more accurate */
> + if (dvdnav_time_search(c->play_state.dvdnav, timestamp) != DVDNAV_STATUS_OK) {
> + av_log(s, AV_LOG_ERROR, "libdvdnav: seeking to %" PRId64 " failed\n", timestamp);
> +
> + return AVERROR_EXTERNAL;
> + }
I think avpriv_report_missing_feature() might be more appropriate here.
Also, does this build properly if a user does not have libdvdnav?
> +
> + new_nav_pts = dvdnav_get_current_time (c->play_state.dvdnav);
> + new_nav_pci = dvdnav_get_current_nav_pci(c->play_state.dvdnav);
> + new_nav_dsi = dvdnav_get_current_nav_dsi(c->play_state.dvdnav);
> +
> + if (new_nav_pci == NULL || new_nav_dsi == NULL) {
> + av_log(s, AV_LOG_ERROR, "Invalid NAV packet after seeking\n");
> +
> + return AVERROR_INVALIDDATA;
> + }
> +
> + c->play_state.in_pgc = 1;
> + c->play_state.in_ps = 0;
> + c->play_state.is_seeking = 1;
> + c->play_state.nav_pts = timestamp;
> + c->play_state.ptm_offset = timestamp;
> + c->play_state.ptm_discont = 0;
> + c->play_state.vobu_e_ptm = new_nav_pci->pci_gi.vobu_s_ptm;
> +
> + c->first_pts = 0;
> + c->play_started = 0;
> + c->pts_offset = timestamp;
> + c->subdemux_reset = 0;
> +
> + if ((ret = dvdvideo_subdemux_reset(s)) < 0)
> + return ret;
> +
> + av_log(s, AV_LOG_DEBUG, "seeking: requested_nav_pts=%" PRId64 " new_nav_pts=%" PRId64 "\n",
> + timestamp, new_nav_pts);
> +
> + return 0;
> +}
> +
> #define OFFSET(x) offsetof(DVDVideoDemuxContext, x)
> static const AVOption dvdvideo_options[] = {
> {"angle", "playback angle number", OFFSET(opt_angle), AV_OPT_TYPE_INT, { .i64=1 }, 1, 9, AV_OPT_FLAG_DECODING_PARAM },
> @@ -1718,11 +1788,12 @@ const FFInputFormat ff_dvdvideo_demuxer = {
> .p.name = "dvdvideo",
> .p.long_name = NULL_IF_CONFIG_SMALL("DVD-Video"),
> .p.priv_class = &dvdvideo_class,
> - .p.flags = AVFMT_NOFILE | AVFMT_SHOW_IDS | AVFMT_TS_DISCONT |
> - AVFMT_NO_BYTE_SEEK | AVFMT_NOGENSEARCH | AVFMT_NOBINSEARCH,
> + .p.flags = AVFMT_SHOW_IDS | AVFMT_TS_DISCONT | AVFMT_SEEK_TO_PTS |
> + AVFMT_NOFILE | AVFMT_NO_BYTE_SEEK | AVFMT_NOGENSEARCH | AVFMT_NOBINSEARCH,
> .priv_data_size = sizeof(DVDVideoDemuxContext),
> .flags_internal = FF_INFMT_FLAG_INIT_CLEANUP,
> .read_close = dvdvideo_close,
> .read_header = dvdvideo_read_header,
> - .read_packet = dvdvideo_read_packet
> + .read_packet = dvdvideo_read_packet,
> + .read_seek = dvdvideo_read_seek
> };
> --
> 2.34.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
-- Sean McGovern
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [FFmpeg-devel] [PATCH 2/7] avformat/dvdvideodec: Implement seeking
2024-07-28 15:20 ` Sean McGovern
@ 2024-07-28 15:30 ` Marth64
0 siblings, 0 replies; 11+ messages in thread
From: Marth64 @ 2024-07-28 15:30 UTC (permalink / raw)
To: Sean McGovern; +Cc: Marth64, FFmpeg development discussions and patches
Hi Sean_McG, good day,
First of all, there is a typo in the commit message. It should say
"at the mercy of dvdnav_time_search()"
> I think avpriv_report_missing_feature() might be more appropriate here.
But there is no missing feature, the implementation is working great
with dvdnav_time_search().
However, dvdnav_jump_to_sector_by_time() is slightly more reliable
than dvdnav_time_search().
Currently, VLC player, my reference for DVD at this time, is using
dvdnav_jump_to_sector_by_time()
via a workaround that I am not willing to bring to FFmpeg:
https://code.videolan.org/videolan/vlc/-/blob/master/modules/access/dvdnav.c#L60
The point of this "PATCHWELCOME" is to say swap to
dvdnav_jump_to_sector_by_time(),
when or if libdvdnav is eventually released again.
> Also, does this build properly if a user does not have libdvdnav?
Since the demuxer was already introduced in march and relies on
dvdnav, dvdnav is a
dependency. The demuxer will not be included in the build at all if the build
is not configured with dvdnav and dvdread. This is the case and is documented
sinc March 2024, and has not changed.
Thank you!
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 11+ messages in thread
* [FFmpeg-devel] [PATCH 3/7] avformat/dvdvideodec: Combine libdvdread and libdvdnav log callbacks
2024-07-28 7:34 [FFmpeg-devel] [PATCH 0/7] avformat/dvdvideodec: Bug fixes, seeking support, and menu chapters Marth64
2024-07-28 7:34 ` [FFmpeg-devel] [PATCH 1/7] avformat/dvdvideodec: Fix racy PTS calculation and frame drops Marth64
2024-07-28 7:34 ` [FFmpeg-devel] [PATCH 2/7] avformat/dvdvideodec: Implement seeking Marth64
@ 2024-07-28 7:34 ` Marth64
2024-07-28 7:34 ` [FFmpeg-devel] [PATCH 4/7] avformat/dvdvideodec: Chapter markers and trimming for menus Marth64
` (3 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Marth64 @ 2024-07-28 7:34 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Marth64
The methods are effectively the same but reference different
public enums, so roll them up into a macro.
Signed-off-by: Marth64 <marth64@proxyid.net>
---
libavformat/dvdvideodec.c | 64 +++++++++++++++------------------------
1 file changed, 24 insertions(+), 40 deletions(-)
diff --git a/libavformat/dvdvideodec.c b/libavformat/dvdvideodec.c
index e8301b1173..f6373d8ade 100644
--- a/libavformat/dvdvideodec.c
+++ b/libavformat/dvdvideodec.c
@@ -173,40 +173,28 @@ typedef struct DVDVideoDemuxContext {
int subdemux_reset; /* signal that subdemuxer should be reset */
} DVDVideoDemuxContext;
-static void dvdvideo_libdvdread_log(void *opaque, dvd_logger_level_t level,
- const char *msg, va_list msg_va)
-{
- AVFormatContext *s = opaque;
- char msg_buf[DVDVIDEO_LIBDVDX_LOG_BUFFER_SIZE];
- int lavu_level = AV_LOG_DEBUG;
-
- vsnprintf(msg_buf, sizeof(msg_buf), msg, msg_va);
-
- if (level == DVD_LOGGER_LEVEL_ERROR)
- lavu_level = AV_LOG_ERROR;
- else if (level == DVD_LOGGER_LEVEL_WARN)
- lavu_level = AV_LOG_WARNING;
-
- av_log(s, lavu_level, "libdvdread: %s\n", msg_buf);
-}
-
-static void dvdvideo_libdvdnav_log(void *opaque, dvdnav_logger_level_t level,
- const char *msg, va_list msg_va)
-{
- AVFormatContext *s = opaque;
- char msg_buf[DVDVIDEO_LIBDVDX_LOG_BUFFER_SIZE];
- int lavu_level = AV_LOG_DEBUG;
-
- vsnprintf(msg_buf, sizeof(msg_buf), msg, msg_va);
-
- if (level == DVDNAV_LOGGER_LEVEL_ERROR)
- lavu_level = AV_LOG_ERROR;
- /* some discs have invalid language codes set for menus, which throws noisy warnings */
- else if (level == DVDNAV_LOGGER_LEVEL_WARN && !av_strstart(msg, "Language", NULL))
- lavu_level = AV_LOG_WARNING;
-
- av_log(s, lavu_level, "libdvdnav: %s\n", msg_buf);
-}
+#define LIBDVDX_LOG_CALLBACK(X, CB_TYPE, LEVEL_TYPE, LEVEL_PREFIX) \
+ static void dvdvideo_##X##_log(void *o, LEVEL_TYPE level, const char *msg, va_list msg_va) \
+ { \
+ AVFormatContext *s = o; \
+ char msg_buf[DVDVIDEO_LIBDVDX_LOG_BUFFER_SIZE]; \
+ int lavu_level = AV_LOG_DEBUG; \
+ \
+ vsnprintf(msg_buf, sizeof(msg_buf), msg, msg_va); \
+ \
+ if (level == LEVEL_PREFIX##_ERROR) \
+ lavu_level = AV_LOG_ERROR; \
+ else if (level == LEVEL_PREFIX##_WARN && \
+ !av_strstart(msg, "Language", NULL)) /* muffle menus with invalid language */ \
+ lavu_level = AV_LOG_WARNING; \
+ \
+ av_log(s, lavu_level, #X": %s\n", msg_buf); \
+ } \
+ \
+ static const CB_TYPE dvdvideo_##X##_log_cb = (CB_TYPE) { .pf_log = dvdvideo_##X##_log }; \
+
+LIBDVDX_LOG_CALLBACK(libdvdread, dvd_logger_cb, dvd_logger_level_t, DVD_LOGGER_LEVEL)
+LIBDVDX_LOG_CALLBACK(libdvdnav, dvdnav_logger_cb, dvdnav_logger_level_t, DVDNAV_LOGGER_LEVEL)
static void dvdvideo_ifo_close(AVFormatContext *s)
{
@@ -226,11 +214,9 @@ static int dvdvideo_ifo_open(AVFormatContext *s)
{
DVDVideoDemuxContext *c = s->priv_data;
- dvd_logger_cb dvdread_log_cb;
title_info_t title_info;
- dvdread_log_cb = (dvd_logger_cb) { .pf_log = dvdvideo_libdvdread_log };
- c->dvdread = DVDOpen2(s, &dvdread_log_cb, s->url);
+ c->dvdread = DVDOpen2(s, &dvdvideo_libdvdread_log_cb, s->url);
if (!c->dvdread) {
av_log(s, AV_LOG_ERROR, "Unable to open the DVD-Video structure\n");
@@ -516,15 +502,13 @@ static int dvdvideo_play_open(AVFormatContext *s, DVDVideoPlaybackState *state)
{
DVDVideoDemuxContext *c = s->priv_data;
- dvdnav_logger_cb dvdnav_log_cb;
dvdnav_status_t dvdnav_open_status;
int32_t disc_region_mask;
int32_t player_region_mask;
int cur_title, cur_pgcn, cur_pgn;
pgc_t *pgc;
- dvdnav_log_cb = (dvdnav_logger_cb) { .pf_log = dvdvideo_libdvdnav_log };
- dvdnav_open_status = dvdnav_open2(&state->dvdnav, s, &dvdnav_log_cb, s->url);
+ dvdnav_open_status = dvdnav_open2(&state->dvdnav, s, &dvdvideo_libdvdnav_log_cb, s->url);
if (!state->dvdnav ||
dvdnav_open_status != DVDNAV_STATUS_OK ||
--
2.34.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 11+ messages in thread* [FFmpeg-devel] [PATCH 4/7] avformat/dvdvideodec: Chapter markers and trimming for menus
2024-07-28 7:34 [FFmpeg-devel] [PATCH 0/7] avformat/dvdvideodec: Bug fixes, seeking support, and menu chapters Marth64
` (2 preceding siblings ...)
2024-07-28 7:34 ` [FFmpeg-devel] [PATCH 3/7] avformat/dvdvideodec: Combine libdvdread and libdvdnav log callbacks Marth64
@ 2024-07-28 7:34 ` Marth64
2024-07-28 7:34 ` [FFmpeg-devel] [PATCH 5/7] avformat/dvdvideodec: Remove unused headers Marth64
` (2 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Marth64 @ 2024-07-28 7:34 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Marth64
Menus can have chapter markers by way of cells, and empty menus
can be detected with existing functionality. Menu selection
also is in need of usability improvement.
Implement chapter markers for menus, let the trim function detect
empty menus, remove the unnecessary ceremony around the -pg
option by simply defaulting it to 1, and finally default the
menu_vts option to 1 since most DVD menus can be found there.
Signed-off-by: Marth64 <marth64@proxyid.net>
---
libavformat/dvdvideodec.c | 106 +++++++++++++++++++++-----------------
1 file changed, 58 insertions(+), 48 deletions(-)
diff --git a/libavformat/dvdvideodec.c b/libavformat/dvdvideodec.c
index f6373d8ade..a908acfa7f 100644
--- a/libavformat/dvdvideodec.c
+++ b/libavformat/dvdvideodec.c
@@ -367,6 +367,14 @@ static int dvdvideo_menu_open(AVFormatContext *s, DVDVideoPlaybackState *state)
if (c->opt_menu_vts > 0)
state->in_vts = 1;
+ if (c->opt_trim && !dvdvideo_is_pgc_promising(s, state->pgc)) {
+ av_log(s, AV_LOG_ERROR, "Menu LU %d, VTS %d, PGC %d looks empty; "
+ "if you want to try anyway, disable the trim option\n",
+ c->opt_menu_lu, c->opt_menu_vts, c->opt_pgc);
+
+ return AVERROR_INVALIDDATA;
+ }
+
if (!(state->vob_file = DVDOpenFile(c->dvdread, c->opt_menu_vts, DVD_READ_MENU_VOBS))) {
av_log(s, AV_LOG_ERROR, !c->opt_menu_vts ?
"Unable to open main menu VOB (VIDEO_TS.VOB)\n" :
@@ -527,7 +535,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);
@@ -893,8 +901,8 @@ static int dvdvideo_chapters_setup_preindex(AVFormatContext *s)
{
DVDVideoDemuxContext *c = s->priv_data;
- int ret = 0, interrupt = 0;
- int nb_chapters = 0, last_ptt = c->opt_chapter_start;
+ int ret, partn, last_partn;
+ int interrupt = 0, nb_chapters = 0;
uint64_t cur_chapter_offset = 0, cur_chapter_duration = 0;
DVDVideoPlaybackState state = {0};
@@ -902,27 +910,38 @@ static int dvdvideo_chapters_setup_preindex(AVFormatContext *s)
int is_nav_packet;
if (c->opt_chapter_start == c->opt_chapter_end)
- return ret;
+ return 0;
- if ((ret = dvdvideo_play_open(s, &state)) < 0)
- return ret;
+ if (c->opt_menu) {
+ if ((ret = dvdvideo_menu_open(s, &state)) < 0)
+ return ret;
+ last_partn = state.celln;
+ } else {
+ if ((ret = dvdvideo_play_open(s, &state)) < 0)
+ return ret;
+ last_partn = c->opt_chapter_start;
+ }
if (state.pgc->nr_of_programs == 1)
goto end_close;
- av_log(s, AV_LOG_INFO,
- "Indexing chapter markers, this will take a long time. Please wait...\n");
+ av_log(s, AV_LOG_INFO, "Indexing chapter markers, this may take a long time. Please wait...\n");
while (!(interrupt = ff_check_interrupt(&s->interrupt_callback))) {
- ret = dvdvideo_play_next_ps_block(s, &state, nav_buf, DVDVIDEO_BLOCK_SIZE,
- &is_nav_packet);
+ if (c->opt_menu)
+ ret = dvdvideo_menu_next_ps_block(s, &state, nav_buf, DVDVIDEO_BLOCK_SIZE, &is_nav_packet);
+ else
+ ret = dvdvideo_play_next_ps_block(s, &state, nav_buf, DVDVIDEO_BLOCK_SIZE, &is_nav_packet);
+
if (ret < 0 && ret != AVERROR_EOF)
goto end_close;
if (!is_nav_packet && ret != AVERROR_EOF)
continue;
- if (state.ptt == last_ptt) {
+ partn = c->opt_menu ? state.celln : state.ptt;
+
+ if (partn == last_partn) {
cur_chapter_duration += state.vobu_duration;
/* ensure we add the last chapter */
if (ret != AVERROR_EOF)
@@ -941,7 +960,7 @@ static int dvdvideo_chapters_setup_preindex(AVFormatContext *s)
cur_chapter_offset += cur_chapter_duration;
cur_chapter_duration = state.vobu_duration;
- last_ptt = state.ptt;
+ last_partn = partn;
if (ret == AVERROR_EOF)
break;
@@ -961,7 +980,10 @@ static int dvdvideo_chapters_setup_preindex(AVFormatContext *s)
ret = 0;
end_close:
- dvdvideo_play_close(s, &state);
+ if (c->opt_menu)
+ dvdvideo_menu_close(s, &state);
+ else
+ dvdvideo_play_close(s, &state);
return ret;
}
@@ -1512,47 +1534,47 @@ static int dvdvideo_read_header(AVFormatContext *s)
int ret;
if (c->opt_menu) {
- if (c->opt_region ||
- c->opt_title > 1 ||
- c->opt_preindex ||
- c->opt_chapter_start > 1 ||
+ if (c->opt_region ||
+ c->opt_title > 1 ||
+ c->opt_chapter_start > 1 ||
c->opt_chapter_end > 0) {
- av_log(s, AV_LOG_ERROR, "-menu is not compatible with the -region, -title, "
- "-preindex, or -chapter_start/-chapter_end options\n");
+
+ av_log(s, AV_LOG_ERROR, "Menu mode does not use region, title, or chapter ranges\n");
+
return AVERROR(EINVAL);
}
if (!c->opt_pgc) {
- av_log(s, AV_LOG_ERROR, "If -menu is enabled, -pgc must be set to a non-zero value\n");
+ av_log(s, AV_LOG_ERROR, "Menu mode requires a PGC number\n");
return AVERROR(EINVAL);
}
if (!c->opt_menu_lu) {
av_log(s, AV_LOG_INFO, "Defaulting to menu language unit #1. "
- "This is not always desirable, validation suggested.\n");
+ "This is not always the desired menu, validation suggested\n");
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 ||
- (ret = dvdvideo_video_stream_setup(s)) < 0 ||
+ if ((ret = dvdvideo_ifo_open(s)) < 0 ||
+ (c->opt_preindex && (ret = dvdvideo_chapters_setup_preindex(s)) < 0) ||
+ (ret = dvdvideo_menu_open(s, &c->play_state)) < 0 ||
+ (ret = dvdvideo_subdemux_open(s)) < 0 ||
+ (ret = dvdvideo_video_stream_setup(s)) < 0 ||
(ret = dvdvideo_audio_stream_add_all(s)) < 0)
return ret;
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 is not compatible with chapter 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);
@@ -1561,23 +1583,11 @@ static int dvdvideo_read_header(AVFormatContext *s)
if (c->opt_title == 0) {
av_log(s, AV_LOG_INFO, "Defaulting to title #1. "
- "This is not always the main feature, validation suggested.\n");
+ "This is not always the main feature, validation suggested\n");
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;
@@ -1751,8 +1761,8 @@ static const AVOption dvdvideo_options[] = {
{"chapter_start", "entry chapter (PTT) number", OFFSET(opt_chapter_start), AV_OPT_TYPE_INT, { .i64=1 }, 1, 99, AV_OPT_FLAG_DECODING_PARAM },
{"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 },
+ {"menu_vts", "menu VTS (0=VMG root menu)", OFFSET(opt_menu_vts), AV_OPT_TYPE_INT, { .i64=1 }, 0, 99, 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.34.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 11+ messages in thread* [FFmpeg-devel] [PATCH 5/7] avformat/dvdvideodec: Remove unused headers
2024-07-28 7:34 [FFmpeg-devel] [PATCH 0/7] avformat/dvdvideodec: Bug fixes, seeking support, and menu chapters Marth64
` (3 preceding siblings ...)
2024-07-28 7:34 ` [FFmpeg-devel] [PATCH 4/7] avformat/dvdvideodec: Chapter markers and trimming for menus Marth64
@ 2024-07-28 7:34 ` Marth64
2024-07-28 7:34 ` [FFmpeg-devel] [PATCH 6/7] avformat/dvdvideodec: Simplify/clarify logs, comments, and class name Marth64
2024-07-28 7:34 ` [FFmpeg-devel] [PATCH 7/7] doc/demuxers: update dvdvideodec documentation Marth64
6 siblings, 0 replies; 11+ messages in thread
From: Marth64 @ 2024-07-28 7:34 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Marth64
Signed-off-by: Marth64 <marth64@proxyid.net>
---
libavformat/dvdvideodec.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/libavformat/dvdvideodec.c b/libavformat/dvdvideodec.c
index a908acfa7f..afc7836038 100644
--- a/libavformat/dvdvideodec.c
+++ b/libavformat/dvdvideodec.c
@@ -44,8 +44,6 @@
#include "libavutil/mem.h"
#include "libavutil/opt.h"
#include "libavutil/samplefmt.h"
-#include "libavutil/time.h"
-#include "libavutil/timestamp.h"
#include "avformat.h"
#include "avio_internal.h"
--
2.34.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 11+ messages in thread* [FFmpeg-devel] [PATCH 6/7] avformat/dvdvideodec: Simplify/clarify logs, comments, and class name
2024-07-28 7:34 [FFmpeg-devel] [PATCH 0/7] avformat/dvdvideodec: Bug fixes, seeking support, and menu chapters Marth64
` (4 preceding siblings ...)
2024-07-28 7:34 ` [FFmpeg-devel] [PATCH 5/7] avformat/dvdvideodec: Remove unused headers Marth64
@ 2024-07-28 7:34 ` Marth64
2024-07-28 7:34 ` [FFmpeg-devel] [PATCH 7/7] doc/demuxers: update dvdvideodec documentation Marth64
6 siblings, 0 replies; 11+ messages in thread
From: Marth64 @ 2024-07-28 7:34 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Marth64
Signed-off-by: Marth64 <marth64@proxyid.net>
---
libavformat/dvdvideodec.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/libavformat/dvdvideodec.c b/libavformat/dvdvideodec.c
index afc7836038..04c5cbea8e 100644
--- a/libavformat/dvdvideodec.c
+++ b/libavformat/dvdvideodec.c
@@ -21,7 +21,7 @@
/*
* See doc/demuxers.texi for a high-level overview.
*
- * The tactical approach is as follows:
+ * The tactical approach for title playback is as follows:
* 1) Open the volume with dvdread
* 2) Analyze the user-requested title and PGC coordinates in the IFO structures
* 3) Request playback at the coordinates and chosen angle with dvdnav
@@ -565,8 +565,8 @@ static int dvdvideo_play_open(AVFormatContext *s, DVDVideoPlaybackState *state)
}
if (c->opt_trim && !dvdvideo_is_pgc_promising(s, pgc)) {
- av_log(s, AV_LOG_ERROR, "Title %d, PGC %d looks empty (may consist of padding cells), "
- "if you want to try anyway, disable the -trim option\n",
+ av_log(s, AV_LOG_ERROR, "Title %d, PGC %d looks empty (may consist of padding cells); "
+ "if you want to try anyway, disable the trim option\n",
c->opt_title, state->pgcn);
return AVERROR_INVALIDDATA;
@@ -757,7 +757,7 @@ static int dvdvideo_play_next_ps_block(AVFormatContext *s, DVDVideoPlaybackState
if (!state->in_ps) {
if (c->opt_trim && !dvdvideo_is_cell_promising(s, state->pgc, state->celln)) {
- av_log(s, AV_LOG_INFO, "Skipping padding cell #%d\n", state->celln);
+ av_log(s, AV_LOG_INFO, "Trimming padding cell #%d\n", state->celln);
i = 0;
continue;
@@ -1175,8 +1175,7 @@ static int dvdvideo_audio_stream_analyze(AVFormatContext *s, audio_attr_t audio_
if (audio_attr.application_mode == 1) {
entry->disposition |= AV_DISPOSITION_KARAOKE;
- av_log(s, AV_LOG_WARNING, "Extended karaoke metadata is not supported at this time "
- "(stream id=%d)\n", startcode);
+ av_log(s, AV_LOG_WARNING, "Karaoke extensions not supported (stream id=%d)\n", startcode);
}
if (audio_attr.code_extension == 2)
@@ -1770,7 +1769,7 @@ static const AVOption dvdvideo_options[] = {
};
static const AVClass dvdvideo_class = {
- .class_name = "DVD-Video demuxer",
+ .class_name = "dvdvideo",
.item_name = av_default_item_name,
.option = dvdvideo_options,
.version = LIBAVUTIL_VERSION_INT
--
2.34.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 11+ messages in thread* [FFmpeg-devel] [PATCH 7/7] doc/demuxers: update dvdvideodec documentation
2024-07-28 7:34 [FFmpeg-devel] [PATCH 0/7] avformat/dvdvideodec: Bug fixes, seeking support, and menu chapters Marth64
` (5 preceding siblings ...)
2024-07-28 7:34 ` [FFmpeg-devel] [PATCH 6/7] avformat/dvdvideodec: Simplify/clarify logs, comments, and class name Marth64
@ 2024-07-28 7:34 ` Marth64
6 siblings, 0 replies; 11+ messages in thread
From: Marth64 @ 2024-07-28 7:34 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Marth64
Signed-off-by: Marth64 <marth64@proxyid.net>
---
doc/demuxers.texi | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/doc/demuxers.texi b/doc/demuxers.texi
index 04293c4813..d67be0312e 100644
--- a/doc/demuxers.texi
+++ b/doc/demuxers.texi
@@ -384,7 +384,7 @@ Default is 0, the first language unit.
@item menu_vts @var{int}
The VTS where the menu lives, or 0 if it is a VMG menu (root-level).
-Default is 0, VMG menu.
+Default is 1, the menu in the first VTS.
@item pgc @var{int}
The entry PGC to start playback, in conjunction with @option{pg}.
@@ -396,17 +396,16 @@ Default is 0, automatically resolve from value of @option{title}.
@item pg @var{int}
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.
+This option is ignored without @option{pgc}.
+Default is the first PG segment of the PGC (PG 1).
@item preindex @var{bool}
Enable this to have accurate chapter (PTT) markers and duration measurement,
which requires a slow second pass read in order to index the chapter marker
-timestamps from NAV packets. This is non-ideal extra work for real optical drives.
+timestamps from NAV packets. This also enables chapter markers on menus.
+The drawback is that this adds busy work for real optical drives.
It is recommended and faster to use this option with a backup of the DVD structure
stored on a hard drive. Not compatible with @option{pgc} and @option{pg}.
-Not applicable to menus.
Default is 0, false.
@item trim @var{bool}
--
2.34.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 11+ messages in thread