Message ID | 20240728073445.725161-3-marth64@proxyid.net |
---|---|
State | New |
Headers | show |
Series | avformat/dvdvideodec: Bug fixes, seeking support, and menu chapters | expand |
Context | Check | Description |
---|---|---|
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | success | Make fate finished |
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
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!
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 };
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(-)