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

[Marth64]

Please, ignore this particular patch. I have found a formatting
mistake (opportunity to remove braces).
I will send a v2.

I apologize for the inconvenience.

On Thu, Jun 27, 2024 at 12:38 AM 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 | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/libavformat/dvdvideodec.c b/libavformat/dvdvideodec.c
> index e7132725b7..b0fb5f0de2 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;
>                  }
> @@ -785,8 +778,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 */
> +                    continue;
> +                }
> +
> +                if (c->opt_trim && !dvdvideo_is_cell_promising(s, state->pgc, state->celln)) {
> +                    av_log(s, AV_LOG_DEBUG, "skipping packet in padding cell #%d\n", state->celln);
> +
> +                    i = 0;
>                      continue;
>                  }
>
> --
> 2.34.1
>