Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Sean McGovern <gseanmcg@gmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Cc: Marth64 <marth64@proxyid.net>
Subject: Re: [FFmpeg-devel] [PATCH 2/7] avformat/dvdvideodec: Implement seeking
Date: Sun, 28 Jul 2024 11:20:41 -0400
Message-ID: <CAPBf_Ok6wr+4LduVJM=yrd0qg8SBH=vZFdmd5Adz4Fo7LFk98g@mail.gmail.com> (raw)
In-Reply-To: <20240728073445.725161-3-marth64@proxyid.net>

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".

  reply	other threads:[~2024-07-28 15:21 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 15:34   ` Marth64
2024-07-28  7:34 ` [FFmpeg-devel] [PATCH 2/7] avformat/dvdvideodec: Implement seeking Marth64
2024-07-28 15:20   ` Sean McGovern [this message]
2024-07-28 15:30     ` Marth64
2024-07-28  7:34 ` [FFmpeg-devel] [PATCH 3/7] avformat/dvdvideodec: Combine libdvdread and libdvdnav log callbacks Marth64
2024-07-28  7:34 ` [FFmpeg-devel] [PATCH 4/7] avformat/dvdvideodec: Chapter markers and trimming for menus Marth64
2024-07-28  7:34 ` [FFmpeg-devel] [PATCH 5/7] avformat/dvdvideodec: Remove unused headers 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAPBf_Ok6wr+4LduVJM=yrd0qg8SBH=vZFdmd5Adz4Fo7LFk98g@mail.gmail.com' \
    --to=gseanmcg@gmail.com \
    --cc=ffmpeg-devel@ffmpeg.org \
    --cc=marth64@proxyid.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git