[FFmpeg-devel] [PATCH v2] avformat/dvdvideodec: Simplify padding cell trim logic

[Marth64]

Please ignore this v2 patch also. There’s an issue with it.
Apologies again.

On Thu, Jun 27, 2024 at 01:06 Marth64 <marth64 at proxyid.net> wrote:

> When -trim option is used (by default), padding cells
> at the beginning of the title are supposed to be ignored.
> The current implementation does the ignoring when we
> attempt to lock on to the PGC navigation event stream,
> but this is a wrong assumption because we have already
> entered the PGC. This causes false positives on some
> discs due to a search for a program stream cell that
> never succeeds.
>
> User would have to know to disable the -trim option
> to work around the issue.
>
> Simplify the logic and move it to the MPEG-PS block
> reading event handling, in turn also fixing the trim
> function for these weirdly authored discs.
>
> Signed-off-by: Marth64 <marth64 at proxyid.net>
> ---
>  libavformat/dvdvideodec.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/libavformat/dvdvideodec.c b/libavformat/dvdvideodec.c
> index e7132725b7..f1c960b56b 100644
> --- a/libavformat/dvdvideodec.c
> +++ b/libavformat/dvdvideodec.c
> @@ -624,7 +624,6 @@ static int dvdvideo_play_next_ps_block(AVFormatContext
> *s, DVDVideoPlaybackState
>      dvdnav_vts_change_event_t *e_vts;
>      dvdnav_cell_change_event_t *e_cell;
>      int cur_title, cur_pgcn, cur_pgn, cur_angle, cur_title_unused,
> cur_ptt, cur_nb_angles;
> -    int is_cell_promising = 0;
>      pci_t *e_pci;
>      dsi_t *e_dsi;
>
> @@ -706,23 +705,17 @@ static int
> dvdvideo_play_next_ps_block(AVFormatContext *s, DVDVideoPlaybackState
>                      continue;
>
>                  e_cell = (dvdnav_cell_change_event_t *) nav_buf;
> -                is_cell_promising = !c->opt_trim ||
> dvdvideo_is_cell_promising(s, state->pgc, e_cell->cellN);
>
> -                av_log(s, AV_LOG_DEBUG, "new cell: prev=%d new=%d
> promising=%d\n",
> -                                        state->celln, e_cell->cellN,
> is_cell_promising);
> +                av_log(s, AV_LOG_DEBUG, "new cell: prev=%d new=%d\n",
> state->celln, e_cell->cellN);
>
>                  if (!state->in_ps && !state->in_pgc) {
>                      if (cur_title == c->opt_title
> &&
>                          (c->opt_pgc || cur_ptt == c->opt_chapter_start)
> &&
>                          cur_pgcn == state->pgcn
> &&
> -                        cur_pgn == state->entry_pgn
> &&
> -                        is_cell_promising) {
> +                        cur_pgn == state->entry_pgn) {
>
>                          state->in_pgc = 1;
>                      }
> -
> -                    if (c->opt_trim && !is_cell_promising)
> -                        av_log(s, AV_LOG_INFO, "Skipping padding cell
> #%d\n", e_cell->cellN);
>                  } else if (state->celln >= e_cell->cellN || state->pgn >
> cur_pgn) {
>                      return AVERROR_EOF;
>                  }
> @@ -784,9 +777,13 @@ static int
> dvdvideo_play_next_ps_block(AVFormatContext *s, DVDVideoPlaybackState
>
>                  return nav_len;
>              case DVDNAV_BLOCK_OK:
> -                if (!state->in_ps) {
> -                    if (state->in_pgc)
> -                        i = 0; /* necessary in case we are skipping junk
> cells at the beginning */
> +                if (!state->in_ps)
> +                    continue;
> +
> +                if (c->opt_trim && !dvdvideo_is_cell_promising(s,
> state->pgc, state->celln)) {
> +                    av_log(s, AV_LOG_DEBUG, "skipping block in trimmed
> cell #%d\n", state->celln);
> +
> +                    i = 0;
>                      continue;
>                  }
>
> --
> 2.34.1
>
>